Auto merge of #138515 - petrochenkov:cfgtrace, r=nnethercote

expand: Leave traces when expanding `cfg_attr` attributes

Currently `cfg_trace` just disappears during expansion, but after this PR `#[cfg_attr(some tokens)]` will leave a `#[cfg_attr_trace(some tokens)]` attribute instead of itself in AST after expansion (the new attribute is built-in and inert, its inner tokens are the same as in the original attribute).
This trace attribute can then be used by lints or other diagnostics, #133823 has some examples.

Tokens in these trace attributes are set to an empty token stream, so the traces are non-existent for proc macros and cannot affect any user-observable behavior.
This is also a weakness, because if a proc macro processes some code with the trace attributes, they will be lost, so the traces are best effort rather than precise.

The next step is to do the same thing with `cfg` attributes (`#[cfg(TRUE)]` currently remains in both AST and tokens after expanding, it should be replaced with a trace instead).

The idea belongs to `@estebank.`

Author: bors

Diff for: compiler/rustc_ast_passes/src/ast_validation.rs

@@ -336,6 +336,7 @@ impl<'a> AstValidator<'a> {
                     sym::allow,
                     sym::cfg,
                     sym::cfg_attr,
+                    sym::cfg_attr_trace,
                     sym::deny,
                     sym::expect,
                     sym::forbid,

Diff for: compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -578,11 +578,12 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
         let mut printed = false;
         for attr in attrs {
             if attr.style == kind {
-                self.print_attribute_inline(attr, is_inline);
-                if is_inline {
-                    self.nbsp();
+                if self.print_attribute_inline(attr, is_inline) {
+                    if is_inline {
+                        self.nbsp();
+                    }
+                    printed = true;
                 }
-                printed = true;
             }
         }
         if printed && trailing_hardbreak && !is_inline {
@@ -591,7 +592,12 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
         printed
     }
 
-    fn print_attribute_inline(&mut self, attr: &ast::Attribute, is_inline: bool) {
+    fn print_attribute_inline(&mut self, attr: &ast::Attribute, is_inline: bool) -> bool {
+        if 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;
+        }
         if !is_inline {
             self.hardbreak_if_not_bol();
         }
@@ -610,6 +616,7 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
                 self.hardbreak()
             }
         }
+        true
     }
 
     fn print_attr_item(&mut self, item: &ast::AttrItem, span: Span) {
@@ -2047,7 +2054,7 @@ impl<'a> State<'a> {
     }
 
     fn print_attribute(&mut self, attr: &ast::Attribute) {
-        self.print_attribute_inline(attr, false)
+        self.print_attribute_inline(attr, false);
     }
 
     fn print_meta_list_item(&mut self, item: &ast::MetaItemInner) {

Diff for: compiler/rustc_expand/src/config.rs

@@ -1,12 +1,15 @@
 //! Conditional compilation stripping.
 
+use std::iter;
+
 use rustc_ast::ptr::P;
 use rustc_ast::token::{Delimiter, Token, TokenKind};
 use rustc_ast::tokenstream::{
     AttrTokenStream, AttrTokenTree, LazyAttrTokenStream, Spacing, TokenTree,
 };
 use rustc_ast::{
-    self as ast, AttrStyle, Attribute, HasAttrs, HasTokens, MetaItem, MetaItemInner, NodeId,
+    self as ast, AttrKind, AttrStyle, Attribute, HasAttrs, HasTokens, MetaItem, MetaItemInner,
+    NodeId, NormalAttr,
 };
 use rustc_attr_parsing as attr;
 use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
@@ -275,10 +278,23 @@ impl<'a> StripUnconfigured<'a> {
     pub(crate) fn expand_cfg_attr(&self, cfg_attr: &Attribute, recursive: bool) -> Vec<Attribute> {
         validate_attr::check_attribute_safety(&self.sess.psess, AttributeSafety::Normal, &cfg_attr);
 
+        // 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 Some((cfg_predicate, expanded_attrs)) =
             rustc_parse::parse_cfg_attr(cfg_attr, &self.sess.psess)
         else {
-            return vec![];
+            return vec![trace_attr];
         };
 
         // Lint on zero attributes in source.
@@ -292,22 +308,21 @@ impl<'a> StripUnconfigured<'a> {
         }
 
         if !attr::cfg_matches(&cfg_predicate, &self.sess, self.lint_node_id, self.features) {
-            return vec![];
+            return vec![trace_attr];
         }
 
         if recursive {
             // We call `process_cfg_attr` recursively in case there's a
             // `cfg_attr` inside of another `cfg_attr`. E.g.
             //  `#[cfg_attr(false, cfg_attr(true, some_attr))]`.
-            expanded_attrs
+            let expanded_attrs = expanded_attrs
                 .into_iter()
-                .flat_map(|item| self.process_cfg_attr(&self.expand_cfg_attr_item(cfg_attr, item)))
-                .collect()
+                .flat_map(|item| self.process_cfg_attr(&self.expand_cfg_attr_item(cfg_attr, item)));
+            iter::once(trace_attr).chain(expanded_attrs).collect()
         } else {
-            expanded_attrs
-                .into_iter()
-                .map(|item| self.expand_cfg_attr_item(cfg_attr, item))
-                .collect()
+            let expanded_attrs =
+                expanded_attrs.into_iter().map(|item| self.expand_cfg_attr_item(cfg_attr, item));
+            iter::once(trace_attr).chain(expanded_attrs).collect()
         }
     }
 

Diff for: compiler/rustc_feature/src/builtin_attrs.rs

@@ -752,6 +752,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.
+    ungated!(
+        cfg_attr_trace, Normal, template!(Word /* irrelevant */), DuplicatesOk,
+        EncodeCrossCrate::No
+    ),
 
     // ==========================================================================
     // Internal attributes, Diagnostics related:

Diff for: compiler/rustc_parse/src/validate_attr.rs

@@ -16,7 +16,7 @@ use rustc_span::{Span, Symbol, sym};
 use crate::{errors, parse_in};
 
 pub fn check_attr(psess: &ParseSess, attr: &Attribute) {
-    if attr.is_doc_comment() {
+    if attr.is_doc_comment() || attr.has_name(sym::cfg_attr_trace) {
         return;
     }
 

Diff for: compiler/rustc_passes/src/check_attr.rs

@@ -272,6 +272,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                             | sym::forbid
                             | sym::cfg
                             | sym::cfg_attr
+                            | sym::cfg_attr_trace
                             // need to be fixed
                             | sym::cfi_encoding // FIXME(cfi_encoding)
                             | sym::pointee // FIXME(derive_coerce_pointee)
@@ -575,6 +576,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
             // conditional compilation
             sym::cfg,
             sym::cfg_attr,
+            sym::cfg_attr_trace,
             // testing (allowed here so better errors can be generated in `rustc_builtin_macros::test`)
             sym::test,
             sym::ignore,
@@ -2641,7 +2643,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];
+        const ATTRS_ALLOWED: &[Symbol] = &[sym::cfg, sym::cfg_attr, sym::cfg_attr_trace];
         let spans = self
             .tcx
             .hir_attrs(where_predicate.hir_id)

Diff for: compiler/rustc_span/src/symbol.rs

@@ -591,6 +591,7 @@ symbols! {
         cfg_accessible,
         cfg_attr,
         cfg_attr_multi,
+        cfg_attr_trace: "<cfg_attr>", // must not be a valid identifier
         cfg_boolean_literals,
         cfg_contract_checks,
         cfg_doctest,

Diff for: src/tools/clippy/clippy_lints/src/attrs/duplicated_attributes.rs

@@ -36,7 +36,11 @@ 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::rustc_on_unimplemented || name == sym::reason {
+    if name == sym::doc
+        || name == sym::cfg_attr
+        || name == sym::cfg_attr_trace
+        || name == sym::rustc_on_unimplemented
+        || name == sym::reason {
         // FIXME: Would be nice to handle `cfg_attr` as well. Only problem is to check that cfg
         // conditions are the same.
         // `#[rustc_on_unimplemented]` contains duplicated subattributes, that's expected.

Diff for: tests/ui/proc-macro/cfg-attr-trace.rs

@@ -0,0 +1,17 @@
+// Ensure that `cfg_attr_trace` attributes aren't observable by proc-macros.
+
+//@ check-pass
+//@ proc-macro: test-macros.rs
+
+#![feature(cfg_eval)]
+
+#[macro_use]
+extern crate test_macros;
+
+#[cfg_eval]
+#[test_macros::print_attr]
+#[cfg_attr(FALSE, test_macros::print_attr)]
+#[cfg_attr(all(), test_macros::print_attr)]
+struct S;
+
+fn main() {}

Diff for: tests/ui/proc-macro/cfg-attr-trace.stdout

@@ -0,0 +1,62 @@
+PRINT-ATTR INPUT (DISPLAY): #[test_macros::print_attr] struct S;
+PRINT-ATTR DEEP-RE-COLLECTED (DISPLAY): #[test_macros :: print_attr] struct S;
+PRINT-ATTR INPUT (DEBUG): TokenStream [
+    Punct {
+        ch: '#',
+        spacing: Alone,
+        span: #0 bytes(271..272),
+    },
+    Group {
+        delimiter: Bracket,
+        stream: TokenStream [
+            Ident {
+                ident: "test_macros",
+                span: #0 bytes(289..300),
+            },
+            Punct {
+                ch: ':',
+                spacing: Joint,
+                span: #0 bytes(300..301),
+            },
+            Punct {
+                ch: ':',
+                spacing: Alone,
+                span: #0 bytes(301..302),
+            },
+            Ident {
+                ident: "print_attr",
+                span: #0 bytes(302..312),
+            },
+        ],
+        span: #0 bytes(272..314),
+    },
+    Ident {
+        ident: "struct",
+        span: #0 bytes(315..321),
+    },
+    Ident {
+        ident: "S",
+        span: #0 bytes(322..323),
+    },
+    Punct {
+        ch: ';',
+        spacing: Alone,
+        span: #0 bytes(323..324),
+    },
+]
+PRINT-ATTR INPUT (DISPLAY): struct S;
+PRINT-ATTR INPUT (DEBUG): TokenStream [
+    Ident {
+        ident: "struct",
+        span: #0 bytes(315..321),
+    },
+    Ident {
+        ident: "S",
+        span: #0 bytes(322..323),
+    },
+    Punct {
+        ch: ';',
+        spacing: Alone,
+        span: #0 bytes(323..324),
+    },
+]