Implement proper stability check for `const impl Trait`, fall back to unstable `const` when undeclared #93960

jhpratt · 2022-02-13T11:04:30Z

^{Doing these as one PR because it fit with the way I was writing code. I can split them into separate commits if desired, though there's not much point.}

Rather than deferring to const eval for checking if a trait is const, we now check up-front. This allows the error to be emitted earlier, notably at the same time as other stability checks.

Also included in this PR is a change of the default const stability level to UNstable. Previously, an item that was const but did not explicitly state it was unstable was implicitly stable.

r? @oli-obk

@rustbot label +A-const-fn +A-stability +C-enhancement +F-const-trait-impl +S-waiting-on-review +T-compiler -T-rustdoc

rust-highfive · 2022-02-13T11:04:32Z

Some changes occurred in clean/types.rs.

cc @camelid

compiler/rustc_attr/src/builtin.rs

compiler/rustc_const_eval/src/transform/check_consts/mod.rs

compiler/rustc_interface/src/passes.rs

compiler/rustc_passes/src/stability.rs

src/test/ui/rfc-2632-const-trait-impl/staged-api.rs

camelid · 2022-02-16T03:45:21Z

Also included in this PR is a change of the default const stability level to UNstable. Previously, an item that was const but did not explicitly state it was unstable was implicitly stable.

Does this break anything that was already (potentially accidentally) stable? I thought it wasn't possible to have a const function in the stdlib without a const stability attribute.

jhpratt · 2022-02-16T04:03:18Z

There were a couple of methods that didn't have it, but they were intentionally stabilized as const. That was fixed in #90998. This just changes the fallback to be unstable as a safety net, so to speak.

oli-obk

Can we now remove // build-fail from some tests?

compiler/rustc_const_eval/src/transform/check_consts/mod.rs

compiler/rustc_interface/src/passes.rs

compiler/rustc_passes/src/stability.rs

jhpratt

And yes, // build-fail can likely be removed from some tests.

compiler/rustc_interface/src/passes.rs

compiler/rustc_const_eval/src/transform/check_consts/mod.rs

oli-obk · 2022-02-17T06:35:05Z

I would prefer solution 2, but I realize it may be a larger diff or require duplication, so i'll trust your judgement on that.

I don't think we'll ever remove this stability check, after all, we will keep adding new traits and possibly make more of them const. Just like we have the const fn stability checks even though const fn is stable

jhpratt · 2022-02-17T06:49:50Z

True, I was thinking of the part that emits the error, which is only part of the change. I'll look into (2).

jhpratt · 2022-02-18T08:39:45Z

Do you see the need for both the stability.rs and the staged-api.rs tests? They will do pretty much the same thing after this change.

oli-obk · 2022-02-18T10:53:31Z

Yea they could be merged in case there is something only tested in one of them

jhpratt · 2022-02-22T20:27:48Z

I need to implement a check for a const trait impl being unstable with the feature enabled, as right now it appears that the compiler doesn't take this into account with regard to unstable-in-stable checking. Aside from that, I've merged the check into the existing stability pass.

jhpratt · 2022-02-27T20:19:45Z

Status update: I'm trying to figure out how to get RUSTC_LOG to work, which would let me debug the existing situation in the compiler. For some reason I can't get it to work at all, as the log level is capped. Once I get that working, it shouldn't be too difficult to implement this check 🤞🏽

oli-obk · 2022-02-28T07:18:56Z

Status update: I'm trying to figure out how to get RUSTC_LOG to work, which would let me debug the existing situation in the compiler. For some reason I can't get it to work at all, as the log level is capped. Once I get that working, it shouldn't be too difficult to implement this check 🤞🏽

You need to set debug-logging = true in config.toml

jhpratt · 2022-03-01T19:02:47Z

Already did that to no avail, unfortunately.

oli-obk · 2022-03-01T21:17:15Z

Note: this will only work for the libstd built with the changed compiler. The initial libstd build will still be done with a downloaded stage0 compiler, maybe that's what you're seeing?

jhpratt · 2022-03-01T21:25:26Z

In that case, how can I ensure it's built myself? x clean by itself isn't sufficient: I've tried.

oli-obk · 2022-03-01T22:12:01Z

My recommendation would be to create a ui test that reproduces the issue, and then only use logging on that test.

If you do want to log libcore, you just need to wait until it gets built after the compiler is built once. Logging will work there. You will still get lots of messages about disabled log levels while building stage 0 libstd and while building the compiler, but after the compiler is built, you will get logging output

bors · 2022-03-04T08:44:50Z

☔ The latest upstream changes (presumably #94096) made this pull request unmergeable. Please resolve the merge conflicts.

This avoids an ambiguity (when reading) where `.level.is_stable()` is not immediately clear whether it is general stability or const stability.

Rather than deferring to const eval for checking if a trait is const, we now check up-front. This allows the error to be emitted earlier, notably at the same time as other stability checks. Also included in this commit is a change of the default const stability level to UNstable. Previously, an item that was `const` but did not explicitly state it was unstable was implicitly stable.

This alters the diagnostics a bit, as the trait method is still stable. The only thing this check does is ensure that compilation fails if a trait implementation is declared const-stable.

rust-log-analyzer · 2022-03-16T04:20:58Z

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

...............................iii.................................................................. 12700/12736
....................................
failures:

---- [ui] ui/rfc-2632-const-trait-impl/staged-api.rs#unstable stdout ----

error in revision `unstable`: /checkout/src/test/ui/rfc-2632-const-trait-impl/staged-api.rs:40: expected error not found: not yet stable as a const fn

error in revision `unstable`: 0 unexpected errors found, 1 expected errors not found
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/rfc-2632-const-trait-impl/staged-api.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--cfg" "unstable" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfc-2632-const-trait-impl/staged-api.unstable" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfc-2632-const-trait-impl/staged-api.unstable/auxiliary"
    Error {
        line_num: 40,
        kind: Some(
            Error,
            Error,
        ),
        msg: "not yet stable as a const fn",
]


thread '[ui] ui/rfc-2632-const-trait-impl/staged-api.rs#unstable' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:1382:13
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
---- [ui] ui/rfc-2632-const-trait-impl/staged-api.rs#stable stdout ----


error in revision `stable`: /checkout/src/test/ui/rfc-2632-const-trait-impl/staged-api.rs:18: unexpected error: '18:1: 21:2: implementation has missing const stability attribute'

error in revision `stable`: /checkout/src/test/ui/rfc-2632-const-trait-impl/staged-api.rs:18: expected error not found: trait implementations cannot be const stable yet

error in revision `stable`: 1 unexpected errors found, 1 expected errors not found
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/rfc-2632-const-trait-impl/staged-api.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--cfg" "stable" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfc-2632-const-trait-impl/staged-api.stable" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfc-2632-const-trait-impl/staged-api.stable/auxiliary"
    Error {
        line_num: 18,
        kind: Some(
            Error,
---
        line_num: 18,
        kind: Some(
            Error,
        ),
        msg: "trait implementations cannot be const stable yet",
]


thread '[ui] ui/rfc-2632-const-trait-impl/staged-api.rs#stable' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:1382:13

failures:
    [ui] ui/rfc-2632-const-trait-impl/staged-api.rs#stable
    [ui] ui/rfc-2632-const-trait-impl/staged-api.rs#unstable

jhpratt · 2022-03-16T20:41:59Z

Alright. So after rebasing and getting RUSTC_LOG working, it looks like the problem is that the const fn is being passed to is_const_stable_const_fn. We then try to get the parent of that, which naturally isn't the trait. What needs to be done is checking the function calls inside of the current parameter. I'm currently looking into the best way to do this. I presume there's something similar already in the codebase.

JohnCSimon · 2022-04-11T02:21:33Z

@jhpratt
Ping from triage: Can you please post the status of this PR?
Thank you.

jhpratt · 2022-04-11T02:22:45Z

I still plan on working on it! I've been busy with some other things and plan on picking it back up in the near future.

jhpratt · 2022-05-14T19:20:33Z

I asked on Zulip if there was anyone interested in taking this PR over (no responses yet). I have other priorities in the compiler, and I know there are still some design decisions to be made for impl const Trait, so this isn't a blocker of anything.

If anyone would like to take this over, please say so! If not, I'll likely close this pull request, at least for the time being. Anyone is free to use the code in this PR in a future PR (or more generally under MIT or Apache-2.0).

jhpratt · 2022-05-17T22:29:35Z

Closing for the reason stated above.

@rustbot label -S-waiting-on-author +S-inactive

Implement proper stability check for const impl Trait, fall back to unstable const when undeclared Continuation of rust-lang#93960 `@jhpratt` it looks to me like the test was simply not testing for the failure you were looking for? Your checks actually do the right thing for const traits?

rust-highfive assigned oli-obk Feb 13, 2022

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 13, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 13, 2022

jhpratt commented Feb 13, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

jhpratt force-pushed the const-stability branch from d156433 to d9849cc Compare February 14, 2022 00:50

oli-obk requested changes Feb 16, 2022

View reviewed changes

compiler/rustc_const_eval/src/transform/check_consts/mod.rs Outdated Show resolved Hide resolved

compiler/rustc_interface/src/passes.rs Outdated Show resolved Hide resolved

compiler/rustc_passes/src/stability.rs Outdated Show resolved Hide resolved

jhpratt commented Feb 17, 2022

View reviewed changes

compiler/rustc_interface/src/passes.rs Outdated Show resolved Hide resolved

compiler/rustc_const_eval/src/transform/check_consts/mod.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2022

jhpratt added 3 commits March 15, 2022 22:53

Fall back to being const-unstable when undeclared

0b94fa4

Add and use stability helper methods

9a52401

This avoids an ambiguity (when reading) where `.level.is_stable()` is not immediately clear whether it is general stability or const stability.

Move check to existing pass

6a45f4d

This alters the diagnostics a bit, as the trait method is still stable. The only thing this check does is ensure that compilation fails if a trait implementation is declared const-stable.

jhpratt force-pushed the const-stability branch from f24e607 to 6a45f4d Compare March 16, 2022 04:04

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 8, 2022

jhpratt closed this May 17, 2022

rustbot added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 17, 2022

oli-obk mentioned this pull request May 19, 2022

Implement proper stability check for const impl Trait, fall back to unstable const when undeclared #97177

Merged

jhpratt deleted the const-stability branch June 8, 2022 04:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement proper stability check for `const impl Trait`, fall back to unstable `const` when undeclared #93960

Implement proper stability check for `const impl Trait`, fall back to unstable `const` when undeclared #93960

jhpratt commented Feb 13, 2022 •

edited

Loading

rust-highfive commented Feb 13, 2022

This comment has been minimized.

camelid commented Feb 16, 2022

jhpratt commented Feb 16, 2022 •

edited

Loading

oli-obk left a comment

jhpratt left a comment

oli-obk commented Feb 17, 2022

jhpratt commented Feb 17, 2022

jhpratt commented Feb 18, 2022

oli-obk commented Feb 18, 2022

jhpratt commented Feb 22, 2022 •

edited

Loading

This comment has been minimized.

jhpratt commented Feb 27, 2022

oli-obk commented Feb 28, 2022

jhpratt commented Mar 1, 2022

oli-obk commented Mar 1, 2022

jhpratt commented Mar 1, 2022

oli-obk commented Mar 1, 2022 •

edited

Loading

bors commented Mar 4, 2022

rust-log-analyzer commented Mar 16, 2022

jhpratt commented Mar 16, 2022

JohnCSimon commented Apr 11, 2022

jhpratt commented Apr 11, 2022

jhpratt commented May 14, 2022

jhpratt commented May 17, 2022

Implement proper stability check for const impl Trait, fall back to unstable const when undeclared #93960

Implement proper stability check for const impl Trait, fall back to unstable const when undeclared #93960

Conversation

jhpratt commented Feb 13, 2022 • edited Loading

rust-highfive commented Feb 13, 2022

This comment has been minimized.

camelid commented Feb 16, 2022

jhpratt commented Feb 16, 2022 • edited Loading

oli-obk left a comment

Choose a reason for hiding this comment

jhpratt left a comment

Choose a reason for hiding this comment

oli-obk commented Feb 17, 2022

jhpratt commented Feb 17, 2022

jhpratt commented Feb 18, 2022

oli-obk commented Feb 18, 2022

jhpratt commented Feb 22, 2022 • edited Loading

This comment has been minimized.

jhpratt commented Feb 27, 2022

oli-obk commented Feb 28, 2022

jhpratt commented Mar 1, 2022

oli-obk commented Mar 1, 2022

jhpratt commented Mar 1, 2022

oli-obk commented Mar 1, 2022 • edited Loading

bors commented Mar 4, 2022

rust-log-analyzer commented Mar 16, 2022

jhpratt commented Mar 16, 2022

JohnCSimon commented Apr 11, 2022

jhpratt commented Apr 11, 2022

jhpratt commented May 14, 2022

jhpratt commented May 17, 2022

Implement proper stability check for `const impl Trait`, fall back to unstable `const` when undeclared #93960

Implement proper stability check for `const impl Trait`, fall back to unstable `const` when undeclared #93960

jhpratt commented Feb 13, 2022 •

edited

Loading

jhpratt commented Feb 16, 2022 •

edited

Loading

jhpratt commented Feb 22, 2022 •

edited

Loading

oli-obk commented Mar 1, 2022 •

edited

Loading