From b6f22400002f7921feed13e35852e3041cf2b145 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 25 Feb 2025 20:26:12 +0100 Subject: [PATCH 1/4] rustdoc: disable forbidden #[target_feature] check --- compiler/rustc_codegen_ssa/src/target_features.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index d8b9bdb55da69..28c6932dd5b9c 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -87,7 +87,10 @@ pub(crate) fn from_target_feature_attr( // But ensure the ABI does not forbid enabling this. // Here we do assume that LLVM doesn't add even more implied features // we don't know about, at least no features that would have ABI effects! - if abi_feature_constraints.incompatible.contains(&name.as_str()) { + // We skip this check in rustdoc, like we skip all target feature related checks. + if !tcx.sess.opts.actually_rustdoc + && abi_feature_constraints.incompatible.contains(&name.as_str()) + { tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr { span: item.span(), feature: name.as_str(), @@ -142,8 +145,11 @@ pub(crate) fn provide(providers: &mut Providers) { rust_target_features: |tcx, cnum| { assert_eq!(cnum, LOCAL_CRATE); if tcx.sess.opts.actually_rustdoc { - // rustdoc needs to be able to document functions that use all the features, so - // whitelist them all + // HACK: rustdoc would like to pretend that we have all the target features, so we + // have to merge all the lists into one. The result has a "random" stability + // (depending on the order in which we consider features); all places that check + // target stability are expected to check `actually_rustdoc` and do nothing when + // that is set. rustc_target::target_features::all_rust_features() .map(|(a, b)| (a.to_string(), b)) .collect() From 039af88e09f4f4beb47406f4771bffc2e61d800a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 25 Feb 2025 20:38:13 +0100 Subject: [PATCH 2/4] also fix potential issues with mixed stable/unstable target features in rustdoc --- .../rustc_codegen_ssa/src/target_features.rs | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index 28c6932dd5b9c..a63e1877e4533 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -10,7 +10,7 @@ use rustc_middle::query::Providers; use rustc_middle::ty::TyCtxt; use rustc_session::parse::feature_err; use rustc_span::{Span, Symbol, sym}; -use rustc_target::target_features; +use rustc_target::target_features::{self, Stability}; use crate::errors; @@ -87,10 +87,7 @@ pub(crate) fn from_target_feature_attr( // But ensure the ABI does not forbid enabling this. // Here we do assume that LLVM doesn't add even more implied features // we don't know about, at least no features that would have ABI effects! - // We skip this check in rustdoc, like we skip all target feature related checks. - if !tcx.sess.opts.actually_rustdoc - && abi_feature_constraints.incompatible.contains(&name.as_str()) - { + if abi_feature_constraints.incompatible.contains(&name.as_str()) { tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr { span: item.span(), feature: name.as_str(), @@ -146,13 +143,37 @@ pub(crate) fn provide(providers: &mut Providers) { assert_eq!(cnum, LOCAL_CRATE); if tcx.sess.opts.actually_rustdoc { // HACK: rustdoc would like to pretend that we have all the target features, so we - // have to merge all the lists into one. The result has a "random" stability - // (depending on the order in which we consider features); all places that check - // target stability are expected to check `actually_rustdoc` and do nothing when - // that is set. - rustc_target::target_features::all_rust_features() - .map(|(a, b)| (a.to_string(), b)) - .collect() + // have to merge all the lists into one. To ensure an unstable target never prevents + // a stable one from working, we merge the stability info of all instances of the + // same target feature name, with the "most stable" taking precedence. And then we + // hope that this doesn't cause issues anywhere else in the compiler... + let mut result: UnordMap = Default::default(); + for (name, stability) in rustc_target::target_features::all_rust_features() { + use std::collections::hash_map::Entry; + match result.entry(name.to_owned()) { + Entry::Vacant(vacant_entry) => { + vacant_entry.insert(stability); + } + Entry::Occupied(mut occupied_entry) => { + // Merge the two stabilities, "more stable" taking precedence. + match (occupied_entry.get(), stability) { + (Stability::Stable, _) + | ( + Stability::Unstable { .. }, + Stability::Unstable { .. } | Stability::Forbidden { .. }, + ) + | (Stability::Forbidden { .. }, Stability::Forbidden { .. }) => { + // The stability in the entry is at least as good as the new one, just keep it. + } + _ => { + // Overwrite stabilite. + occupied_entry.insert(stability); + } + } + } + } + } + result } else { tcx.sess .target From dc04c0ca48c7285d74a0489354ed7d013dc25799 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 28 Feb 2025 16:51:53 +0100 Subject: [PATCH 3/4] add test --- tests/rustdoc-ui/target-feature-stability.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 tests/rustdoc-ui/target-feature-stability.rs diff --git a/tests/rustdoc-ui/target-feature-stability.rs b/tests/rustdoc-ui/target-feature-stability.rs new file mode 100644 index 0000000000000..4ade9690310e3 --- /dev/null +++ b/tests/rustdoc-ui/target-feature-stability.rs @@ -0,0 +1,18 @@ +//! This is a regression test for , ensuring +//! that we can use the `neon` target feature on ARM-32 targets in rustdoc despite there +//! being a "forbidden" feature of the same name for aarch64, and rustdoc merging the +//! target features of all targets. +//@ check-pass +//@ compile-flags: --target armv7-unknown-linux-gnueabihf + +#![crate_type = "lib"] +#![feature(no_core, lang_items)] +#![feature(arm_target_feature)] +#![no_core] + +#[lang = "sized"] +pub trait Sized {} + +// `fp-armv8` is "forbidden" on aarch64 as we tie it to `neon`. +#[target_feature(enable = "fp-armv8")] +pub fn fun() {} From 4c939db0e775df21a0b409b7603eaaf0056e8f86 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 28 Feb 2025 16:56:36 +0100 Subject: [PATCH 4/4] also skip abi_required_features check in rustdoc --- .../rustc_codegen_ssa/src/target_features.rs | 17 +++++++++++------ tests/rustdoc-ui/target-feature-stability.rs | 16 +++++++++++++--- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index a63e1877e4533..95a5e96fe46e4 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -87,12 +87,17 @@ pub(crate) fn from_target_feature_attr( // But ensure the ABI does not forbid enabling this. // Here we do assume that LLVM doesn't add even more implied features // we don't know about, at least no features that would have ABI effects! - if abi_feature_constraints.incompatible.contains(&name.as_str()) { - tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr { - span: item.span(), - feature: name.as_str(), - reason: "this feature is incompatible with the target ABI", - }); + // We skip this logic in rustdoc, where we want to allow all target features of + // all targets, so we can't check their ABI compatibility and anyway we are not + // generating code so "it's fine". + if !tcx.sess.opts.actually_rustdoc { + if abi_feature_constraints.incompatible.contains(&name.as_str()) { + tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr { + span: item.span(), + feature: name.as_str(), + reason: "this feature is incompatible with the target ABI", + }); + } } target_features.push(TargetFeature { name, implied: name != feature_sym }) } diff --git a/tests/rustdoc-ui/target-feature-stability.rs b/tests/rustdoc-ui/target-feature-stability.rs index 4ade9690310e3..17fa3ccfe3e89 100644 --- a/tests/rustdoc-ui/target-feature-stability.rs +++ b/tests/rustdoc-ui/target-feature-stability.rs @@ -1,9 +1,13 @@ //! This is a regression test for , ensuring -//! 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 //! being a "forbidden" feature of the same name for aarch64, and rustdoc merging the //! target features of all targets. //@ check-pass -//@ compile-flags: --target armv7-unknown-linux-gnueabihf +//@ revisions: arm aarch64 +//@[arm] compile-flags: --target armv7-unknown-linux-gnueabihf +//@[arm] needs-llvm-components: arm +//@[aarch64] compile-flags: --target aarch64-unknown-none-softfloat +//@[aarch64] needs-llvm-components: aarch64 #![crate_type = "lib"] #![feature(no_core, lang_items)] @@ -15,4 +19,10 @@ pub trait Sized {} // `fp-armv8` is "forbidden" on aarch64 as we tie it to `neon`. #[target_feature(enable = "fp-armv8")] -pub fn fun() {} +pub fn fun1() {} + +// This would usually be rejected as it changes the ABI. +// But we disable that check in rustdoc since we are building "for all targets" and the +// check can't really handle that. +#[target_feature(enable = "soft-float")] +pub fn fun2() {}