Conversation
There was a problem hiding this comment.
thank you!
The overall approach works, but I'm not 100% sure that doing the work directly in the parser is the right thing to do.
See e.g. this comment:
#435 (comment)
One possible "more clean" solution is to have proper "AugSetItem" and "AugSetAttr" nodes, and the we do desugaring in astframe as we do for for loops and exising AugAssign. What do you think?
Moreover, we need to make sure that the index is evaluated only once.
E.g.:
def foo() -> int:
print('foo')
return 0
x[foo()] += 1
needs to print foo only once, but I think that with your change it would print it twice
|
|
||
| def test_aug_assign_subscript(self): | ||
| mod = self.compile(""" | ||
| from _list import list |
There was a problem hiding this comment.
I think it's better to use __spy__.interp_list here.
I'd like to avoid test_basic to depend on _list.spy, because that needs "the whole spy" to work, and thus it defeats a bit the utility of having a "test_basic"
|
Thanks for the review, the approach just seemed like the easiest solution but I do see that it breaks away from desugaring in the AST frame and the other issues you mentioned. I'll work on the cleaner solution when I can. |
Closes #177
This adds compound assignment support for attributes and subscripts.