Open BenTheKush opened 1 year ago
For what it's worth, from a client perspective I think there is strong value added for an AST library to not do this desugaring of a += b
-> a = a + b
. I may want to treat these two operations differently, and it's possible that I will try to access a source location that doesn't exist.
For instance, there is no way for me to access the location of the +
in add expression aside from looking between the left and right expressions in the original source. But now, I need to double check that I don't end up with a +=
instead of a +
...it's weird that we aren't guaranteed that Add{left, right}
corresponds to a +
operator in the original code.
Anyway, not the point of this issue, just my two cents
You're right, we should have an Expression::AddAssign
in the AST. There are a few other things we de-sugar which we should not, like type()
functions.
Describe the bug An expression
a += b
is parsed as apt::Expression::AssignAdd{left, right}
. When this is translated from apt
to anast
it is desugared into anAssign { ..., Expression::Add { left, right}}
. Whenleft
is a Storage access,left
's location is wrong when it gets desugared: it ends up spanning the entireAssignAdd
expression instead of just the left expression.To Reproduce I have a repository that reproduces the bug.
Hyperledger Solang version
Include the complete solidity source code
Additional context Running my code visits each binary expression (after desugaring), and checks to ensure that the
left.loc().end() < right.loc.start()
, and if not, prints an error. This is the output of my test on the above program:It's clear that the bug exists only when the LHS of an op-assign (
+=
,|=
) is a storage access. This does not occur for local variables.