expand: Leave traces when expanding cfg attributes

Author: petrochenkov

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -334,8 +334,7 @@ impl<'a> AstValidator<'a> {
             .filter(|attr| {
                 let arr = [
                     sym::allow,
-                    sym::cfg,
-                    sym::cfg_attr,
+                    sym::cfg_trace,
                     sym::cfg_attr_trace,
                     sym::deny,
                     sym::expect,

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -593,7 +593,7 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
     }
 
     fn print_attribute_inline(&mut self, attr: &ast::Attribute, is_inline: bool) -> bool {
-        if attr.has_name(sym::cfg_attr_trace) {
+        if attr.has_name(sym::cfg_trace) || attr.has_name(sym::cfg_attr_trace) {
             // It's not a valid identifier, so avoid printing it
             // to keep the printed code reasonably parse-able.
             return false;

compiler/rustc_expand/src/config.rs

@@ -156,6 +156,19 @@ pub fn pre_configure_attrs(sess: &Session, attrs: &[Attribute]) -> ast::AttrVec
         .collect()
 }
 
+pub(crate) fn attr_into_trace(mut attr: Attribute, trace_name: Symbol) -> Attribute {
+    match &mut attr.kind {
+        AttrKind::Normal(normal) => {
+            let NormalAttr { item, tokens } = &mut **normal;
+            item.path.segments[0].ident.name = trace_name;
+            // This makes the trace attributes unobservable to token-based proc macros.
+            *tokens = Some(LazyAttrTokenStream::new(AttrTokenStream::default()));
+        }
+        AttrKind::DocComment(..) => unreachable!(),
+    }
+    attr
+}
+
 #[macro_export]
 macro_rules! configure {
     ($this:ident, $node:ident) => {
@@ -280,16 +293,7 @@ impl<'a> StripUnconfigured<'a> {
 
         // A trace attribute left in AST in place of the original `cfg_attr` attribute.
         // It can later be used by lints or other diagnostics.
-        let mut trace_attr = cfg_attr.clone();
-        match &mut trace_attr.kind {
-            AttrKind::Normal(normal) => {
-                let NormalAttr { item, tokens } = &mut **normal;
-                item.path.segments[0].ident.name = sym::cfg_attr_trace;
-                // This makes the trace attributes unobservable to token-based proc macros.
-                *tokens = Some(LazyAttrTokenStream::new(AttrTokenStream::default()));
-            }
-            AttrKind::DocComment(..) => unreachable!(),
-        }
+        let trace_attr = attr_into_trace(cfg_attr.clone(), sym::cfg_attr_trace);
 
         let Some((cfg_predicate, expanded_attrs)) =
             rustc_parse::parse_cfg_attr(cfg_attr, &self.sess.psess)

compiler/rustc_expand/src/expand.rs

@@ -33,7 +33,7 @@ use rustc_span::{ErrorGuaranteed, FileName, Ident, LocalExpnId, Span, sym};
 use smallvec::SmallVec;
 
 use crate::base::*;
-use crate::config::StripUnconfigured;
+use crate::config::{StripUnconfigured, attr_into_trace};
 use crate::errors::{
     EmptyDelegationMac, GlobDelegationOutsideImpls, GlobDelegationTraitlessQpath, IncompleteParse,
     RecursionLimitReached, RemoveExprNotSupported, RemoveNodeNotSupported, UnsupportedKeyValue,
@@ -2003,7 +2003,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                 let attr_name = attr.ident().unwrap().name;
                 // `#[cfg]` and `#[cfg_attr]` are special - they are
                 // eagerly evaluated.
-                if attr_name != sym::cfg && attr_name != sym::cfg_attr_trace {
+                if attr_name != sym::cfg_trace && attr_name != sym::cfg_attr_trace {
                     self.cx.sess.psess.buffer_lint(
                         UNUSED_ATTRIBUTES,
                         attr.span,
@@ -2027,11 +2027,10 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
     ) -> (bool, Option<ast::MetaItem>) {
         let (res, meta_item) = self.cfg().cfg_true(&attr);
         if res {
-            // FIXME: `cfg(TRUE)` attributes do not currently remove themselves during expansion,
-            // and some tools like rustdoc and clippy rely on that. Find a way to remove them
-            // while keeping the tools working.
-            self.cx.expanded_inert_attrs.mark(&attr);
-            node.visit_attrs(|attrs| attrs.insert(pos, attr));
+            // A trace attribute left in AST in place of the original `cfg` attribute.
+            // It can later be used by lints or other diagnostics.
+            let trace_attr = attr_into_trace(attr, sym::cfg_trace);
+            node.visit_attrs(|attrs| attrs.insert(pos, trace_attr));
         }
 
         (res, meta_item)

compiler/rustc_feature/src/builtin_attrs.rs

@@ -760,10 +760,14 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         template!(Word, List: r#""...""#), DuplicatesOk,
         EncodeCrossCrate::Yes, INTERNAL_UNSTABLE
     ),
-    // Trace that is left when a `cfg_attr` attribute is expanded.
-    // The attribute is not gated, to avoid stability errors, but it cannot be used in stable or
-    // unstable code directly because `sym::cfg_attr_trace` is not a valid identifier, it can only
-    // be generated by the compiler.
+    // Traces that are left when `cfg` and `cfg_attr` attributes are expanded.
+    // The attributes are not gated, to avoid stability errors, but they cannot be used in stable
+    // or unstable code directly because `sym::cfg_(attr_)trace` are not valid identifiers, they
+    // can only be generated by the compiler.
+    ungated!(
+        cfg_trace, Normal, template!(Word /* irrelevant */), DuplicatesOk,
+        EncodeCrossCrate::No
+    ),
     ungated!(
         cfg_attr_trace, Normal, template!(Word /* irrelevant */), DuplicatesOk,
         EncodeCrossCrate::No

compiler/rustc_parse/src/validate_attr.rs

@@ -16,7 +16,8 @@ use rustc_span::{Span, Symbol, sym};
 use crate::{errors, parse_in};
 
 pub fn check_attr(psess: &ParseSess, attr: &Attribute) {
-    if attr.is_doc_comment() || attr.has_name(sym::cfg_attr_trace) {
+    if attr.is_doc_comment() || attr.has_name(sym::cfg_trace) || attr.has_name(sym::cfg_attr_trace)
+    {
         return;
     }
 
@@ -215,11 +216,7 @@ pub fn check_builtin_meta_item(
     template: AttributeTemplate,
     deny_unsafety: bool,
 ) {
-    // Some special attributes like `cfg` must be checked
-    // before the generic check, so we skip them here.
-    let should_skip = |name| name == sym::cfg;
-
-    if !should_skip(name) && !is_attr_template_compatible(&template, &meta.kind) {
+    if !is_attr_template_compatible(&template, &meta.kind) {
         emit_malformed_attribute(psess, style, meta.span, name, template);
     }
 

compiler/rustc_passes/src/check_attr.rs

@@ -272,6 +272,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                             | sym::forbid
                             | sym::cfg
                             | sym::cfg_attr
+                            | sym::cfg_trace
                             | sym::cfg_attr_trace
                             // need to be fixed
                             | sym::cfi_encoding // FIXME(cfi_encoding)
@@ -574,8 +575,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         // NOTE: when making changes to this list, check that `error_codes/E0736.md` remains accurate
         const ALLOW_LIST: &[rustc_span::Symbol] = &[
             // conditional compilation
-            sym::cfg,
-            sym::cfg_attr,
+            sym::cfg_trace,
             sym::cfg_attr_trace,
             // testing (allowed here so better errors can be generated in `rustc_builtin_macros::test`)
             sym::test,
@@ -2656,7 +2656,7 @@ impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {
         // only `#[cfg]` and `#[cfg_attr]` are allowed, but it should be removed
         // if we allow more attributes (e.g., tool attributes and `allow/deny/warn`)
         // in where clauses. After that, only `self.check_attributes` should be enough.
-        const ATTRS_ALLOWED: &[Symbol] = &[sym::cfg, sym::cfg_attr, sym::cfg_attr_trace];
+        const ATTRS_ALLOWED: &[Symbol] = &[sym::cfg_trace, sym::cfg_attr_trace];
         let spans = self
             .tcx
             .hir_attrs(where_predicate.hir_id)

compiler/rustc_query_system/src/ich/mod.rs

@@ -8,7 +8,7 @@ mod hcx;
 mod impls_syntax;
 
 pub const IGNORED_ATTRIBUTES: &[Symbol] = &[
-    sym::cfg,
+    sym::cfg_trace, // FIXME should this really be ignored?
     sym::rustc_if_this_changed,
     sym::rustc_then_this_would_need,
     sym::rustc_dirty,

compiler/rustc_span/src/symbol.rs

@@ -623,6 +623,7 @@ symbols! {
         cfg_target_has_atomic_equal_alignment,
         cfg_target_thread_local,
         cfg_target_vendor,
+        cfg_trace: "<cfg>", // must not be a valid identifier
         cfg_ub_checks,
         cfg_version,
         cfi,

src/librustdoc/clean/mod.rs

@@ -2773,7 +2773,7 @@ fn add_without_unwanted_attributes<'hir>(
                 if ident == sym::doc {
                     filter_doc_attr(&mut normal.args, is_inline);
                     attrs.push((Cow::Owned(attr), import_parent));
-                } else if is_inline || ident != sym::cfg {
+                } else if is_inline || ident != sym::cfg_trace {
                     // If it's not a `cfg()` attribute, we keep it.
                     attrs.push((Cow::Owned(attr), import_parent));
                 }

src/librustdoc/clean/types.rs

@@ -1059,7 +1059,7 @@ pub(crate) fn extract_cfg_from_attrs<'a, I: Iterator<Item = &'a hir::Attribute>
             // `doc(cfg())` overrides `cfg()`).
             attrs
                 .clone()
-                .filter(|attr| attr.has_name(sym::cfg))
+                .filter(|attr| attr.has_name(sym::cfg_trace))
                 .filter_map(|attr| single(attr.meta_item_list()?))
                 .filter_map(|attr| Cfg::parse_without(attr.meta_item()?, hidden_cfg).ok().flatten())
                 .fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg)

src/tools/clippy/clippy_lints/src/attrs/duplicated_attributes.rs

@@ -37,7 +37,6 @@ fn check_duplicated_attr(
     let Some(ident) = attr.ident() else { return };
     let name = ident.name;
     if name == sym::doc
-        || name == sym::cfg_attr
         || name == sym::cfg_attr_trace
         || name == sym::rustc_on_unimplemented
         || name == sym::reason {
@@ -47,7 +46,7 @@ fn check_duplicated_attr(
         return;
     }
     if let Some(direct_parent) = parent.last()
-        && ["cfg", "cfg_attr"].contains(&direct_parent.as_str())
+        && direct_parent == sym::cfg_trace.as_str()
         && [sym::all, sym::not, sym::any].contains(&name)
     {
         // FIXME: We don't correctly check `cfg`s for now, so if it's more complex than just a one

src/tools/clippy/clippy_lints/src/cfg_not_test.rs

@@ -32,7 +32,7 @@ declare_lint_pass!(CfgNotTest => [CFG_NOT_TEST]);
 
 impl EarlyLintPass for CfgNotTest {
     fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &rustc_ast::Attribute) {
-        if attr.has_name(rustc_span::sym::cfg) && contains_not_test(attr.meta_item_list().as_deref(), false) {
+        if attr.has_name(rustc_span::sym::cfg_trace) && contains_not_test(attr.meta_item_list().as_deref(), false) {
             span_lint_and_then(
                 cx,
                 CFG_NOT_TEST,

src/tools/clippy/clippy_lints/src/methods/is_empty.rs

@@ -41,7 +41,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, receiver: &Expr<'_
 fn is_under_cfg(cx: &LateContext<'_>, id: HirId) -> bool {
     cx.tcx
         .hir_parent_id_iter(id)
-        .any(|id| cx.tcx.hir_attrs(id).iter().any(|attr| attr.has_name(sym::cfg)))
+        .any(|id| cx.tcx.hir_attrs(id).iter().any(|attr| attr.has_name(sym::cfg_trace)))
 }
 
 /// Similar to [`clippy_utils::expr_or_init`], but does not go up the chain if the initialization

src/tools/clippy/clippy_utils/src/lib.rs

@@ -2629,7 +2629,7 @@ pub fn peel_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>
 pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool {
     if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
         if let Res::Def(_, def_id) = path.res {
-            return cx.tcx.has_attr(def_id, sym::cfg) || cx.tcx.has_attr(def_id, sym::cfg_attr);
+            return cx.tcx.has_attr(def_id, sym::cfg_trace) || cx.tcx.has_attr(def_id, sym::cfg_attr);
         }
     }
     false
@@ -2699,7 +2699,7 @@ pub fn is_in_test_function(tcx: TyCtxt<'_>, id: HirId) -> bool {
 /// use [`is_in_cfg_test`]
 pub fn is_cfg_test(tcx: TyCtxt<'_>, id: HirId) -> bool {
     tcx.hir_attrs(id).iter().any(|attr| {
-        if attr.has_name(sym::cfg)
+        if attr.has_name(sym::cfg_trace)
             && let Some(items) = attr.meta_item_list()
             && let [item] = &*items
             && item.has_name(sym::test)
@@ -2723,11 +2723,11 @@ pub fn is_in_test(tcx: TyCtxt<'_>, hir_id: HirId) -> bool {
 
 /// Checks if the item of any of its parents has `#[cfg(...)]` attribute applied.
 pub fn inherits_cfg(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
-    tcx.has_attr(def_id, sym::cfg)
+    tcx.has_attr(def_id, sym::cfg_trace)
         || tcx
             .hir_parent_iter(tcx.local_def_id_to_hir_id(def_id))
             .flat_map(|(parent_id, _)| tcx.hir_attrs(parent_id))
-            .any(|attr| attr.has_name(sym::cfg))
+            .any(|attr| attr.has_name(sym::cfg_trace))
 }
 
 /// Walks up the HIR tree from the given expression in an attempt to find where the value is

tests/pretty/tests-are-sorted.pp

@@ -10,7 +10,6 @@
 //@ pp-exact:tests-are-sorted.pp
 
 extern crate test;
-#[cfg(test)]
 #[rustc_test_marker = "m_test"]
 #[doc(hidden)]
 pub const m_test: test::TestDescAndFn =
@@ -35,7 +34,6 @@
 fn m_test() {}
 
 extern crate test;
-#[cfg(test)]
 #[rustc_test_marker = "z_test"]
 #[doc(hidden)]
 pub const z_test: test::TestDescAndFn =
@@ -61,7 +59,6 @@
 fn z_test() {}
 
 extern crate test;
-#[cfg(test)]
 #[rustc_test_marker = "a_test"]
 #[doc(hidden)]
 pub const a_test: test::TestDescAndFn =

tests/ui/conditional-compilation/cfg-attr-syntax-validation.rs

@@ -29,7 +29,6 @@ macro_rules! generate_s10 {
     ($expr: expr) => {
         #[cfg(feature = $expr)]
         //~^ ERROR expected unsuffixed literal, found expression `concat!("nonexistent")`
-        //~| ERROR expected unsuffixed literal, found expression `concat!("nonexistent")`
         struct S10;
     }
 }

tests/ui/conditional-compilation/cfg-attr-syntax-validation.stderr

@@ -65,19 +65,7 @@ LL | generate_s10!(concat!("nonexistent"));
    |
    = note: this error originates in the macro `generate_s10` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: expected unsuffixed literal, found expression `concat!("nonexistent")`
-  --> $DIR/cfg-attr-syntax-validation.rs:30:25
-   |
-LL |         #[cfg(feature = $expr)]
-   |                         ^^^^^
-...
-LL | generate_s10!(concat!("nonexistent"));
-   | ------------------------------------- in this macro invocation
-   |
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
-   = note: this error originates in the macro `generate_s10` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error: aborting due to 11 previous errors
+error: aborting due to 10 previous errors
 
 Some errors have detailed explanations: E0537, E0565.
 For more information about an error, try `rustc --explain E0537`.

tests/ui/conditional-compilation/invalid-node-range-issue-129166.rs

@@ -1,11 +1,12 @@
 // This was triggering an assertion failure in `NodeRange::new`.
 
+//@ check-pass
+
 #![feature(cfg_eval)]
 #![feature(stmt_expr_attributes)]
 
 fn f() -> u32 {
     #[cfg_eval] #[cfg(not(FALSE))] 0
-    //~^ ERROR removing an expression is not supported in this position
 }
 
 fn main() {}

tests/ui/conditional-compilation/invalid-node-range-issue-129166.stderr

@@ -2,7 +2,6 @@ macro_rules! mac {
     ($attr_item: meta) => {
         #[cfg($attr_item)]
         //~^ ERROR expected unsuffixed literal, found `meta` metavariable
-        //~| ERROR expected unsuffixed literal, found `meta` metavariable
         struct S;
     }
 }
@@ -11,7 +10,6 @@ mac!(an(arbitrary token stream));
 
 #[cfg(feature = -1)]
 //~^ ERROR expected unsuffixed literal, found `-`
-//~| ERROR expected unsuffixed literal, found `-`
 fn handler() {}
 
 fn main() {}

tests/ui/parser/attribute/attr-bad-meta-4.rs

@@ -1,5 +1,5 @@
 error: expected unsuffixed literal, found `-`
-  --> $DIR/attr-bad-meta-4.rs:12:17
+  --> $DIR/attr-bad-meta-4.rs:11:17
    |
 LL | #[cfg(feature = -1)]
    |                 ^
@@ -15,25 +15,5 @@ LL | mac!(an(arbitrary token stream));
    |
    = note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: expected unsuffixed literal, found `meta` metavariable
-  --> $DIR/attr-bad-meta-4.rs:3:15
-   |
-LL |         #[cfg($attr_item)]
-   |               ^^^^^^^^^^
-...
-LL | mac!(an(arbitrary token stream));
-   | -------------------------------- in this macro invocation
-   |
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
-   = note: this error originates in the macro `mac` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error: expected unsuffixed literal, found `-`
-  --> $DIR/attr-bad-meta-4.rs:12:17
-   |
-LL | #[cfg(feature = -1)]
-   |                 ^
-   |
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
-
-error: aborting due to 4 previous errors
+error: aborting due to 2 previous errors