fix assignment/access ordering in comprehensions #423
Conversation
facebook-github-bot
added
the
CLA Signed
Codecov Report
@@ Coverage Diff @@
## master #423 +/- ##
==========================================
+ Coverage 94.31% 94.32% +0.01%
==========================================
Files 232 232
Lines 22531 22574 +43
==========================================
+ Hits 21251 21294 +43
Misses 1280 1280
Continue to review full report at Codecov.
|
| a_param = f.params.params[0].name | ||
| a_param_assignment = list(scopes[a_param]["a"])[0] | ||
| a_param_refs = list(a_param_assignment.references) | ||
| self.assertEqual(a_param_refs, []) |
There was a problem hiding this comment.
Doesn't the param a have references in the comprehensions (the last a appear in the comprehension)?
[a for a in [] for b in a]
[b for a in [] for b in a]
There was a problem hiding this comment.
no, that last a refers to the binding in the first for, so it's equivalent to:
def f(a_1):
[a_2 for a_2 in [] for b_1 in a_2]
[b_2 for a_3 in [] for b_2 in a_3]
[a_5 for a_4 in [] for a_5 in []]
There was a problem hiding this comment.
Got it. I didn't aware the nested for loop comprehension.
[a for a in a] is an example that I'm curious about whether it'll work fine.
The 3rd a should be an access of the function parameter a.
The first a is an Access of the Assignment of the 2nd a.
Can you also add this as a test case?
| a_param = f.params.params[0].name | ||
| a_param_assignment = list(scopes[a_param]["a"])[0] | ||
| a_param_refs = list(a_param_assignment.references) | ||
| self.assertEqual(a_param_refs, []) |
There was a problem hiding this comment.
Got it. I didn't aware the nested for loop comprehension.
[a for a in a] is an example that I'm curious about whether it'll work fine.
The 3rd a should be an access of the function parameter a.
The first a is an Access of the Assignment of the 2nd a.
Can you also add this as a test case?
Summary
This change fixes attaching accesses to the right assignment in list comprehensions, so in
The
aat the beginning of the list comprehension is now attached to theadeclared afterforinstead of the declaration on the first line.Test Plan
Added unit tests