Skip to content

Conversation

@leonardt
Copy link
Owner

No description provided.

@leonardt leonardt requested a review from cdonovick August 22, 2019 23:46
@leonardt
Copy link
Owner Author

This actually introduces a different bug when looking at nested function calls

def test_double_nested_function_call():
    def bar(x):
        return x

    def baz(x):
        return x + 1

    @end_rewrite()
    @ssa()
    @begin_rewrite()
    def foo(a, b, c):
        if b:
            a = bar(a)
        else:
            a = bar(a)
        if c:
            b = bar(b)
        else:
            b = bar(b)
        return a, b
    print(inspect.getsource(foo))
    assert inspect.getsource(foo) == """\
def foo(a, b, c):
    a0 = bar(a)
    a1 = bar(a)
    a2 = a0 if b else a1
    b0 = bar(b)
    b1 = bar(b)
    b2 = b0 if c else b1
    __return_value0 = a2
    return __return_value0
"""

fails with

______________________________________ test_double_nested_function_call _______________________________________

    def test_double_nested_function_call():
        def bar(x):
            return x

        def baz(x):
            return x + 1

        @end_rewrite()
        @ssa()
        @begin_rewrite()
        def foo(a, b, c):
            if b:
                a = bar(a)
            else:
                a = bar(a)
            if c:
                b = bar(b)
            else:
                b = bar(b)
            return a, b
        print(inspect.getsource(foo))
>       assert inspect.getsource(foo) == """\
    def foo(a, b, c):
        a0 = bar(a)
        a1 = bar(a)
        a2 = a0 if b else a1
        b0 = bar(b)
        b1 = bar(b)
        b2 = b0 if c else b1
        __return_value0 = a2
        return __return_value0
    """
E       AssertionError: assert 'def foo(a, b...turn_value0\n' == 'def foo(a, b,...turn_value0\n'
E         Skipping 70 identical leading characters in diff, use -v to show
E           e a1
E         -     bar0 = bar if b else bar
E         -     b0 = bar0(b)
E         ?             -
E         +     b0 = bar(b)
E         -     b1 = bar0(b)...
E
E         ...Full output truncated (8 lines hidden), use '-vv' to show

tests/test_ssa.py:166: AssertionError
-------------------------------------------- Captured stdout call ---------------------------------------------
def foo(a, b, c):
    a0 = bar(a)
    a1 = bar(a)
    a2 = a0 if b else a1
    bar0 = bar if b else bar
    b0 = bar0(b)
    b1 = bar0(b)
    b2 = b0 if c else b1
    __return_value0 = a2, b2
    return __return_value0

@leonardt
Copy link
Owner Author

It seems to be ssa-ifying the function name, so perhaps there's some logic that relies on the fact that assignments are visited in the other order.

@leonardt
Copy link
Owner Author

(reverting the commit to swap the assign ordering fixes the issue with the function name being ssa-ified)

@cdonovick
Copy link
Collaborator

Visiting the RHS is important.

if visiting LHS first:

def foo()
   x = 1
   x = x + 1

becomes

def foo()
   x0 = 1
   x1 = x1 + 1

I'm confused as why function names would be ssa-ified visiting LHS first. Only names in the store context should be ssa-ified. I look into it.

@cdonovick cdonovick merged commit 0313b69 into master Aug 23, 2019
@cdonovick cdonovick deleted the func-arg-issue branch August 23, 2019 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants