unify two -Ctarget-feature parsers

This does change the logic a bit: previously, we didn't forward reverse
implications of negated features to the backend, instead relying on the backend
to handle the implication itself.

Author: RalfJung

compiler/rustc_middle/src/target_features.rs

@@ -145,6 +145,54 @@ pub fn check_target_feature_trait_unsafe(tcx: TyCtxt<'_>, id: LocalDefId, attr_s
     }
 }
 
+/// Parse the value of `-Ctarget-feature`, also expanding implied features,
+/// and call the closure for each (expanded) Rust feature. If the list contains
+/// a syntactically invalid item (not starting with `+`/`-`), the error callback is invoked.
+fn parse_rust_feature_flag<'a>(
+    sess: &Session,
+    target_feature_flag: &'a str,
+    err_callback: impl Fn(&'a str),
+    mut callback: impl FnMut(FxHashSet<&'a str>, /* enable */ bool),
+) {
+    // A cache for the backwards implication map.
+    let mut inverse_implied_features: Option<FxHashMap<&str, FxHashSet<&str>>> = None;
+
+    for feature in target_feature_flag.split(',') {
+        if let Some(base_feature) = feature.strip_prefix('+') {
+            callback(sess.target.implied_target_features(base_feature), true)
+        } else if let Some(base_feature) = feature.strip_prefix('-') {
+            // If `f1` implies `f2`, then `!f2` implies `!f1` -- this is standard logical contraposition.
+            // So we have to find all the reverse implications of `base_feature` and disable them, too.
+
+            let inverse_implied_features = inverse_implied_features.get_or_insert_with(|| {
+                let mut set: FxHashMap<&str, FxHashSet<&str>> = FxHashMap::default();
+                for (f, _, is) in sess.target.rust_target_features() {
+                    for i in is.iter() {
+                        set.entry(i).or_default().insert(f);
+                    }
+                }
+                set
+            });
+
+            // Inverse mplied target features have their own inverse implied target features, so we
+            // traverse the map until there are no more features to add.
+            let mut features = FxHashSet::default();
+            let mut new_features = vec![base_feature];
+            while let Some(new_feature) = new_features.pop() {
+                if features.insert(new_feature) {
+                    if let Some(implied_features) = inverse_implied_features.get(&new_feature) {
+                        new_features.extend(implied_features)
+                    }
+                }
+            }
+
+            callback(features, false)
+        } else if !feature.is_empty() {
+            err_callback(feature)
+        }
+    }
+}
+
 /// Utility function for a codegen backend to compute `cfg(target_feature)`, or more specifically,
 /// to populate `sess.unstable_target_features` and `sess.target_features` (these are the first and
 /// 2nd component of the return value, respectively).
@@ -161,7 +209,7 @@ pub fn cfg(
 ) -> (Vec<Symbol>, Vec<Symbol>) {
     // Compute which of the known target features are enabled in the 'base' target machine. We only
     // consider "supported" features; "forbidden" features are not reflected in `cfg` as of now.
-    let mut features: FxHashSet<Symbol> = sess
+    let mut features: UnordSet<Symbol> = sess
         .target
         .rust_target_features()
         .iter()
@@ -176,43 +224,23 @@ pub fn cfg(
         .collect();
 
     // Add enabled and remove disabled features.
-    for (enabled, feature) in
-        target_feature_flag.split(',').filter_map(|s| match s.chars().next() {
-            Some('+') => Some((true, Symbol::intern(&s[1..]))),
-            Some('-') => Some((false, Symbol::intern(&s[1..]))),
-            _ => None,
-        })
-    {
-        if enabled {
-            // Also add all transitively implied features.
-
-            // We don't care about the order in `features` since the only thing we use it for is the
-            // `features.contains` below.
-            #[allow(rustc::potential_query_instability)]
-            features.extend(
-                sess.target
-                    .implied_target_features(feature.as_str())
-                    .iter()
-                    .map(|s| Symbol::intern(s)),
-            );
-        } else {
-            // Remove transitively reverse-implied features.
-
-            // We don't care about the order in `features` since the only thing we use it for is the
-            // `features.contains` below.
+    parse_rust_feature_flag(
+        sess,
+        target_feature_flag,
+        /* err_callback */ |_| {},
+        |new_features, enabled| {
+            // Iteration order is irrelevant since this only influences an `UnordSet`.
             #[allow(rustc::potential_query_instability)]
-            features.retain(|f| {
-                if sess.target.implied_target_features(f.as_str()).contains(&feature.as_str()) {
-                    // If `f` if implies `feature`, then `!feature` implies `!f`, so we have to
-                    // remove `f`. (This is the standard logical contraposition principle.)
-                    false
-                } else {
-                    // We can keep `f`.
-                    true
+            if enabled {
+                features.extend(new_features.into_iter().map(|f| Symbol::intern(f)));
+            } else {
+                // Remove `new_features` from `features`.
+                for new in new_features {
+                    features.remove(&Symbol::intern(new));
                 }
-            });
-        }
-    }
+            }
+        },
+    );
 
     // Filter enabled features based on feature gates.
     let f = |allow_unstable| {
@@ -275,25 +303,22 @@ pub fn flag_to_backend_features<'a, const N: usize>(
 
     // Compute implied features
     let mut rust_features = vec![];
-    for feature in sess.opts.cg.target_feature.split(',') {
-        if let Some(feature) = feature.strip_prefix('+') {
-            rust_features.extend(
-                UnordSet::from(sess.target.implied_target_features(feature))
-                    .to_sorted_stable_ord()
-                    .iter()
-                    .map(|&&s| (true, s)),
-            )
-        } else if let Some(feature) = feature.strip_prefix('-') {
-            // FIXME: Why do we not remove implied features on "-" here?
-            // We do the equivalent above in `target_config`.
-            // See <https://github.com/rust-lang/rust/issues/134792>.
-            rust_features.push((false, feature));
-        } else if !feature.is_empty() {
+    parse_rust_feature_flag(
+        sess,
+        &sess.opts.cg.target_feature,
+        /* err_callback */
+        |feature| {
             if diagnostics {
                 sess.dcx().emit_warn(error::UnknownCTargetFeaturePrefix { feature });
             }
-        }
-    }
+        },
+        |new_features, enabled| {
+            rust_features.extend(
+                UnordSet::from(new_features).to_sorted_stable_ord().iter().map(|&&s| (enabled, s)),
+            );
+        },
+    );
+
     // Remove features that are meant for rustc, not the backend.
     rust_features.retain(|(_, feature)| {
         // Retain if it is not a rustc feature