Skip to content

Commit 06c720f

Browse files
authored
Improve if and let* form Python generation (#792)
Fixes #793
1 parent f7da5a2 commit 06c720f

File tree

3 files changed

+31
-12
lines changed

3 files changed

+31
-12
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
* Allow vars to be callable to adhere to Clojure conventions (#767)
1919
* Adjust input path compatibility in `basilisp.core/load` input path to be relative to the namespace or the root path (#782)
2020
* No longer warn on unused bindings when their name begins with `_` (#756)
21+
* Improve the Python generation for `if` and `let*` forms to avoid unnecessary extra assignments (#793)
2122

2223
### Fixed
2324
* Fix issue with `(count nil)` throwing an exception (#759)

src/basilisp/lang/compiler/generator.py

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2138,10 +2138,18 @@ def _if_to_py_ast(ctx: GeneratorContext, node: If) -> GeneratedPyAST:
21382138
then_ast = __if_body_to_py_ast(ctx, node.then, result_name)
21392139
else_ast = __if_body_to_py_ast(ctx, node.else_, result_name)
21402140

2141-
test_name = genname(_IF_TEST_PREFIX)
2142-
test_assign = ast.Assign(
2143-
targets=[ast.Name(id=test_name, ctx=ast.Store())], value=test_ast.node
2144-
)
2141+
# Suppress the duplicate assignment statement if the `if` statement test is already
2142+
# an ast.Name instance.
2143+
if_test_deps: List[ast.AST] = []
2144+
if isinstance(test_ast.node, ast.Name):
2145+
test_name = test_ast.node.id
2146+
else:
2147+
test_name = genname(_IF_TEST_PREFIX)
2148+
if_test_deps.append(
2149+
ast.Assign(
2150+
targets=[ast.Name(id=test_name, ctx=ast.Store())], value=test_ast.node
2151+
)
2152+
)
21452153

21462154
ifstmt = ast.If(
21472155
test=ast.BoolOp(
@@ -2168,7 +2176,7 @@ def _if_to_py_ast(ctx: GeneratorContext, node: If) -> GeneratedPyAST:
21682176
node=ast.Name(id=result_name, ctx=ast.Load())
21692177
if result_name is not None
21702178
else _noop_node(),
2171-
dependencies=list(chain(test_ast.dependencies, [test_assign, ifstmt])),
2179+
dependencies=list(chain(test_ast.dependencies, if_test_deps, [ifstmt])),
21722180
)
21732181

21742182

@@ -2297,19 +2305,23 @@ def _let_to_py_ast(ctx: GeneratorContext, node: Let) -> GeneratedPyAST:
22972305
sym.symbol(binding.name), binding_name, LocalType.LET
22982306
)
22992307

2300-
let_result_name = genname(_LET_RESULT_PREFIX)
23012308
body_ast = _synthetic_do_to_py_ast(ctx, node.body)
23022309
let_body_ast.extend(map(statementize, body_ast.dependencies))
23032310

23042311
if node.env.pos == NodeSyntacticPosition.EXPR:
2305-
let_body_ast.append(
2306-
ast.Assign(
2307-
targets=[ast.Name(id=let_result_name, ctx=ast.Store())],
2308-
value=body_ast.node,
2312+
if isinstance(body_ast.node, ast.Name):
2313+
let_result_node = body_ast.node
2314+
else:
2315+
let_result_name = genname(_LET_RESULT_PREFIX)
2316+
let_result_node = ast.Name(id=let_result_name, ctx=ast.Load())
2317+
let_body_ast.append(
2318+
ast.Assign(
2319+
targets=[ast.Name(id=let_result_name, ctx=ast.Store())],
2320+
value=body_ast.node,
2321+
)
23092322
)
2310-
)
23112323
return GeneratedPyAST(
2312-
node=ast.Name(id=let_result_name, ctx=ast.Load()),
2324+
node=let_result_node,
23132325
dependencies=let_body_ast,
23142326
)
23152327
else:

tests/basilisp/compiler_test.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2900,6 +2900,9 @@ def test_if(self, lcompile: CompileFn):
29002900
"""
29012901
assert "YELLING" == lcompile(code)
29022902

2903+
def test_if_with_name(self, lcompile: CompileFn):
2904+
assert lcompile("(let* [a true] (if a :a :b))") == kw.keyword("a")
2905+
29032906
def test_truthiness(self, lcompile: CompileFn):
29042907
# Valid false values
29052908
assert kw.keyword("b") == lcompile("(if false :a :b)")
@@ -3244,6 +3247,9 @@ def test_let_may_have_empty_body(self, lcompile: CompileFn):
32443247
assert None is lcompile("(let* [])")
32453248
assert None is lcompile("(let* [a :kw])")
32463249

3250+
def test_let_return_bound_name(self, lcompile: CompileFn):
3251+
assert lcompile("(let* [a :a b a] b)") == kw.keyword("a")
3252+
32473253
def test_let_bindings_get_tag_ast(self, lcompile: CompileFn, ns: runtime.Namespace):
32483254
lcompile('(let [^python/str s "a string"] s)')
32493255
hints = typing.get_type_hints(ns.module)

0 commit comments

Comments
 (0)