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

Tracking Issue for infallible promotion #80619

Closed
1 of 2 tasks
RalfJung opened this issue Jan 2, 2021 · 17 comments
Closed
1 of 2 tasks

Tracking Issue for infallible promotion #80619

RalfJung opened this issue Jan 2, 2021 · 17 comments
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jan 2, 2021

This is a tracking issue for the RFC "3027" (rust-lang/rfcs#3027).
The RFC does not have a feature gate.

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

Implementation history

@RalfJung RalfJung added T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC A-const-eval Area: Constant evaluation (MIR interpretation) labels Jan 2, 2021
@RalfJung
Copy link
Member Author

RalfJung commented Jan 2, 2021

In #80243 we learned that there is at least some code (e.g. rodrimati1992/abi_stable_crates#46) that relies in non-trivial ways on const fn calls being promoted in a const/static initializer. We need to figure out how to move forward with such code. I see these options:

  • Live with the fact that promoteds in const/static initializers can fail even if they are not required for said initializer. This means no breaking change here, but it will require care in MIR optimizations, MIR printing, maybe more.
  • Make sure that all potentally-failing promoteds in const/static initializers are required to compute the value of the const. This will require some break change. There are different alternatives here:
    • Never promote anything that might fail -- this breaks some real code (no idea how much, it's only 2 crates in crater), but leads to a rather clean situation.
    • Only promote things that fail if they are in parts of the CFG that will definitely be evaluated -- that will however be rather surprising behavior, so maybe we want to do this only for old editions. In particular, a proper post-dominance analysis might be too expensive, so we might just check if the entire CFG is linear, or we only promote on the "linear head" of the CFG; in either case, adding a conditional somewhere in the code will stop promotion of things further down that code, which is rather odd.

@Aaron1011
Copy link
Member

Never promote anything that might fail -- this breaks some real code (no idea how much, it's only 2 crates in crater), but leads to a rather clean situation.

Can that code be made to compile with inline const blocks?

@RalfJung
Copy link
Member Author

RalfJung commented Jan 2, 2021

@Aaron1011 yes it can, but only once inline const blocks can use generics of the surrounding context.

@bstrie bstrie changed the title Tracking Issue for infallibe promotion Tracking Issue for infallible promotion Feb 14, 2021
@RalfJung
Copy link
Member Author

RalfJung commented May 9, 2021

I think this is probably the best option:

Live with the fact that promoteds in const/static initializers can fail even if they are not required for said initializer. This means no breaking change here, but it will require care in MIR optimizations, MIR printing, maybe more.

#85112 ensures that at least a normal build handles this correctly. @oli-obk any idea how to check other MIR consumers such as MIR printing for this?

@oli-obk
Copy link
Contributor

oli-obk commented May 9, 2021

MIR printing should just print the path to unevaluated constants and not eval them at all. The only other thing would be optimizations, but these aren't run for constants' bodies

@RalfJung
Copy link
Member Author

MIR printing should just print the path to unevaluated constants and not eval them at all.

As in, it already does that, or it should be changed to do that?

@oli-obk
Copy link
Contributor

oli-obk commented May 12, 2021

I believe it does that. At least I specifically remember seeing paths being printed for things that could have been evaluated.

@RalfJung
Copy link
Member Author

RalfJung commented May 15, 2021

If we end up deciding we are okay with promoting const fn calls in const/static bodies, we might then also accept other things like general array indexing or division -- we could accept basically everything that qualifies syntactically, i.e., anything that does not depend on surrounding variables. I am not sure what is the better choice -- this makes the rules even more inconsistent, but the rules already are wildly inconsistent to the extend that people do get confused by it (e.g., #85181).

@joshtriplett joshtriplett added the S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. label Jul 20, 2022
@joshtriplett
Copy link
Member

What items from RFC 3027 remain to be implemented?

We talked about this in today's @rust-lang/lang meeting, and we know that const { ... } still needs further implementation, but we weren't sure if that was a blocker for this or if all the work specified by the RFC was complete.

@RalfJung
Copy link
Member Author

@joshtriplett the main open question is what is listed as unresolved question in the issue:

What should we do about promoting const fn calls in const/static initializers? See this crater analysis and this Zulip thread.

Basically we still promote arbitrary function calls inside const/static initializers. We need to decide whether we want to

  • Live with this technical debt
  • Try to slowly phase it out in favor of const blocks (once those are stable)

This is a MIR building thing so it could potentially be done at an edition boundary. However, as long as any edition promotes arbitrary fn calls, MIR optimizations need to be careful and not overeagerly evaluate constants they find to determine their value. So the technical debt does not become any better by making this an edition thing. I am not sure whether doing this only for the sake of "rules of the latest edition are easier to explain" is worth it.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 15, 2022

Looks like inline_const will be stabilized soon. :D #104087

I wonder if that could let us move towards deprecating promotion more?

  • Entirely stop promoting function calls in a future edition (only promoting trivial things like &3)? Is "easier to explain language" sufficient reason to do that?
  • I guess we can't remove this for older editions but maybe we can desugar it to const blocks somewhere early, and thus simplify the promotion machinery?

@RalfJung
Copy link
Member Author

I'd really like to explain promotion entirely as "implicitly inserting const blocks" -- and ideally even implement it like that. So here's a plan:

  • Get inline_const stabilized, actually, soon, maybe, finally, eventually?
  • On all editions, promote (via implicit inline consts) the things that we currently promote in runtime code (in particular, these are all infallible)
  • On old editions, additionally promote (via implicit inline consts) const function calls. Show a warning that suggests to use inline consts instead; this is how we get compatibility with the newer edition.

This is a breaking change, for code like that:

const fn panic() { panic!(); }

const C: () = {
  if false { let x = &panic(); }
}

We'd start rejecting such code, on all editions, because the promoted becomes a required_const. But I hope such code to be rare...
Specifically this affects constants that have unreachable branches that contain an &call() that fails to evaluate. (If the branch is inside a const fn it's fine, only directly in the const does it become a problem.)

To get the desired compiler model changes, there's not really any non-breaking path -- the goal is to make it so that any const we can find in the body of an item can be evaluated and we know doing so will never cause new compilation failures.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 16, 2023

To do that we'll have to move promotion checks to THIR or even HIR, right?

Tho I guess we could do it in MIR if we figure out #115613 and thus can land #111693 without a perf regression

We need those, because we have no way of generating new DefIds (needed for those generated inline consts) in MIR

@RalfJung
Copy link
Member Author

I think it'd be better to do it in THIR/HIR, yes.

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 23, 2024
promotion: don't promote int::MIN / -1

Looks like I entirely forgot about this case when adding the div-by-zero check, which was supposed to ensure that we never promote operations that can fail...
Cc rust-lang#80619

This is a breaking change, so needs a crater run.
r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 24, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch.

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 24, 2024
…-obk

promotion: don't promote int::MIN / -1

Looks like I entirely forgot about this case when adding the div-by-zero check, which was supposed to ensure that we never promote operations that can fail...
Cc rust-lang#80619

This is a breaking change, so needs a crater run.
r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 24, 2024
…-obk

promotion: don't promote int::MIN / -1

Looks like I entirely forgot about this case when adding the div-by-zero check, which was supposed to ensure that we never promote operations that can fail...
Cc rust-lang#80619

This is a breaking change, so needs a crater run.
r? ``@oli-obk``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 25, 2024
Rollup merge of rust-lang#121515 - RalfJung:fallible-promotion, r=oli-obk

promotion: don't promote int::MIN / -1

Looks like I entirely forgot about this case when adding the div-by-zero check, which was supposed to ensure that we never promote operations that can fail...
Cc rust-lang#80619

This is a breaking change, so needs a crater run.
r? ``@oli-obk``
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 10, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 12, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :) Here's the t-lang [nomination comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 12, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :) Here's the t-lang [nomination comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 12, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :) Here's the t-lang [nomination comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 12, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :) Here's the t-lang [nomination comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 13, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :) Here's the t-lang [nomination comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 13, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :) Here's the t-lang [nomination comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 13, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :) Here's the t-lang [nomination comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 14, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :)
**Here's the t-lang [nomination comment](rust-lang#121557 (comment)

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 14, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :)
**Here's the t-lang [nomination comment](rust-lang#121557 (comment)

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 21, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :)
**Here's the t-lang [nomination comment](rust-lang#121557 (comment) And here's the [FCP comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 21, 2024
…t, r=<try>

experiment: never promote fn calls

Cc rust-lang#80619 -- it's been a while since we did this experiment, let's get an idea for what the ecosystem looks like today.
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 21, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :)
**Here's the t-lang [nomination comment](rust-lang#121557 (comment) And here's the [FCP comment](rust-lang#121557 (comment)).

r? `@oli-obk`
@RalfJung
Copy link
Member Author

Regarding the chance to entirely get rid of promotion of function calls -- currently, that breaks the build of cargo, due to this constant in time:

const UTC_OFFSET_FORMAT: &[FormatItem<'_>] = &[
    FormatItem::Component(Component::OffsetHour({
        let mut m = modifier::OffsetHour::default();
        m.sign_is_mandatory = true;
        m
    })),
    FormatItem::Optional(&FormatItem::Compound(&[
        FormatItem::Literal(b":"),
        FormatItem::Component(Component::OffsetMinute(modifier::OffsetMinute::default())),
        FormatItem::Optional(&FormatItem::Compound(&[
            FormatItem::Literal(b":"),
            FormatItem::Component(Component::OffsetSecond(modifier::OffsetSecond::default())),
        ])),
    ])),
];

The default() calls here are function calls which are being promoted.

This could easily be fixed by using associated consts instead of functions. But who knows what we'd run into next...

bors added a commit to rust-lang-ci/rust that referenced this issue Mar 23, 2024
…<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :)
**Here's the t-lang [nomination comment](rust-lang#121557 (comment) And here's the [FCP comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 23, 2024
…oli-obk

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :)
**Here's the t-lang [nomination comment](rust-lang#121557 (comment) And here's the [FCP comment](rust-lang#121557 (comment)).

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 23, 2024
…oli-obk

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :)
**Here's the t-lang [nomination comment](rust-lang#121557 (comment) And here's the [FCP comment](rust-lang#121557 (comment)).

r? `@oli-obk`
@RalfJung
Copy link
Member Author

#121557 landed. So we have finally achieved Elysium the property that all potentially failing consts are captured by required_consts. :)

I think we can consider the RFC to be fully implemented now. 🎉
I have opened #124328 to track cleaning up the "hacks" we had to add to get here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
Status: Idea
Development

No branches or pull requests

4 participants