Skip to content

Stabilize guaranteed compile time evaluation of unnamed constant items #93838

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

Closed
XrXr opened this issue Feb 9, 2022 · 10 comments
Closed

Stabilize guaranteed compile time evaluation of unnamed constant items #93838

XrXr opened this issue Feb 9, 2022 · 10 comments
Labels
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.

Comments

@XrXr
Copy link
Contributor

XrXr commented Feb 9, 2022

This is a request to stabilize the implementation fact that the definition of unnamed constant items are always evaluated at compile time (through CTFE). There is no associated RFC as this feature is already available in stable releases and this is mostly a request for using an FCP to reach consensus for future support.

The stabilization of const-panic introduced a way for CTFE to emit hard compile errors. This allows users to perform compile time assertions using familiar language constructs and a growing set of const APIs. Before const-panic, users could emit the const_err lint, which was less dependable since whether lints fail builds is configurable.

Authors of compile time assertions that utilize const-panic would probably like to have confidence that what they wrote is in fact evaluated by rustc during compilation. Currently, users can ensure evaluation by utilizing const contexts that are inputs to the type checker:

const _: [(); panic!("custom error")] = [];

This trick relies on the fact that type checking is guaranteed to happen during compilation. Utilizing this to initiate constant evaluation feels more cumbersome than necessary.

This report aims to stabilize a more obvious way to initiate compile time evaluation.

What is being stabilized

The current rustc behavior that unnamed constant definitions are evaluated at least once at compile time. Note that compile time evaluation happens even when the function enclosing the unnamed constant is unused:

fn unused() {
    const _: () = panic!("always a compile error");
}

Stabilizing this seems unlikely to cause maintenance issues in the future due to the constrained nature of unnamed constants. However, I defer to people with rustc maintenance experience to judge this claim.

What is not being stabilized

The evaluation timing of named constant items. Named constant items can be associated constants, which could use generics. Unnamed constants cannot be associated constants and cannot use generic parameters. While it might be useful to also stabilize the evaluation timing of named constant items, they require more consideration and are out of scope for this report.


I would like to thank Ralf Jung, Oli Scherer, and Eric Huss for the generous amount of guidance I received. Here is the Zulip discussion leading up to this stabilization request.

@Mark-Simulacrum Mark-Simulacrum added I-lang-nominated Nominated for discussion during a lang team meeting. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Feb 11, 2022
@joshtriplett
Copy link
Member

We discussed this in today's @rust-lang/lang meeting. We'd be fine with stabilizing such a guarantee.

We'd want to make sure that the language used to make the guarantee still allows the compiler to optimize away code (e.g. anything that doesn't have a visible side effect such as a panic).

@Mark-Simulacrum Mark-Simulacrum removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Feb 22, 2022
@joshtriplett
Copy link
Member

joshtriplett commented Feb 22, 2022

Do we already have tests that confirm this behavior?

If so, this may just need a reference PR that we can FCP.

@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-compiler-nominated Nominated for discussion during a compiler team meeting. labels Feb 22, 2022
@pnkfelix
Copy link
Member

(just passing this on to the compiler team to double-check that people there are okay with what's being proposed here. And if firmer language is required to be checked against the rust-lang/reference, then we need to check that nominations there show up on the T-compiler agenda.)

@apiraino
Copy link
Contributor

apiraino commented Mar 3, 2022

Discussed in T-compiler meeting (on Zulip)

@rustbot label -I-compiler-nominated

@rustbot rustbot removed the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Mar 3, 2022
@Aaron1011
Copy link
Member

Note that compile time evaluation happens even when the function enclosing the unnamed constant is unused:

I think we should clarify that this is true even for unused generic functions:

fn foo<T>() {
    const _: () = {
        panic!("Const eval!");
    };
}
error[[E0080]](https://doc.rust-lang.org/nightly/error-index.html#E0080): evaluation of constant value failed
 [--> src/lib.rs:3:9
](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)  |
3 |         panic!("Const eval!");
  |         ^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Const eval!', src/lib.rs:3:9
  |
  = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info)

Since (as @XrXr mentioned) unnamed const items cannot use generics, we evaluate unnamed const items in the body of unused generic functions.

@MauriceKayser
Copy link

MauriceKayser commented Oct 3, 2022

I think this is related:

struct A();
impl A {
    const _1: [(); panic!("1")] = [];
    const _2: () = panic!("2");
}
const _3: () = panic!("3");

Currently 1 and 3 panic, but 2 does not.

XrXr added a commit to XrXr/reference that referenced this issue Feb 3, 2023
See rust-lang/rust#93838
I think at this point everyone is onboard with blessing this
behavior as stable.
XrXr added a commit to XrXr/reference that referenced this issue Feb 3, 2023
See rust-lang/rust#93838
It seems like everyone is onboard with blessing this behavior as stable.
XrXr added a commit to XrXr/reference that referenced this issue Feb 3, 2023
See rust-lang/rust#93838
It seems like everyone is onboard with blessing this behavior as stable.
@XrXr
Copy link
Contributor Author

XrXr commented Feb 3, 2023

I submitted PR to the reference rust-lang/reference#1328 to document the timing.

We'd want to make sure that the language used to make the guarantee still allows the compiler to optimize away code (e.g. anything that doesn't have a visible side effect such as a panic).

@joshtriplett can I ask for a review of the language?

@XrXr
Copy link
Contributor Author

XrXr commented Feb 3, 2023

@MauriceKayser all of those are named constant items so is out of scope for this issue. I also find it odd that associated constants are not always evaluated. From my understanding the timing there is related to how impl blocks can be generic, so even though in this example it could be evaluated, it is not, as in general evaluation is not possible until the generic parameters are concrete. This gives a sort of consistency, even though it's a bit surprising to me.

In any case, it's not in-scope for this issue.

@XrXr
Copy link
Contributor Author

XrXr commented Feb 14, 2023

Do we already have tests that confirm this behavior?

The only related test I found is const-eval/unused-broken-const.rs, first introduced in 2018. It doesn't test const _ specifically and doesn't test the unused generic function case mentioned above. I guess the discussed cases are covered because UnusedBrokenConst: LateLintPass.

There are many tests that implicitly depend on the definition being evaluated, though. Some examples:

Should I expand const-eval/unused-broken-const.rs?

XrXr added a commit to XrXr/reference that referenced this issue Feb 14, 2023
Especially since it seems that there is no test in rust-lang/rust that
covers the unamed constant case (even though the implementation covers
it). rust-lang/rust#93838 (comment)
@XrXr
Copy link
Contributor Author

XrXr commented Mar 15, 2023

Now that rust-lang/reference#1328 is merged, I'm going to go ahead and close this. This functionality is probably stable enough even without any explicit confirmation through an FCP, RFC or what have you. Weakening this guarantee would probably break a lot of crates in the ecosystem, so I trust that such change will not happen without careful consideration.

Thanks everyone for your attention!

@XrXr XrXr closed this as completed Mar 15, 2023
Tiger0202 added a commit to Tiger0202/rust-lang that referenced this issue Dec 11, 2024
See rust-lang/rust#93838
It seems like everyone is onboard with blessing this behavior as stable.
Tiger0202 added a commit to Tiger0202/rust-lang that referenced this issue Dec 11, 2024
Especially since it seems that there is no test in rust-lang/rust that
covers the unamed constant case (even though the implementation covers
it). rust-lang/rust#93838 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

No branches or pull requests

8 participants