Skip to content

transpile: Use raw pointers for lvalues with side effects#1764

Open
Rua wants to merge 7 commits into
immunant:masterfrom
Rua:lvalue-raw-pointer
Open

transpile: Use raw pointers for lvalues with side effects#1764
Rua wants to merge 7 commits into
immunant:masterfrom
Rua:lvalue-raw-pointer

Conversation

@Rua

@Rua Rua commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

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.

@ahomescu

ahomescu commented May 2, 2026

Copy link
Copy Markdown
Contributor

Is there no snapshot test that covers this? If not, could you add one?

@ahomescu ahomescu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment on snapshot test.

@Rua

Rua commented May 2, 2026

Copy link
Copy Markdown
Contributor Author

There is, but because of #1763 it doesn't trigger. I can add an additional test that does trigger this code path currently.

@Rua Rua force-pushed the lvalue-raw-pointer branch 3 times, most recently from 997bedc to e83155a Compare May 2, 2026 10:19
@Rua Rua requested a review from ahomescu May 5, 2026 09:34
@Rua Rua force-pushed the lvalue-raw-pointer branch from e83155a to 51826a0 Compare May 5, 2026 09:39
Comment thread c2rust-transpile/tests/snapshots/conditions.c Outdated
Comment thread c2rust-transpile/tests/snapshots/conditions.c
@Rua Rua force-pushed the lvalue-raw-pointer branch from 51826a0 to 5e80bd1 Compare May 6, 2026 14:15
@Rua Rua requested a review from ahomescu May 6, 2026 14:32
@Rua Rua force-pushed the lvalue-raw-pointer branch from 5e80bd1 to 08a4176 Compare May 6, 2026 14:56
Comment thread c2rust-transpile/src/translator/named_references.rs
Comment thread c2rust-transpile/src/translator/operators.rs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this be ctx.used()? Then you wouldn't need the other in the callee, like my other comment mentions.

@Rua Rua May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Rua Rua force-pushed the lvalue-raw-pointer branch 5 times, most recently from 451dc1f to 5a1bd5f Compare May 13, 2026 09:23
@Rua Rua force-pushed the lvalue-raw-pointer branch 2 times, most recently from afa06c4 to 0287123 Compare May 16, 2026 19:05
@Rua Rua force-pushed the lvalue-raw-pointer branch 3 times, most recently from 0fb2b7d to 8e540b4 Compare May 19, 2026 16:51
@Rua Rua force-pushed the lvalue-raw-pointer branch from 8e540b4 to 66b5b19 Compare May 30, 2026 14:24
// 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");

@ahomescu ahomescu Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let ptr_name = self.renamer.borrow_mut().pick_name("c2rust_side_effects");
let ptr_name = self.renamer.borrow_mut().pick_name("c2rust_lvalue");

@Rua Rua Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread c2rust-transpile/src/translator/named_references.rs
@Rua Rua force-pushed the lvalue-raw-pointer branch from 66b5b19 to 8b539b3 Compare June 19, 2026 10:29
@Rua Rua force-pushed the lvalue-raw-pointer branch from 8b539b3 to f396110 Compare June 19, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants