Skip to content

Commit

Permalink
Rename parameters when inlining if they shadow an existing bound name
Browse files Browse the repository at this point in the history
Fixes #73
  • Loading branch information
jonathanhogg committed Feb 9, 2025
1 parent c3fe14d commit 5e68b63
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 9 deletions.
25 changes: 19 additions & 6 deletions src/flitter/language/tree.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1018,21 +1018,34 @@ cdef class Call(Expression):
all_dynamic_args = False
else:
all_literal_args = False
cdef list bindings
cdef list bindings, renames
cdef dict kwargs
cdef int64_t i
cdef int64_t i, j=0
cdef str temp_name
if func_expr is not None and not func_expr.captures and not (func_expr.recursive and all_dynamic_args):
kwargs = {binding.name: binding.expr for binding in keyword_args}
bindings = []
renames = []
for i, binding in enumerate(func_expr.parameters):
if i < len(args):
bindings.append(PolyBinding((binding.name,), <Expression>args[i]))
expr = <Expression>args[i]
elif binding.name in kwargs:
bindings.append(PolyBinding((binding.name,), <Expression>kwargs[binding.name]))
expr = <Expression>kwargs[binding.name]
elif binding.expr is not None:
bindings.append(PolyBinding((binding.name,), binding.expr))
expr = binding.expr
else:
expr = Literal(null_)
if binding.name in context.names:
temp_name = f'__t{j}'
j += 1
while temp_name in context.names:
temp_name = f'__t{j}'
j += 1
bindings.append(PolyBinding((temp_name,), expr))
renames.append(PolyBinding((binding.name,), Name(temp_name)))
else:
bindings.append(PolyBinding((binding.name,), Literal(null_)))
bindings.append(PolyBinding((binding.name,), expr))
bindings.extend(renames)
if func_expr.recursive:
if context.call_depth == 0:
context.call_depth = 1
Expand Down
17 changes: 14 additions & 3 deletions tests/test_simplifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,16 @@ def test_simple_inlined_missing_default_parameter_used(self):
Literal(null),
static={'f': f})

def test_simple_inlined_shadowed_name(self):
"""An inlined function parameter must not shadow a bound name in case calculation of an argument
value needs it. The parameter will be renamed to a temporary name and the simplifier will
push that new name through into the body."""
f = Function('f', (Binding('x', None), Binding('y', None)), Add(Name('x'), Name('y')), captures=(), inlineable=True)
self.assertSimplifiesTo(Call(Name('f'), (Add(Name('z'), Literal(1)), Add(Name('x'), Literal(1)))),
Let((PolyBinding(('__t0',), Add(Name('z'), Literal(1))),
PolyBinding(('y',), Add(Name('x'), Literal(1)))), Add(Name('__t0'), Name('y'))),
static={'f': f}, dynamic={'x', 'z'})


class TestRecursiveCall(SimplifierTestCase):
def setUp(self):
Expand All @@ -1120,10 +1130,11 @@ def test_inlineable_recursive_all_non_literal(self):
static={'f': self.f}, dynamic={'z', 'w'})

def test_inlineable_recursive_literal_bound(self):
"""A call to a recursive function with at least literal arguments will cause an inline attempt.
In this case the literal argument determines the bounds and so recursive inlining will succeed."""
"""A call to a recursive function with at least one literal argument will cause an inline attempt.
In this case the literal argument determines the bounds and so recursive inlining will succeed.
Note that a temporary variable re-naming occurs here because of the inlining."""
self.assertSimplifiesTo(Call(Name('f'), (Literal(2), Name('w'))),
Add(Name('w'), Let((PolyBinding(('y',), Multiply(Literal(0.5), Name('w'))),), Positive(Name('y')))),
Add(Name('w'), Let((PolyBinding(('__t1',), Multiply(Literal(0.5), Name('w'))),), Positive(Name('__t1')))),
static={'f': self.f}, dynamic={'w'})

def test_inlineable_recursive_dynamic_bound(self):
Expand Down

0 comments on commit 5e68b63

Please sign in to comment.