rustdoc: when merging target features, keep the highest stability #137632

RalfJung · 2025-02-25T19:31:07Z

This addresses #137366. (Not closing since we might consider a backport.)

rustdoc wants to pretend that it runs for all targets at once and has all target features, so tcx.rust_target_features() will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden #[target_feature] attributes was not aware of that.

This PR makes the tcx.rust_target_features() info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with #137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.

rustbot · 2025-02-25T19:31:11Z

r? @workingjubilee

rustbot has assigned @workingjubilee.
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

RalfJung · 2025-02-25T19:32:20Z

Cc @rust-lang/rustdoc

…in rustdoc

RalfJung · 2025-02-25T19:42:22Z

I haven't added a test as I am not sure how to build one... is it even possible to have a no_core rustdoc tests where we could set --target to an ARM target to check this?

workingjubilee · 2025-02-26T07:08:47Z

😵‍💫 does rustdoc even respond to things like #![no_core]?

workingjubilee · 2025-02-26T07:09:17Z

I suppose it must or core wouldn't work but I'm not feeling entirely sure right now...

RalfJung · 2025-02-28T15:10:08Z

Given this is fixing a stable-to-stable regression I feel it's okay to land this without a test... though if a rustdoc person has an idea for how to test this that would of course be great. :)

GuillaumeGomez · 2025-02-28T15:19:07Z

I'm not even sure to understand what needs to be checked in rustdoc. From reading the issue, what I see is that it failed compilation. What do you want to check in rustdoc?

RalfJung · 2025-02-28T15:25:13Z

It fails compilation only in rustdoc and only on ARM-32 targets. So we'd need a test that checks that some code builds fine in rustdoc on ARM-32. Ideally this test can be run on all hosts, which we usually do with #![no_core] tests that don't need a sysroot and thus can easily be cross-compiled, so we can just set --target <whatever>.

GuillaumeGomez · 2025-02-28T15:30:17Z

Do we have an equivalent test for rustc? If so you can write the same one for rustdoc.

RalfJung · 2025-02-28T15:41:43Z

The rustc test would be something like

//@ compile-flags: --target armv7-unknown-linux-gnueabihf

#![crate_type = "lib"]
#![feature(no_core, lang_items)]
#![no_core]

#[lang = "sized"]
pub trait Sized {}

#[target_feature(enable = "neon,v7,aes")]
pub fn fun() {}

So do I just dump such a file into tests/rustdoc?

GuillaumeGomez · 2025-02-28T15:43:22Z

In tests/rustdoc-ui with //@ check-pass but otherwise yes. :)

RalfJung · 2025-02-28T15:47:01Z

Hm that does not seem to do the right thing, the test passes even without my PR.

RalfJung · 2025-02-28T15:49:28Z

Okay strange, of the two examples given in #137366, only one reproduces this way. 🤷 I guess one confirmed test is enough. ;)

RalfJung · 2025-02-28T16:16:11Z

Ah, right, the neon target feature is no longer "forbidden", we handle that via the ABI instead and that... I have no idea what that does under rustdoc, probably something nonsensical, but at least it does not error. ;)

The last commit disables this check in rustdoc. It also adds a test.

workingjubilee · 2025-02-28T23:05:38Z

compiler/rustc_codegen_ssa/src/target_features.rs

+                    if !tcx.sess.opts.actually_rustdoc {
+                        if abi_feature_constraints.incompatible.contains(&name.as_str()) {


isn't this just &&?

name is a Symbol. So &&name does not work, no.

I was commenting on both lines because I meant that this is && the control flow operator:

if x { if y { }; }; if x && y { };

Oh, that's what you meant.^^

Yeah you are right.

Now it's already rolled up so I'll rather not push to this PR.

workingjubilee · 2025-02-28T23:07:49Z

tests/rustdoc-ui/target-feature-stability.rs

@@ -0,0 +1,28 @@
+//! This is a regression test for <https://github.com/rust-lang/rust/issues/137366>, ensuring
+//! that we can use the `neon` target feature on ARM-32 targets in rustdoc despite there


Suggested change

//! that we can use the `neon` target feature on ARM-32 targets in rustdoc despite there

//! that we can use the `neon` target feature on ARM32 targets in rustdoc despite there

workingjubilee

r=me with nits addressed

RalfJung · 2025-03-01T14:14:45Z

Nominating for backport as #137366 is a stable-to-stable regression.

…r=workingjubilee rustdoc: when merging target features, keep the highest stability This addresses rust-lang#137366. (Not closing since we might consider a backport.) rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that. This PR makes the `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#136865 (Perform deeper compiletest path normalization for `$TEST_BUILD_DIR` to account for compare-mode/debugger cases, and normalize long type file filename hashes) - rust-lang#136922 (Pattern types: Avoid having to handle an Option for range ends in the type system or the HIR) - rust-lang#137081 (change config.toml to bootstrap.toml) - rust-lang#137103 ({json|html}docck: catch and error on deprecated syntax) - rust-lang#137632 (rustdoc: when merging target features, keep the highest stability) - rust-lang#137684 (Add rustdoc support for `--emit=dep-info[=path]`) - rust-lang#137794 (make qnx pass a test) - rust-lang#137801 (tests: Unignore target modifier tests on all platforms) - rust-lang#137826 (test(codegen): add looping_over_ne_bytes test for rust-lang#133528) Failed merges: - rust-lang#137147 (Add exclude to config.toml) r? `@ghost` `@rustbot` modify labels: rollup

…r=workingjubilee rustdoc: when merging target features, keep the highest stability This addresses rust-lang#137366. (Not closing since we might consider a backport.) rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that. This PR makes the `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#136865 (Perform deeper compiletest path normalization for `$TEST_BUILD_DIR` to account for compare-mode/debugger cases, and normalize long type file filename hashes) - rust-lang#137081 (change config.toml to bootstrap.toml) - rust-lang#137103 ({json|html}docck: catch and error on deprecated syntax) - rust-lang#137632 (rustdoc: when merging target features, keep the highest stability) - rust-lang#137684 (Add rustdoc support for `--emit=dep-info[=path]`) - rust-lang#137794 (make qnx pass a test) - rust-lang#137801 (tests: Unignore target modifier tests on all platforms) - rust-lang#137826 (test(codegen): add looping_over_ne_bytes test for rust-lang#133528) r? `@ghost` `@rustbot` modify labels: rollup

…r=workingjubilee rustdoc: when merging target features, keep the highest stability This addresses rust-lang#137366. (Not closing since we might consider a backport.) rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that. This PR makes the `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#136865 (Perform deeper compiletest path normalization for `$TEST_BUILD_DIR` to account for compare-mode/debugger cases, and normalize long type file filename hashes) - rust-lang#137103 ({json|html}docck: catch and error on deprecated syntax) - rust-lang#137632 (rustdoc: when merging target features, keep the highest stability) - rust-lang#137684 (Add rustdoc support for `--emit=dep-info[=path]`) - rust-lang#137794 (make qnx pass a test) - rust-lang#137801 (tests: Unignore target modifier tests on all platforms) - rust-lang#137826 (test(codegen): add looping_over_ne_bytes test for rust-lang#133528) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#137103 ({json|html}docck: catch and error on deprecated syntax) - rust-lang#137632 (rustdoc: when merging target features, keep the highest stability) - rust-lang#137684 (Add rustdoc support for `--emit=dep-info[=path]`) - rust-lang#137794 (make qnx pass a test) - rust-lang#137801 (tests: Unignore target modifier tests on all platforms) - rust-lang#137826 (test(codegen): add looping_over_ne_bytes test for rust-lang#133528) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#137632 - RalfJung:rustdoc-target-features, r=workingjubilee rustdoc: when merging target features, keep the highest stability This addresses rust-lang#137366. (Not closing since we might consider a backport.) rustdoc wants to pretend that it runs for all targets at once and has all target features, so `tcx.rust_target_features()` will actually be all the target features. For target features that exist on multiple targets, the stability info for one of the targets will be picked (first or last in the list, I guess). All the code consuming that query has to be aware that the data is basically nonsense when running in rustdoc, but the logic checking for unstable or forbidden `#[target_feature]` attributes was not aware of that. This PR makes the `tcx.rust_target_features()` info in rustdoc slightly less nonsensical (and decidedly less random) by having the "most stable" target feature take precedent. That deals with rust-lang#137366 (a conflict between a stable and a "forbidden" target feature of the same name for different targets), and also deals with the situation (that we did not seem to have yet) of a conflict between a stable and an unstable target feature of the same name. Note that if there are two unstable target features of the same name, rustdoc might still require the "wrong" nightly feature to be enabled -- but this can only possibly affect unstable code so I guess we can wait until that actually happens, and then someone will have to rewrite this entire thing to be less hacky.

backport: rust-lang/rust#137632 to fix cross compilation of docs

apiraino · 2025-03-06T15:41:34Z

hey @rust-lang/rustdoc Are you interested (or neutral or against) in having this patch backported to beta channel. It fixes a regression on stable.

jieyouxu · 2025-03-09T02:58:39Z

(t-rustdoc backport nomination thread is #t-rustdoc > beta-nominated: #137632)

rustdoc: disable forbidden #[target_feature] check

b6f2240

rustbot assigned workingjubilee Feb 25, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 25, 2025

also fix potential issues with mixed stable/unstable target features …

039af88

…in rustdoc

RalfJung mentioned this pull request Feb 25, 2025

rustdoc fails when building rust 1.85.0 for ARM targets #137366

Open

This comment has been minimized.

Sign in to view

add test

dc04c0c

RalfJung force-pushed the rustdoc-target-features branch from b072867 to eff59b8 Compare February 28, 2025 16:02

This comment has been minimized.

Sign in to view

RalfJung force-pushed the rustdoc-target-features branch from eff59b8 to 6b63c57 Compare February 28, 2025 16:15

This comment has been minimized.

Sign in to view

RalfJung force-pushed the rustdoc-target-features branch from 6b63c57 to 63088e3 Compare February 28, 2025 16:31

workingjubilee reviewed Feb 28, 2025

View reviewed changes

workingjubilee approved these changes Feb 28, 2025

View reviewed changes

RalfJung added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 1, 2025

RalfJung 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 Mar 1, 2025

matthiaskrgr mentioned this pull request Mar 1, 2025

Rollup of 9 pull requests #137856

Closed

matthiaskrgr mentioned this pull request Mar 2, 2025

Rollup of 8 pull requests #137884

Closed

matthiaskrgr mentioned this pull request Mar 2, 2025

Rollup of 7 pull requests #137903

Closed

matthiaskrgr mentioned this pull request Mar 3, 2025

Rollup of 6 pull requests #137914

Merged

bors merged commit 962e492 into rust-lang:master Mar 3, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 3, 2025

TomJo2000 added a commit to TomJo2000/termux-packages-prs that referenced this pull request Mar 5, 2025

bump(main/rust): 1.85.0

ab9c5ef

backport: rust-lang/rust#137632 to fix cross compilation of docs

TomJo2000 mentioned this pull request Mar 5, 2025

bump(main/rust): 1.85.0 termux/termux-packages#23606

Closed

RalfJung deleted the rustdoc-target-features branch March 6, 2025 17:36

jieyouxu added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. beta-nominated Nominated for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Mar 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: when merging target features, keep the highest stability #137632

rustdoc: when merging target features, keep the highest stability #137632

RalfJung commented Feb 25, 2025 •

edited

Loading

rustbot commented Feb 25, 2025

RalfJung commented Feb 25, 2025

RalfJung commented Feb 25, 2025

workingjubilee commented Feb 26, 2025

workingjubilee commented Feb 26, 2025

RalfJung commented Feb 28, 2025

GuillaumeGomez commented Feb 28, 2025

RalfJung commented Feb 28, 2025 •

edited

Loading

GuillaumeGomez commented Feb 28, 2025

RalfJung commented Feb 28, 2025 •

edited

Loading

GuillaumeGomez commented Feb 28, 2025

This comment has been minimized.

RalfJung commented Feb 28, 2025

RalfJung commented Feb 28, 2025

This comment has been minimized.

RalfJung commented Feb 28, 2025

This comment has been minimized.

workingjubilee Feb 28, 2025

RalfJung Mar 1, 2025 •

edited

Loading

workingjubilee Mar 3, 2025

RalfJung Mar 3, 2025

RalfJung Mar 3, 2025

workingjubilee Feb 28, 2025

workingjubilee left a comment •

edited

Loading

RalfJung commented Mar 1, 2025

apiraino commented Mar 6, 2025 •

edited

Loading

jieyouxu commented Mar 9, 2025

		if !tcx.sess.opts.actually_rustdoc {
		if abi_feature_constraints.incompatible.contains(&name.as_str()) {

		@@ -0,0 +1,28 @@
		//! This is a regression test for <https://github.com/rust-lang/rust/issues/137366>, ensuring
		//! that we can use the `neon` target feature on ARM-32 targets in rustdoc despite there

rustdoc: when merging target features, keep the highest stability #137632

rustdoc: when merging target features, keep the highest stability #137632

Conversation

RalfJung commented Feb 25, 2025 • edited Loading

rustbot commented Feb 25, 2025

RalfJung commented Feb 25, 2025

RalfJung commented Feb 25, 2025

workingjubilee commented Feb 26, 2025

workingjubilee commented Feb 26, 2025

RalfJung commented Feb 28, 2025

GuillaumeGomez commented Feb 28, 2025

RalfJung commented Feb 28, 2025 • edited Loading

GuillaumeGomez commented Feb 28, 2025

RalfJung commented Feb 28, 2025 • edited Loading

GuillaumeGomez commented Feb 28, 2025

This comment has been minimized.

RalfJung commented Feb 28, 2025

RalfJung commented Feb 28, 2025

This comment has been minimized.

RalfJung commented Feb 28, 2025

This comment has been minimized.

workingjubilee Feb 28, 2025

Choose a reason for hiding this comment

RalfJung Mar 1, 2025 • edited Loading

Choose a reason for hiding this comment

workingjubilee Mar 3, 2025

Choose a reason for hiding this comment

RalfJung Mar 3, 2025

Choose a reason for hiding this comment

RalfJung Mar 3, 2025

Choose a reason for hiding this comment

workingjubilee Feb 28, 2025

Choose a reason for hiding this comment

workingjubilee left a comment • edited Loading

Choose a reason for hiding this comment

RalfJung commented Mar 1, 2025

apiraino commented Mar 6, 2025 • edited Loading

jieyouxu commented Mar 9, 2025

RalfJung commented Feb 25, 2025 •

edited

Loading

RalfJung commented Feb 28, 2025 •

edited

Loading

RalfJung commented Feb 28, 2025 •

edited

Loading

RalfJung Mar 1, 2025 •

edited

Loading

workingjubilee left a comment •

edited

Loading

apiraino commented Mar 6, 2025 •

edited

Loading