Restrict promotion to infallible operations

[RalfJung]

Proposal

Summary and problem statement

I propose to resolve the "promotion mess" by only promoting code that we know will not fail to const-evaluate.

Motivation, use-cases, and solution sketches

Promotion of code to a const has been a constant source of "challenges" (aka problems) and surprises over the years. The first I saw is this soundness issue and since then we kept accumulating special cases in various parts of the compiler. Also see why we cannot promote arbitrary const fn calls, and this "meta issue".

I think we can solve this problem once and for all by ensuring that we only promote code that cannot fail to const-evaluate. Then we can get rid of all the code in rustc that has to somehow do something when evaluating a promoted failed. If we also make const_err a hard error we can in fact assume that const-evaluation errors are always directly reported to the user, which leads to even further simplifications and enables us to fix some diagnostics issues.

Technical note: it might seem that we have to rule out promotion of arithmetic in debug mode as overflows would cause evaluation failure. That is however not the case. An addition in release mode is compiled to a CheckedAdd MIR operation that never fails, which returns an (<int>, bool), and is followed by a check of said bool to possibly raise a panic. We only ever promote the CheckedAdd, so evaluation of the promoted will never fail, even if the operation overflows.

So I think we should work towards making all CTFE failures hard errors, and I started putting down some notes for that. However, this will require some breaking changes around promotion:

• We should no longer promote things like &(1/0) or &[2][12]. When promoting fallible operations like division, modulo, and indexing (and I think those are all the fallible operations we promote, but I might have missed something), then we have to make sure that this concrete promoted will not fail -- we need to check for div-by-0 and do the bounds check before accepting an expression for promotion. I propose we check if the index/divisor are constants, in which case the analysis is trivial, and just reject promotion for non-constant indices/divisors. If that is too breaking, a backup plan might be to somehow treat this more like CheckedAdd, where we promote the addition but not the assertion, which does ensure that the promoted never fails to evaluate even on overflow. (But I think that only works for divison/modulo, where we could return a "dummy value"; it doesn't work for indexing in general.)
• To achieve full "promoteds never fail", we have to severely dial back promotion inside const/static initializers -- basically to the same level as promotion inside fn and const fn. Currently there are two ways in which promotion inside const/static initializers is special here: first of all union field accesses are promoted (I am trying to take that back in stop promoting union field accesses in 'const' rust#77526), and secondly calls to all const fn are promoted. If we cannot take back both of these, we will instead need to treat MIR that originates from const/static initializers more carefully than other MIR -- even in code that ought to compile, there might be constants in there which fail to evaluate, so MIR optimizations and MIR evaluation (aka Miri) need to be careful to not evaluate such consts. This would be some unfortunate technical debt, but in my opinion still way better than the situation we currently find ourselves in.

Alternative: restrict promotion to patterns

I have in the past raised support for restricting promotion even further, namely to only those expressions that would also be legal as patterns. @ecstatic-morse has also expressed support for this goal. However, I now think that this is unnecessarily restrictive -- I do not see any further benefit that we would gain by ruling out expressions that will always succeed, but would not be legal as patterns. In any case, even if we want to go for pattern-only promotion in the future, that would only mean ruling out even more promotion than what I am proposing, so this proposal should still be a reasonable first step in that direction.

Prioritization

I guess this falls under the "Const generics and constant evaluation" priority.

Links and related work

• The problem with promotion: Solving the problem with promotion const-eval#53
• My plan for promotion
• Inline const expressions provide an way for code to basically opt-in to more powerful promotion. (At first they will likely not be allowed to depend on generics of the surrounding function, but that extension would be possible.)

Initial people involved

@ecstatic-morse, @oli-obk and me (aka the const-eval WG) have been talking about this and slowly chipping away at const promotion to make it less ill-behaved. The state described above is the result of as much cleanup as we felt comfortable doing just based on crater runs and the "accidental stabilization" argument.

What happens now?

This issue is part of the experimental MCP process described in RFC 2936. Once this issue is filed, a Zulip topic will be opened for discussion, and the lang-team will review open MCPs in its weekly triage meetings. You should receive feedback within a week or two.

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[rustbot]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[joshtriplett]

We discussed this in today's @rust-lang/lang meeting. Consensus was that we're in favor of this, as long as it doesn't cause serious breakage. The backup plans give more confidence in that approach.

@RalfJung Have you considered a future-incompatibility warning here, continuing to do full promotion for now but issuing future-incompat warnings for such cases?

We're assuming the work described here would happen under the auspices of the const-eval group.

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[oli-obk]

Have you considered a future-incompatibility warning here, continuing to do full promotion for now but issuing future-incompat warnings for such cases?

That's enormously hard to do. The root problem is that we don't know it's going to be a problem for the user until mir borrowck, but the user didn't request this to happen. So we can't do a future incompat lint on all cases that stop happening, because in many many cases making it not have the 'static lifetime has no effects on the compilation success of the code. As an example:

let x = &some_expr;

will keep compiling no matter whether some_expr stops getting promoted, but

let x: &'static _ = &some_expr;

will fail to compile when some_expr stops getting promoted.

While it would likely be possible to adjust mir borrowck to be able to kind of do both analyses at the same time and only emit a future incompat warning in case of some marker, I don't know if this can be done in a reasonable amount of work and complexity.

[nikomatsakis]

@rfcbot reviewed

[cramertj]

Checking my box assuming that this change will be paired with a crater run to check for major breakage / fallout.

[RalfJung]

Yes, we'll definitely use crater on each PR that could break something, and we'll get T-lang involved in the discussion should there be notable amounts of regressions. (The PRs we landed so far had 0 stable regressions in crater.)

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @joshtriplett, I wasn't able to add the final-comment-period label, please do so.

[nikomatsakis]

Pending PR: rust-lang/rust#77526

I raised the question on Zulip of whether we feel like an RFC is appropriate here. I do think that this is an area of "language design" where clarification would be useful, though I'm not sure if I feel the need for that to hold up landing PRs like rust-lang/rust#77526.

[nikomatsakis]

We discussed this in our lang-team meeting today. @RalfJung the sense was that we do feel like an RFC would be great here, even though it need not block implementation PRs. The point of the RFC would be to

• supercede the existing RFC on promotion and document the newer, more conservative model
• combined with pointing out the explicit syntax that can now be used
• given that we hope to achieve this transition without the use of editions, the RFC wouldn't have to cover that, though if editions are ultimately required it'd be a nice place to document what's going on

In other words, the goal of the RFC is to advertise and document the overall plan in a central place. We wouldn't anticipate a lot of opposition given the soundness implications and problems with the current model, though of course it may always happen that there are useful suggestions or other considerations that arise in the course of RFC discussion.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

[nikomatsakis]

FCP has completed. I'm going to close the issue for now, @RalfJung we should talk about the idea of drafting an RFC or other document as described above.

[RalfJung]

I did an RFC draft: https://github.com/RalfJung/rfcs/blob/infallible-promotion/text/0000-infallible-promotion.md

However, the "guide-level" and "reference-level" explanation are still mostly stubs. I have no idea what to put there. This is also taking waaay more time than I thought it would, which is kind of draining, so I'd really appreciate some help here.

Also see this Zulip thread.