bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends` by GrigorenkoPV · Pull Request #142212 · rust-lang/rust

GrigorenkoPV · 2025-06-08T20:13:51Z

rustbot · 2025-06-08T20:13:55Z

rustbot has assigned @albertlarsan68.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-06-08T20:13:58Z

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

workingjubilee · 2025-06-09T04:14:51Z

src/bootstrap/src/core/config/toml/rust.rs

+        if !VALID_CODEGEN_BACKENDS.contains(&&**backend) {
+            let section = section();
+            panic!(
+                "Invalid value '{backend}' for '{section}.codegen-backends'. Permitted values: {VALID_CODEGEN_BACKENDS:?}."


mh, I remember a PR deliberately introduce the fallthrough to the previous else if this condition fails to obtain? it was about using an out-of-tree codegen backend, I believe.

cc @FractalFir @WaffleLapkin

I suspect the intent was to panic for codegen backend names starting with rustc_codegen_, while only warning for unknown backends. As written before this PR however it would only panic/warn when the name starts with rustc_codegen_ and don't check the name otherwise however.

I am not very familiar with this part of the compiler, but, to be honest, I don't see why we should panic on an unknown backend. If I am understanding things correctly, then this will warn people about unknown codegen backends anyway.

My backend in particular does not strictly need this to function(I can override the backend using RUSTFLAGS), but I still think a warning may be better than an outright error here.

The panic is for when we know for sure that the backend is specified wrong. Backends should never be specified with the rustc_codegen_ prefix. That is already implicitly added, so you would end up with a path like compiler/rustc_codegen_rustc_codegen_foo if you specified rustc_codegen_foo. Do note that we already panic on other invalid config options.

I agree with everything above, I think. Panic on rustc_codegen_ prefix makes sense, but checking for known values (llvm/cranelift/gcc) is annoying for out-of-tree backends like my employer's.

GrigorenkoPV · 2025-07-01T21:52:31Z

r? bootstrap

Kobzol

Left two nits, otherwise LGTM.

src/bootstrap/src/core/config/toml/rust.rs

…n-backends`

Kobzol · 2025-07-02T15:24:26Z

Thanks!

@bors r+ rollup

bors · 2025-07-02T15:24:28Z

📌 Commit 2ab641d has been approved by Kobzol

It is now in the queue for this repository.

Rollup of 11 pull requests Successful merges: - #131923 (Derive `Copy` and `Hash` for `IntErrorKind`) - #138340 (Remove some unsized tuple impls now that we don't support unsizing tuples anymore) - #141219 (Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global`) - #142212 (bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends`) - #142237 (Detect more cases of unused_parens around types) - #142964 (Attribute rework: a parser for single attributes without arguments) - #143070 (Rewrite `macro_rules!` parser to not use the MBE engine itself) - #143235 (Assemble const bounds via normal item bounds in old solver too) - #143261 (Feed `explicit_predicates_of` instead of `predicates_of`) - #143276 (loop match: handle opaque patterns) - #143306 (Add `track_caller` attributes to trace origen of Clippy lints) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-apple try-job: x86_64-msvc-1 try-job: x86_64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: test-various

Rollup merge of #142212 - GrigorenkoPV:codegens, r=Kobzol bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends` As per #142184 (comment). Closes #142184.

…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#131923 (Derive `Copy` and `Hash` for `IntErrorKind`) - rust-lang#138340 (Remove some unsized tuple impls now that we don't support unsizing tuples anymore) - rust-lang#141219 (Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global`) - rust-lang#142212 (bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends`) - rust-lang#142237 (Detect more cases of unused_parens around types) - rust-lang#142964 (Attribute rework: a parser for single attributes without arguments) - rust-lang#143070 (Rewrite `macro_rules!` parser to not use the MBE engine itself) - rust-lang#143235 (Assemble const bounds via normal item bounds in old solver too) - rust-lang#143261 (Feed `explicit_predicates_of` instead of `predicates_of`) - rust-lang#143276 (loop match: handle opaque patterns) - rust-lang#143306 (Add `track_caller` attributes to trace origen of Clippy lints) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-apple try-job: x86_64-msvc-1 try-job: x86_64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: test-various

rustbot assigned albertlarsan68 Jun 8, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 8, 2025

workingjubilee reviewed Jun 9, 2025

View reviewed changes

workingjubilee changed the title ~~bootstrap: validate rust.codegen-backends & targer.<triple>.codegen-backends~~ bootstrap: validate rust.codegen-backends & target.<triple>.codegen-backends Jun 9, 2025

GrigorenkoPV force-pushed the codegens branch from 41aa521 to 39e619c Compare June 26, 2025 22:50

rustbot assigned Kobzol and unassigned albertlarsan68 Jul 1, 2025

Kobzol reviewed Jul 2, 2025

View reviewed changes

src/bootstrap/src/core/config/toml/rust.rs Outdated Show resolved Hide resolved

src/bootstrap/src/core/config/toml/rust.rs Outdated Show resolved Hide resolved

bootstrap: validate rust.codegen-backends & `targer.<triple>.codege…

2ab641d

…n-backends`

GrigorenkoPV force-pushed the codegens branch from 39e619c to 2ab641d Compare July 2, 2025 13:57

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 2, 2025

matthiaskrgr mentioned this pull request Jul 2, 2025

Rollup of 11 pull requests #143338

Merged

bors merged commit d50240b into rust-lang:master Jul 3, 2025
10 checks passed

rustbot added this to the 1.90.0 milestone Jul 3, 2025

rust-timer added a commit that referenced this pull request Jul 3, 2025

Unrolled build for #142212

7135352

Rollup merge of #142212 - GrigorenkoPV:codegens, r=Kobzol bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends` As per #142184 (comment). Closes #142184.

GrigorenkoPV deleted the codegens branch July 3, 2025 13:26

Uh oh!

Comments

Conversation

GrigorenkoPV commented Jun 8, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 8, 2025

Uh oh!

rustbot commented Jun 8, 2025

Uh oh!

workingjubilee Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

bjorn3 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

FractalFir Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

bjorn3 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

WaffleLapkin Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

GrigorenkoPV commented Jul 1, 2025

Uh oh!

Kobzol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Kobzol commented Jul 2, 2025

Uh oh!

bors commented Jul 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!

GrigorenkoPV commented Jun 8, 2025 •

edited by rustbot

Loading