rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.4k stars 12.59k forks source link

Confusing diagnostics for unstable features when building Rust with stable rustc #110067

Closed catamorphism closed 1 year ago

catamorphism commented 1 year ago

I'm trying to build Rust on a Tier 3 platform (riscv64-alpine-linux-musl; actually it's riscv64-unknown-musl that's listed in Tier 3, but I don't think that affects the problem). Since there are no snapshots for this platform, I installed rustc from the distro's package manager (apk) and created a config.toml file to force using the pre-installed rustc rather than downloading snapshots:

profile = "compiler"

[build]
rustc  = "/usr/bin/rustc"
cargo  = "/usr/bin/cargo"
rustfmt = "/usr/bin/rustfmt"
host   = ["riscv64-alpine-linux-musl"]
target = ["riscv64-alpine-linux-musl"]

[rust]
musl-root = "/usr"
debug     = true

[target.riscv64-alpine-linux-musl]
musl-libdir = "/usr/lib"

And the output of rustc --version -v:

# rustc --version -v
rustc 1.68.2 (9eb3afe9e 2023-03-27) (Alpine Linux)
binary: rustc
commit-hash: 9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0
commit-date: 2023-03-27
host: riscv64-alpine-linux-musl
release: 1.68.2
LLVM version: 15.0.7

I ran ./x.py build; when it got as far as building the core library, I got a series of errors that were very hard to make sense of until I realized that it's because core library code depends on unstable features and I was using a stable rustc to compile it:

building stage0 library artifacts (riscv64-alpine-linux-musl) 
  Downloaded adler v1.0.2
  Downloaded miniz_oxide v0.5.3
  Downloaded unicode-width v0.1.10
  Downloaded hashbrown v0.12.3
  Downloaded compiler_builtins v0.1.91
  Downloaded cc v1.0.77
  Downloaded addr2line v0.17.0
  Downloaded rustc-demangle v0.1.21
  Downloaded gimli v0.26.2
  Downloaded 9 crates (1.2 MB) in 1.45s
   Compiling cc v1.0.77
   Compiling compiler_builtins v0.1.91
   Compiling core v0.0.0 (/home/scratch/rust/library/core)
error: expected identifier, found keyword `move`
   --> library/core/src/ops/try_trait.rs:395:15
    |
395 |         const move |a, b| NeverShortCircuit(f(a, b))
    |               ^^^^ expected identifier, found keyword

error: expected one of `:`, `;`, or `=`, found `|`
   --> library/core/src/ops/try_trait.rs:395:20
    |
395 |         const move |a, b| NeverShortCircuit(f(a, b))
    |                    ^ expected one of `:`, `;`, or `=`

There were many more errors, but I think they all relate to unstable features.

It took me ages to figure out that (for example) the first error is due to the const_closures feature being disabled in stable.

I understand that the stable compiler won't allow using unstable features even with -Zunstable-options, but is it possible for the parser to recognize this code even so and reject it with something like "feature const_closures is not enabled" instead of the much more generic "expected identifier" error?

workingjubilee commented 1 year ago

Huh, usually Rust gets around this by setting RUSTC_BOOTSTRAP=1, and usually when we bootstrap it's with a beta compiler, which does need that set by x.py, and that would allow all the features. I wonder if RUSTC_BOOTSTRAP wasn't set?

catamorphism commented 1 year ago

Huh, usually Rust gets around this by setting RUSTC_BOOTSTRAP=1, and usually when we bootstrap it's with a beta compiler, which does need that set by x.py, and that would allow all the features. I wonder if RUSTC_BOOTSTRAP wasn't set?

It's set, but the command that's actually failing (it took some effort to figure out which executable was actually failing) is:

"/usr/bin/rustc" "--crate-name" "core" "--edition=2021" "library/core/src/lib.rs" "--error-format=json" "--json=diagnostic-rendered-ansi,artifacts,future-incompat" "--diagnostic-width=80" "--crate-type" "lib" "--emit=dep-info,metadata,link" "--cfg=bootstrap" "-Zunstable-options" "--check-cfg=values(bootstrap)"

And what's installed for me in /usr/bin/rustc is a stable compiler. I realized after the fact that I can't use it to bootstrap, but with a different error message I might have noticed that sooner.

jyn514 commented 1 year ago

Forwards compatibility in diagnostics seems extremely hard. I think a much simpler and more helpful fix is to check the output of rustc --version in bootstrap and give a hard error if it's not either the current version or N-1.

jyn514 commented 1 year ago

Note that the problem here is that you were trying to use 1.68 to build 1.70; it's unrelated to stable vs beta, the problem is that it was two versions behind instead of only one.

jyn514 commented 1 year ago

Mentoring instructions: in src/bootstrap/config.rs, if build.rustc is set, run that process with --version. Compare that output to the contents of src/version using the semver crate; if the minor versions aren't either the same or 1 apart, give a hard error.

Note that this should also disallow e.g. building 1.68 with 1.69; we use deny-warnings in most cases and have no guarantees that unstable features will be the same between releases. Only building 1.68 with 1.67 or 1.68 should be allowed.

jyn514 commented 1 year ago

btw @catamorphism I've noticed you running into a few different bootstrap bugs the past few days — reading through https://rustc-dev-guide.rust-lang.org/building/bootstrapping.html may or may not make it easier to diagnose these issues without having to spend ages interpreting the diagnostics. I'm also happy to chat about "how should this work" sorts of things on Zulip.

catamorphism commented 1 year ago

btw @catamorphism I've noticed you running into a few different bootstrap bugs the past few days — reading through https://rustc-dev-guide.rust-lang.org/building/bootstrapping.html may or may not make it easier to diagnose these issues without having to spend ages interpreting the diagnostics.

Yes, I've read that.

I'm also happy to chat about "how should this work" sorts of things on Zulip.

Thanks!

est31 commented 1 year ago

Note that the problem here is that you were trying to use 1.68 to build 1.70; it's unrelated to stable vs beta, the problem is that it was two versions behind instead of only one.

The error message could be improved though. Maybe one could make the bump-stage0 to put rust-version = "<version of stage0 beta>" into libcore/bootstrap's Cargo.toml?

jyn514 commented 1 year ago

@est31 I don't think rust-version is a good way to solve this, no - that won't give a warning or error if you use a future version to compile an older version. Also it won't be able to catch local-rebuild issues where the nightly version of the bootstrap compiler is too old. See my suggestion in https://github.com/rust-lang/rust/issues/110067#issuecomment-1500786014.

catamorphism commented 1 year ago

Forwards compatibility in diagnostics seems extremely hard. I think a much simpler and more helpful fix is to check the output of rustc --version in bootstrap and give a hard error if it's not either the current version or N-1.

Hmm, I'm afraid I might have said something unclear. I'm not asking for forward compatibility in diagnostics. The const_closures feature was added in version 1.68.0, according to https://github.com/rust-lang/rust/blob/master/compiler/rustc_feature/src/active.rs#L343 , and I was using rustc version 1.68.2. So it should "know" about that feature, but my understanding is that when I was invoking rustc it was with flags that prevent unstable features from being used.

jyn514 commented 1 year ago

@catamorphism hmm, in that case something strange is going on because playground gives a much more reasonable error than the one you saw: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=72c1630fcbccba1d2df75abbe61a54bf

error[[E0658]](https://doc.rust-lang.org/stable/error_codes/E0658.html): const closures are experimental
 --> src/lib.rs:2:13
  |
2 |     let _ = const move |a, b| NeverShortCircuit(f(a, b));
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: [see issue #106003 <https://github.com/rust-lang/rust/issues/106003>](https://github.com/rust-lang/rust/issues/106003) for more information
jyn514 commented 1 year ago

@catamorphism what commit of rustc were you trying to build when you got this error? I'm going to see if I can replicate it locally.

catamorphism commented 1 year ago

@catamorphism what commit of rustc were you trying to build when you got this error? I'm going to see if I can replicate it locally.

I was trying to build 94524020ea12f7947275063b65f8b7d705be073e .

jyn514 commented 1 year ago

Ah ok, it was related to the context of the const closure: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9c71507b2e0273d5062102e2cc0756e9

If you look at the same gist with beta, the error message is much better:

error[E0658]: const trait impls are experimental
 --> src/lib.rs:5:24
  |
5 | const fn foo() -> impl ~const FnMut(usize, usize) -> NeverShortCircuit {
  |                        ^^^^^^
  |
  = note: see issue #67792 <https://github.com/rust-lang/rust/issues/67792> for more information

error[E0658]: const closures are experimental
 --> src/lib.rs:6:5
  |
6 |     const move |a, b| NeverShortCircuit(f(a, b))
  |     ^^^^^
  |
  = note: see issue #106003 <https://github.com/rust-lang/rust/issues/106003> for more information

So I think this particular diagnostic issue has already been fixed. I still want to keep this issue open to track checking the version of the bootstrap compiler, though.