transpile: Use raw pointers for lvalues with side effects#1764
Conversation
|
Is there no snapshot test that covers this? If not, could you add one? |
ahomescu
left a comment
There was a problem hiding this comment.
See previous comment on snapshot test.
|
There is, but because of #1763 it doesn't trigger. I can add an additional test that does trigger this code path currently. |
997bedc to
e83155a
Compare
There was a problem hiding this comment.
Couldn't this be ctx.used()? Then you wouldn't need the other in the callee, like my other comment mentions.
There was a problem hiding this comment.
It could, but putting it on the convert_cast call directly matches the other code and generally makes it more explicit that it's there.
451dc1f to
5a1bd5f
Compare
afa06c4 to
0287123
Compare
0fb2b7d to
8e540b4
Compare
| // This is the case where we explicitly need to factor out possible side-effects. | ||
|
|
||
| let ptr_name = self.renamer.borrow_mut().fresh(); | ||
| let ptr_name = self.renamer.borrow_mut().pick_name("c2rust_side_effects"); |
There was a problem hiding this comment.
| let ptr_name = self.renamer.borrow_mut().pick_name("c2rust_side_effects"); | |
| let ptr_name = self.renamer.borrow_mut().pick_name("c2rust_lvalue"); |
There was a problem hiding this comment.
It's not technically the lvalue, but a pointer to it. Would c2rust_lvalue_ptr make more sense?
Asking mainly because c2rust_lvalue is already used for storing a compound literal as an lvalue, in cases where its address is taken. I don't know if it's desirable here to use the same name for a different meaning.
Named references needed to be removed from binary conditionals because it prevented this from working. Named references are intended for lvalues, but binary conditionals are rvalues. That means they can't be raw-borrowed, but can be stored by value.