transpile: Refactor, document and correct uses of used() flag - #1975
transpile: Refactor, document and correct uses of used() flag#1975Rua wants to merge 12 commits into
used() flag#1975Conversation
Rua
changed the title
used() flagused() flag
c02d700 to
95b17f7
Compare
fw-immunant
left a comment
fw-immunant
left a comment
There was a problem hiding this comment.
Code changes all look good, but before merging I want to go verify that this doesn't interfere with the way Hayroll inserts no-op expressions into c2rust inputs to link C preprocessor macro expansions with their c2rust translations.
| op if op.is_logical() => { | ||
| let lhs = self.convert_condition(ctx, true, lhs)?; | ||
| let rhs = self.convert_condition(ctx, true, rhs)?; | ||
| let lhs = self.convert_condition(ctx.used(), true, lhs)?; |
There was a problem hiding this comment.
Shouldn't this inherit from the operator? E.g. in a == b the two operands are only used if the entire comparison is.
There was a problem hiding this comment.
Logical operators are || &&, that's a comparison operator.
| } | ||
| }?; | ||
|
|
||
| // Some unused unary operators (`-foo()`) may have side effects, so we need |
There was a problem hiding this comment.
Could you add a test for volatile reads and writes (something like volatile int vi; -vi;) and make sure we're not losing them? And __extension__ although I think we already have a test for that.
There was a problem hiding this comment.
I noticed that this code already loses the vi; statement in the current master (without the negation):
volatile int vi;
vi;Is that a problem? I'm not sure what the semantics of C are meant to be here. But if this is supposed to emit a volatile read, then it would need to be fixed separately from this PR.
There was a problem hiding this comment.
I've made a separate PR for that issue now, and made this one depend on it.
c81a115 to
394b8d4
Compare
3 participants
convert_side_effects_exprin more places #1976I've documented how
is_usedis supposed to be set, and added/removedused()accordingly.If you're wondering what's up with some of the code in the
exprs.ctest suddenly disappearing: The transpiler normally eliminates unused code, which includes unused results of binary expressions:1 + 1;gets eliminated altogether1 + func();gets reduced tofunc();But for some reason unary expressions were not handled the same way, and were emitted even if they didn't do anything. Now they get removed too.