Skip to content

Conversation

@Rua
Copy link
Contributor

@Rua Rua commented Sep 2, 2025

I noticed that there was some logic to determine whether reference-to-pointer decay happens automatically, or needs a cast. I don't know if that's still needed at all, I can strip it out if not. I never dealt with inline assembly before, so I wasn't sure how to handle references vs pointers in that context. I left it as it is for now.

@Rua
Copy link
Contributor Author

Rua commented Sep 5, 2025

Hmm, the CI failed on &raw mut not being allowed in statics. I thought I had tested that...

@Rua Rua force-pushed the raw-borrow branch 2 times, most recently from 5055982 to 1343170 Compare September 5, 2025 18:08
@Rua
Copy link
Contributor Author

Rua commented Sep 5, 2025

I asked around on The #dark-arts channel of the Rust Discord, and they assured me that &raw mut is allowed in statics. It seems that in current Rust, &raw mut is allowed in statics, but in the nightly being used for c2rust, it was not yet allowed?

@Rua Rua force-pushed the raw-borrow branch 2 times, most recently from ba9d215 to fd5ac23 Compare September 5, 2025 21:21
@bungcip
Copy link
Contributor

bungcip commented Sep 7, 2025

c2rust-transpile still use old nightly ( i think 2022-08-08), and in those version, &raw mut still unstable. I expect there is still bug in those version

@Rua
Copy link
Contributor Author

Rua commented Sep 7, 2025

In that nightly version, the addr_of and addr_of_mut macros are already stabilised. And internally, those macros just call &raw const and &raw mut. So any bugs would have affected stable code.

@Rua Rua marked this pull request as draft September 8, 2025 15:00
@Rua
Copy link
Contributor Author

Rua commented Sep 8, 2025

Sadly I am not able to make this work right now. I'm running into niche cases where a temporary is borrowed and its lifetime is extended. Rust does not extend the lifetime when using the raw borrow operator, only the normal borrow operator. This is making a few of the tests still fail.

If there is a way to know exactly in which cases the borrow operator references a temporary, then that could potentially solve the issue.

@Rua
Copy link
Contributor Author

Rua commented Sep 10, 2025

I think I found a way to deal with the issue, at least in the case of compound literals. That makes this pretty much ready I think, all the tests run successfully.

Two questions still remain:

@Rua Rua marked this pull request as ready for review September 10, 2025 16:05
Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this!

Does this fix #301, too?

Also, could you add the test cases from all of the issues? Preferably as snapshot tests.

@Rua Rua force-pushed the raw-borrow branch 4 times, most recently from 61ea8d7 to 5d2398a Compare September 13, 2025 10:57
@Rua
Copy link
Contributor Author

Rua commented Sep 13, 2025

Sorry for all the code churn. The commits kinda got messed up by rebasing and I decided to just redo everything from the start.

Currently, the CI is failing because it's trying to raw-borrow what is being translated to a const in Rust. In C, this is a macro that expands to a literal string. I think because it's a macro, the special exception for string literals here is no longer triggering, and it's instead being processed as a regular array-to-pointer decay. But because the macro expansion is being stored as a const, this doesn't work.
https://github.com/immunant/c2rust/blob/master/c2rust-transpile/src/translator/mod.rs#L4503

Is there a way to test whether an array-to-pointer decay expression is a macro expansion that was translated into a Rust const like this? Then I can add a special case to handle that situation.

@Rua Rua force-pushed the raw-borrow branch 2 times, most recently from decb06f to d6de3dd Compare September 24, 2025 09:42
@fw-immunant
Copy link
Contributor

Sorry for all the code churn. The commits kinda got messed up by rebasing and I decided to just redo everything from the start.

Currently, the CI is failing because it's trying to raw-borrow what is being translated to a const in Rust. In C, this is a macro that expands to a literal string. I think because it's a macro, the special exception for string literals here is no longer triggering, and it's instead being processed as a regular array-to-pointer decay. But because the macro expansion is being stored as a const, this doesn't work. https://github.com/immunant/c2rust/blob/master/c2rust-transpile/src/translator/mod.rs#L4503

Is there a way to test whether an array-to-pointer decay expression is a macro expansion that was translated into a Rust const like this? Then I can add a special case to handle that situation.

ctx.is_const on the AstContext indicates whether translation is currently in a const context. Translation can branch on that to either choose a different (const-compatible) idiom for expressing the C computation or to fail (return Err(_)) and the caller will attempt to translate the code without creating a const.

@Rua
Copy link
Contributor Author

Rua commented Sep 27, 2025

I'm marking this as a draft for now, because there's still some issues to be figured out regarding temporaries being borrowed. Probably depends on a solution for #1217 and #1379.

@Rua Rua marked this pull request as draft September 27, 2025 10:46
@Rua Rua changed the title Use raw borrows instead of normal borrows transpile: Use raw borrows instead of normal borrows Oct 14, 2025
@Rua
Copy link
Contributor Author

Rua commented Oct 14, 2025

ctx.is_const on the AstContext indicates whether translation is currently in a const context. Translation can branch on that to either choose a different (const-compatible) idiom for expressing the C computation or to fail (return Err(_)) and the caller will attempt to translate the code without creating a const.

I added a check for ctx.is_const, but it seems that the value is false, so the CI is generating raw-borrow errors instead of using as_ptr.

EDIT: I figured out I needed translate_as_macro instead. Now the tests are almost passing.

@Rua Rua force-pushed the raw-borrow branch 7 times, most recently from c8cd778 to 40abc63 Compare October 15, 2025 13:33
@Rua Rua force-pushed the raw-borrow branch 2 times, most recently from 2097b91 to 5159bd2 Compare October 26, 2025 17:42
@Rua Rua force-pushed the raw-borrow branch 2 times, most recently from 3f16658 to 198e9d7 Compare October 27, 2025 15:29
@Rua
Copy link
Contributor Author

Rua commented Nov 13, 2025

Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

I haven't abandoned this PR, but as I worked on it I kept finding more things that needed fixing first before this can work:

Thanks for the continued work! This is definitely a big issue, and fixing it would make our output significantly more sound, so it's very much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants