Hide implicit target features from diagnostics when possible

Author: calebzulawski

compiler/rustc_codegen_gcc/src/attributes.rs

@@ -75,7 +75,7 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
     let function_features = codegen_fn_attrs
         .target_features
         .iter()
-        .map(|features| features.as_str())
+        .map(|features| features.name.as_str())
         .collect::<Vec<&str>>();
 
     if let Some(features) = check_tied_features(

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -496,7 +496,7 @@ pub fn from_fn_attrs<'ll, 'tcx>(
     to_add.extend(tune_cpu_attr(cx));
 
     let function_features =
-        codegen_fn_attrs.target_features.iter().map(|f| f.as_str()).collect::<Vec<&str>>();
+        codegen_fn_attrs.target_features.iter().map(|f| f.name.as_str()).collect::<Vec<&str>>();
 
     if let Some(f) = llvm_util::check_tied_features(
         cx.tcx.sess,

compiler/rustc_codegen_ssa/src/target_features.rs

@@ -6,6 +6,7 @@ use rustc_errors::Applicability;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
 use rustc_middle::bug;
+use rustc_middle::middle::codegen_fn_attrs::TargetFeature;
 use rustc_middle::query::Providers;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::parse::feature_err;
@@ -18,7 +19,7 @@ pub fn from_target_feature(
     tcx: TyCtxt<'_>,
     attr: &ast::Attribute,
     supported_target_features: &UnordMap<String, Option<Symbol>>,
-    target_features: &mut Vec<Symbol>,
+    target_features: &mut Vec<TargetFeature>,
 ) {
     let Some(list) = attr.meta_item_list() else { return };
     let bad_item = |span| {
@@ -99,14 +100,27 @@ pub fn from_target_feature(
         }));
     }
 
-    // Add both explicit and implied target features, using a set to deduplicate
-    let mut target_features_set = UnordSet::new();
+    // Add explicit features
+    target_features.extend(
+        added_target_features.iter().copied().map(|name| TargetFeature { name, implied: false }),
+    );
+
+    // Add implied features
+    let mut implied_target_features = UnordSet::new();
     for feature in added_target_features.iter() {
-        target_features_set
+        implied_target_features
             .extend_unord(tcx.implied_target_features(*feature).clone().into_items());
     }
-    target_features_set.extend(added_target_features);
-    target_features.extend(target_features_set.into_sorted_stable_ord())
+    for feature in added_target_features.iter() {
+        implied_target_features.remove(feature);
+    }
+    target_features.extend(
+        implied_target_features
+            .into_sorted_stable_ord()
+            .iter()
+            .copied()
+            .map(|name| TargetFeature { name, implied: true }),
+    )
 }
 
 /// Computes the set of target features used in a function for the purposes of
@@ -115,7 +129,7 @@ fn asm_target_features(tcx: TyCtxt<'_>, did: DefId) -> &FxIndexSet<Symbol> {
     let mut target_features = tcx.sess.unstable_target_features.clone();
     if tcx.def_kind(did).has_codegen_attrs() {
         let attrs = tcx.codegen_fn_attrs(did);
-        target_features.extend(&attrs.target_features);
+        target_features.extend(attrs.target_features.iter().map(|feature| feature.name));
         match attrs.instruction_set {
             None => {}
             Some(InstructionSetAttr::ArmA32) => {

compiler/rustc_const_eval/src/interpret/call.rs

@@ -317,19 +317,26 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             && attrs
                 .target_features
                 .iter()
-                .any(|feature| !self.tcx.sess.target_features.contains(feature))
+                .any(|feature| !self.tcx.sess.target_features.contains(&feature.name))
         {
+            // Don't include implicit features in the error, unless only implicit features are
+            // missing. This should be rare, because it can only happen when an implicit feature
+            // is disabled, e.g. `+avx2,-avx`
+            let missing_explicit_features = attrs.target_features.iter().any(|feature| {
+                !feature.implied && !self.tcx.sess.target_features.contains(&feature.name)
+            });
             throw_ub_custom!(
                 fluent::const_eval_unavailable_target_features_for_fn,
                 unavailable_feats = attrs
                     .target_features
                     .iter()
-                    .filter(|&feature| !self.tcx.sess.target_features.contains(feature))
+                    .filter(|&feature| !(missing_explicit_features && feature.implied)
+                        && !self.tcx.sess.target_features.contains(&feature.name))
                     .fold(String::new(), |mut s, feature| {
                         if !s.is_empty() {
                             s.push_str(", ");
                         }
-                        s.push_str(feature.as_str());
+                        s.push_str(feature.name.as_str());
                         s
                     }),
             );

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

@@ -28,7 +28,7 @@ pub struct CodegenFnAttrs {
     pub link_ordinal: Option<u16>,
     /// The `#[target_feature(enable = "...")]` attribute and the enabled
     /// features (only enabled features are supported right now).
-    pub target_features: Vec<Symbol>,
+    pub target_features: Vec<TargetFeature>,
     /// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found.
     pub linkage: Option<Linkage>,
     /// The `#[linkage = "..."]` attribute on foreign items and the value we found.
@@ -51,6 +51,15 @@ pub struct CodegenFnAttrs {
     pub patchable_function_entry: Option<PatchableFunctionEntry>,
 }
 
+#[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
+pub struct TargetFeature {
+    /// The name of the target feature (e.g. "avx")
+    pub name: Symbol,
+    /// The feature is implied by another feature, rather than explicitly added by the
+    /// `#[target_feature]` attribute
+    pub implied: bool,
+}
+
 #[derive(Copy, Clone, Debug, TyEncodable, TyDecodable, HashStable)]
 pub struct PatchableFunctionEntry {
     /// Nops to prepend to the function

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -5,6 +5,7 @@ use std::ops::Bound;
 use rustc_errors::DiagArgValue;
 use rustc_hir::def::DefKind;
 use rustc_hir::{self as hir, BindingMode, ByRef, HirId, Mutability};
+use rustc_middle::middle::codegen_fn_attrs::TargetFeature;
 use rustc_middle::mir::BorrowKind;
 use rustc_middle::span_bug;
 use rustc_middle::thir::visit::Visitor;
@@ -31,7 +32,7 @@ struct UnsafetyVisitor<'a, 'tcx> {
     safety_context: SafetyContext,
     /// The `#[target_feature]` attributes of the body. Used for checking
     /// calls to functions with `#[target_feature]` (RFC 2396).
-    body_target_features: &'tcx [Symbol],
+    body_target_features: &'tcx [TargetFeature],
     /// When inside the LHS of an assignment to a field, this is the type
     /// of the LHS and the span of the assignment expression.
     assignment_info: Option<Ty<'tcx>>,
@@ -442,14 +443,29 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                     // is_like_wasm check in hir_analysis/src/collect.rs
                     let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features;
                     if !self.tcx.sess.target.options.is_like_wasm
-                        && !callee_features
-                            .iter()
-                            .all(|feature| self.body_target_features.contains(feature))
+                        && !callee_features.iter().all(|feature| {
+                            self.body_target_features.iter().any(|f| f.name == feature.name)
+                        })
                     {
+                        // Don't include implicit features in the error, unless only implicit
+                        // features are missing.
+                        let missing_explicit_features = callee_features.iter().any(|feature| {
+                            !feature.implied
+                                && !self.body_target_features.iter().any(|body_feature| {
+                                    !feature.implied && body_feature.name == feature.name
+                                })
+                        });
                         let missing: Vec<_> = callee_features
                             .iter()
                             .copied()
-                            .filter(|feature| !self.body_target_features.contains(feature))
+                            .filter(|feature| {
+                                !(missing_explicit_features && feature.implied)
+                                    && !self
+                                        .body_target_features
+                                        .iter()
+                                        .any(|body_feature| body_feature.name == feature.name)
+                            })
+                            .map(|feature| feature.name)
                             .collect();
                         let build_enabled = self
                             .tcx

compiler/rustc_mir_transform/src/inline.rs

@@ -479,7 +479,9 @@ impl<'tcx> Inliner<'tcx> {
             return Err("incompatible instruction set");
         }
 
-        if callee_attrs.target_features != self.codegen_fn_attrs.target_features {
+        let callee_feature_names = callee_attrs.target_features.iter().map(|f| f.name);
+        let this_feature_names = self.codegen_fn_attrs.target_features.iter().map(|f| f.name);
+        if callee_feature_names.ne(this_feature_names) {
             // In general it is not correct to inline a callee with target features that are a
             // subset of the caller. This is because the callee might contain calls, and the ABI of
             // those calls depends on the target features of the surrounding function. By moving a

src/tools/miri/tests/fail/function_calls/simd_feature_flag_difference.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: calling a function that requires unavailable target features: avx, sse3, sse4.1, sse4.2, ssse3
+error: Undefined Behavior: calling a function that requires unavailable target features: avx
   --> $DIR/simd_feature_flag_difference.rs:LL:CC
    |
 LL |     unsafe { foo(0.0, x) }
-   |              ^^^^^^^^^^^ calling a function that requires unavailable target features: avx, sse3, sse4.1, sse4.2, ssse3
+   |              ^^^^^^^^^^^ calling a function that requires unavailable target features: avx
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

src/tools/miri/tests/fail/function_calls/target_feature.rs

@@ -4,7 +4,7 @@
 fn main() {
     assert!(!is_x86_feature_detected!("ssse3"));
     unsafe {
-        ssse3_fn(); //~ ERROR: calling a function that requires unavailable target features: sse3, ssse3
+        ssse3_fn(); //~ ERROR: calling a function that requires unavailable target features: ssse3
     }
 }
 

src/tools/miri/tests/fail/function_calls/target_feature.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: calling a function that requires unavailable target features: sse3, ssse3
+error: Undefined Behavior: calling a function that requires unavailable target features: ssse3
   --> $DIR/target_feature.rs:LL:CC
    |
 LL |         ssse3_fn();
-   |         ^^^^^^^^^^ calling a function that requires unavailable target features: sse3, ssse3
+   |         ^^^^^^^^^^ calling a function that requires unavailable target features: ssse3
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information