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

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

Closed
catamorphism opened this issue Apr 7, 2023 · 14 comments · Fixed by #111538
Closed
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@catamorphism
Copy link
Contributor

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?

@catamorphism catamorphism added the C-bug Category: This is a bug. label Apr 7, 2023
@catamorphism catamorphism changed the title Confusing error when building Rust with stable rustc Confusing diagnostics for unstable features when building Rust with stable rustc Apr 7, 2023
@workingjubilee
Copy link
Member

workingjubilee commented Apr 8, 2023

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
Copy link
Contributor Author

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
Copy link
Member

jyn514 commented Apr 8, 2023

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 jyn514 added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-diagnostics Area: Messages for errors, warnings, and lints T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed C-bug Category: This is a bug. labels Apr 8, 2023
@jyn514
Copy link
Member

jyn514 commented Apr 8, 2023

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
Copy link
Member

jyn514 commented Apr 8, 2023

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 jyn514 added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Apr 8, 2023
@jyn514
Copy link
Member

jyn514 commented Apr 8, 2023

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
Copy link
Contributor Author

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
Copy link
Member

est31 commented Apr 8, 2023

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
Copy link
Member

jyn514 commented Apr 8, 2023

@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 #110067 (comment).

@catamorphism
Copy link
Contributor Author

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
Copy link
Member

jyn514 commented Apr 11, 2023

@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
Copy link
Member

jyn514 commented Apr 11, 2023

@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
Copy link
Contributor Author

@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 9452402 .

@jyn514
Copy link
Member

jyn514 commented Apr 11, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants