Auto merge of rust-lang#138844 - petrochenkov:cfgtrace2, r=<try>

expand: Leave traces when expanding `cfg` attributes

This is the same as rust-lang#138515, but for `cfg(true)` instead of `cfg_attr`.

The difference is that `cfg(true)`s already left "traces" after themselves - the `cfg` attributes themselves, with `expanded_inert_attrs` set to true, with full tokens, available to proc macros.
This is not a reasonably expected behavior, but it could not be removed without a replacement, because a [major rustdoc feature](rust-lang/rfcs#3631) and a number of clippy lints rely on it. This PR implements a replacement.

This needs a crater run, because it changes observable behavior (in an intended way) - proc macros can no longer see expanded `cfg(true)` attributes.

(Some minor unnecessary special casing for `sym::cfg_attr` is also removed in this PR.)

r? `@nnethercote`

Author: bors

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,
@@ -1941,7 +1941,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 {
+                if attr_name != sym::cfg_trace && attr_name != sym::cfg_attr_trace {
                     self.cx.sess.psess.buffer_lint(
                         UNUSED_ATTRIBUTES,
                         attr.span,
@@ -1965,11 +1965,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

@@ -611,6 +611,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));
                 }