Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up implicit const promotion behavior #124328

Open
RalfJung opened this issue Apr 24, 2024 · 0 comments
Open

Clean up implicit const promotion behavior #124328

RalfJung opened this issue Apr 24, 2024 · 0 comments
Labels
A-const-prop Area: Constant propagation A-maybe-future-edition Something we may consider for a future edition. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. WG-const-eval Working group: Const evaluation

Comments

@RalfJung
Copy link
Member

RalfJung commented Apr 24, 2024

In #80619 we tamed promotion to no longer cause fundamental problems around whether and when const-evaluation is allowed to fail. All our promoteds are now either infallible, or in guaranteed-live code.

However, as part of this, the rules for what we promote when became "a bit" obscure:

  • &(1/1) gets promoted, but &(1/(1+0)) does not. This is because when the RHS of the division is not a constant, it may be 0, and computing the promoted value would fail, so we can't promote this.
  • Inside the initializer expression of a const/static, &myfunction() gets promoted only if the control flow graph of the initializer is a straight line until it reaches this point. (See here for examples.) This is to ensure that we do not promote function calls in dead code.
  • &Enum::Variant gets promoted even if other enum variants contain UnsafeCell, which is borderline unsound.

We should probably clean that up, by not promoting function calls or division/modulo ever (requiring a const block instead), and also not doing value-based reasoning for interior mutability during promotion. I assume this requires an edition transition. (Here is a crater run that just tried to not promote function calls ever. Cargo itself doesn't even build any more so we did not get numbers.) Edition transitions for promotions are non-trivial; they would have to work something like this:

  • Do not promote these things.
  • Run borrowck.
  • If that fails, try promoting them and see if that makes borrowck pass. If yes, issue a forward-compat lint.

That's non-trivial but would also help clean up a very organically grown (and hence confusingly-shaped) part of the language, and resolve some cases that are at least borderline unsound.

FWIW, for division/modulo, there is in theory an alternative -- we could treat them similar to what we do with overflows in add/sub/mul. This requires having a version of these operators that always succeeds. Then we can promote &(a/b) to using that kind of division, and additionally include a runtime check (outside the promoted) for division-by-0 (and for overflow due to division-by-int-min).

Cc @rust-lang/wg-const-eval

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 24, 2024
@RalfJung RalfJung changed the title Clean up const promotion behavior Clean up implicit const promotion behavior Apr 24, 2024
@fmease fmease added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-const-prop Area: Constant propagation C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-const-eval Working group: Const evaluation and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 24, 2024
@bstrie bstrie added the A-maybe-future-edition Something we may consider for a future edition. label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-prop Area: Constant propagation A-maybe-future-edition Something we may consider for a future edition. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. WG-const-eval Working group: Const evaluation
Projects
None yet
Development

No branches or pull requests

4 participants