-
Notifications
You must be signed in to change notification settings - Fork 270
Limit to write unused unary expr only when expr is not pure #1344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Negate
, Plus
, Complement
, and Not
operatorNegate
, Plus
, Complement
, and Not
operator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the CI failure, if you're only developing on Linux, the macOS-specific snapshots won't be automatically updated so you have to manually update them based on what CI (or a macOS machine) says.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, actually, this test is OS specific since the printf declarations are slightly different. It's an annoying difference, so you could just try to rewrite it without using printf for the side effect (or just declaring it yourself).
ok, now testcase is not using printf. let see the this PR pass the testsuite. |
Negate
, Plus
, Complement
, and Not
operatorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the help on this!
// to add them to stmts. | ||
if ctx.is_unused() { | ||
// Some unused unary operators (`-foo()`) may have side effects, so we need | ||
// to add them to stmts when arg expr is not pure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// to add them to stmts when arg expr is not pure | |
// to add them to stmts when `arg` is not pure. |
-i; | ||
+i; | ||
~i; | ||
!i; | ||
&i; | ||
*&i; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So these unary ops now emit nothing, and while that's generally correct (I haven't thought through the exact semantics of the *&
one yet), it may also delete code that was intentionally written in C, similar to #1326. For things like i++
, removing the i;
and only keeping the i += 1;
makes perfect sense, but I'm not sure for these other unary operators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, i thought that c2rust have policy to removing unused things, i saw some other code that skip generating rust ouput when ctx.is_unused() is true, if this is wrong I will the change the code to remove in increment expression only.
currently, c2rust transpiled this code
to this
which contain useless
i
expression. this is because UnaryExpr can contain side effect expression which cannot be ignored,however not all unary expr can contain side effects. this pull request limit to only Negate , Plus, Complement, and Not. However we can limit it to only expr which is not pure.