Add += functionality #225
Add += functionality #225
Conversation
src/type_checker/checker.rs
Outdated
| @@ -389,6 +389,112 @@ impl<B: Backend> TypeChecker<B> { | |||
| | Op2::Division | |||
| | Op2::BoolAnd | |||
| | Op2::BoolOr => lhs_node.typ, | |||
| Op2::PlusEqual => { | |||
| // todo: better way to do this instead of copying assignment logic? | |||
There was a problem hiding this comment.
Here I copied the same logic from ExprKind::Assignment. I am not sure if this is the correct way to do it.
There was a problem hiding this comment.
Yeah, this looks redundant to the assignment.
One alternative may be to transform the expression from lhs += rhs to lhs = lhs + rhs. The transformation would need to be before the type checking phase. Maybe this can happen in the name resolution pipeline:
noname/src/name_resolution/expr.rs
Line 55 in 78ff4ba
@mimoo wdyt?
There was a problem hiding this comment.
@bufferhe4d
I just had a discussion with @mimoo. We see the += more like a shortcut instead of new operator. We thought it would be better to implement the transformation as described above at the name resolution. That would simplify the PR and avoid the redundancy in the type checking.
Please feel free to comment if you have any questions.
There was a problem hiding this comment.
Alright, this looks like a much more elegant approach. Thanks. Few questions:
- If we decide to do it under name resolution, it feels like either we need to keep a new operator under Op2 to recognize this op and resolve. Or we need a new ExprKind (could be called AugmentedAssignment). I guess you would prefer the latter to get rid of defining a new operation under Op2 Right?
- If we also wouldn't prefer to create a new ExprKind . Shall I attempt to make this resolution directly in parse_rhs ?
Line 410 in 78ff4ba
There was a problem hiding this comment.
It would be better if we can avoid a new Op2 for this case.
So looks like the parse_rhs is the way to go. That is a great insight @bufferhe4d 👍
There was a problem hiding this comment.
Alright, here is my new attempt. I also went ahead and implemented -= and *= inside parse_rhs, added some tests as well. I hope this works as intended now. Please let me know if anything is problematic. Thanks.
This reverts commit 1bfb7ad.
for +=, -=, *=
|
There are CI errors on formatting. You may want to run |
Indeed, should be good to go now. |
|
Merged. Thanks again! |
Attempt to resolve #175