Skip to content

Conversation

fw-immunant
Copy link
Contributor

Atop #1306, as I'd rather not regress all const translations to be unsafe in master now that we translate const macros by default.

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.

See #1335 (review), too, but in general, what's the difference between the unsafe block for a const and the unsafe on an unsafe fn (which in the latest edition is meant to also be an unsafe block in the unsafe fn, too)? They seem exactly the same to me, and I think we should approach them the same. So a change like this (which I haven't looked through all of the details of yet) makes sense to me if it also applies to an unsafe fn. Note that there are often a bunch of unsafe fns that are fully safe and don't need the unsafe, and removing the unsafe for a const that doesn't need it and remove the unsafe for a fn that doesn't need it seem like the exact same problem.

@fw-immunant
Copy link
Contributor Author

See #1335 (review), too, but in general, what's the difference between the unsafe block for a const and the unsafe on an unsafe fn (which in the latest edition is meant to also be an unsafe block in the unsafe fn, too)? They seem exactly the same to me, and I think we should approach them the same. So a change like this (which I haven't looked through all of the details of yet) makes sense to me if it also applies to an unsafe fn. Note that there are often a bunch of unsafe fns that are fully safe and don't need the unsafe, and removing the unsafe for a const that doesn't need it and remove the unsafe for a fn that doesn't need it seem like the exact same problem.

The difference is that there is no unsafe block surrounding definitions of consts prior to #1335 (nor after the first commit of this PR, transpile: revert set consts unsafe). This change (transpile: in const contexts, use `unsafe` around unsafe initializers) is really about following the same logic for initializers as we already follow for default zero values, several lines below: prior to this commit, our translation here follows the TranslationContext's notion of whether we're in a const context for the default zero values but doesn't for initializers.

When we change the edition we target, it will make sense to use WithStmts::to_unsafe_pure_expr in more places and we won't need to dispatch on the constness of the context to decide between to_unsafe_pure_expr and into_value anymore. The precision that this PR adds w/r/t which generated expressions are unsafe will also be useful for the translation of function bodies at that point.

@kkysen kkysen force-pushed the kkysen/expanded-translate-const-macros-conservative branch 2 times, most recently from 81551c0 to ecddc95 Compare September 2, 2025 06:00
Base automatically changed from kkysen/expanded-translate-const-macros-conservative to master September 2, 2025 06:28
@fw-immunant
Copy link
Contributor Author

History is cleaned up now; this should be good to merge.

@kkysen kkysen force-pushed the fw/const-less-unsafe branch from b06febb to 7dbe6b1 Compare September 22, 2025 09:46
@kkysen kkysen merged commit 973811e into master Sep 22, 2025
5 checks passed
@kkysen kkysen deleted the fw/const-less-unsafe branch September 22, 2025 10:12
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