More attribute cleanups

compiler/rustc_ast/src/attr/mod.rs

@@ -202,21 +202,18 @@ impl Attribute {
         }
     }
 
-    // Named `get_tokens` to distinguish it from the `<Attribute as HasTokens>::tokens` method.
-    pub fn get_tokens(&self) -> TokenStream {
-        match &self.kind {
-            AttrKind::Normal(normal) => TokenStream::new(
-                normal
-                    .tokens
-                    .as_ref()
-                    .unwrap_or_else(|| panic!("attribute is missing tokens: {self:?}"))
-                    .to_attr_token_stream()
-                    .to_token_trees(),
-            ),
-            &AttrKind::DocComment(comment_kind, data) => TokenStream::token_alone(
+    pub fn token_trees(&self) -> Vec<TokenTree> {
+        match self.kind {
+            AttrKind::Normal(ref normal) => normal
+                .tokens
+                .as_ref()
+                .unwrap_or_else(|| panic!("attribute is missing tokens: {self:?}"))
+                .to_attr_token_stream()
+                .to_token_trees(),
+            AttrKind::DocComment(comment_kind, data) => vec![TokenTree::token_alone(
                 token::DocComment(comment_kind, self.style, data),
                 self.span,
-            ),
+            )],
         }
     }
 }

compiler/rustc_ast/src/tokenstream.rs

@@ -16,7 +16,7 @@
 use crate::ast::{AttrStyle, StmtKind};
 use crate::ast_traits::{HasAttrs, HasTokens};
 use crate::token::{self, Delimiter, Nonterminal, Token, TokenKind};
-use crate::AttrVec;
+use crate::{AttrVec, Attribute};
 
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::sync::{self, Lrc};
@@ -179,11 +179,10 @@ impl AttrTokenStream {
         AttrTokenStream(Lrc::new(tokens))
     }
 
-    /// Converts this `AttrTokenStream` to a plain `Vec<TokenTree>`.
-    /// During conversion, `AttrTokenTree::AttrsTarget` get 'flattened'
-    /// back to a `TokenStream` of the form `outer_attr attr_target`.
-    /// If there are inner attributes, they are inserted into the proper
-    /// place in the attribute target tokens.
+    /// Converts this `AttrTokenStream` to a plain `Vec<TokenTree>`. During
+    /// conversion, any `AttrTokenTree::AttrsTarget` gets "flattened" back to a
+    /// `TokenStream`, as described in the comment on
+    /// `attrs_and_tokens_to_token_trees`.
     pub fn to_token_trees(&self) -> Vec<TokenTree> {
         let mut res = Vec::with_capacity(self.0.len());
         for tree in self.0.iter() {
@@ -200,67 +199,84 @@ impl AttrTokenStream {
                     ))
                 }
                 AttrTokenTree::AttrsTarget(target) => {
-                    let idx = target
-                        .attrs
-                        .partition_point(|attr| matches!(attr.style, crate::AttrStyle::Outer));
-                    let (outer_attrs, inner_attrs) = target.attrs.split_at(idx);
-
-                    let mut target_tokens = target.tokens.to_attr_token_stream().to_token_trees();
-                    if !inner_attrs.is_empty() {
-                        let mut found = false;
-                        // Check the last two trees (to account for a trailing semi)
-                        for tree in target_tokens.iter_mut().rev().take(2) {
-                            if let TokenTree::Delimited(span, spacing, delim, delim_tokens) = tree {
-                                // Inner attributes are only supported on extern blocks, functions,
-                                // impls, and modules. All of these have their inner attributes
-                                // placed at the beginning of the rightmost outermost braced group:
-                                // e.g. fn foo() { #![my_attr] }
-                                //
-                                // Therefore, we can insert them back into the right location
-                                // without needing to do any extra position tracking.
-                                //
-                                // Note: Outline modules are an exception - they can
-                                // have attributes like `#![my_attr]` at the start of a file.
-                                // Support for custom attributes in this position is not
-                                // properly implemented - we always synthesize fake tokens,
-                                // so we never reach this code.
-
-                                let mut stream = TokenStream::default();
-                                for inner_attr in inner_attrs {
-                                    stream.push_stream(inner_attr.get_tokens());
-                                }
-                                stream.push_stream(delim_tokens.clone());
-                                *tree = TokenTree::Delimited(*span, *spacing, *delim, stream);
-                                found = true;
-                                break;
-                            }
-                        }
-
-                        assert!(
-                            found,
-                            "Failed to find trailing delimited group in: {target_tokens:?}"
-                        );
-                    }
-                    for attr in outer_attrs {
-                        res.extend(attr.get_tokens().0.iter().cloned());
-                    }
-                    res.extend(target_tokens);
+                    attrs_and_tokens_to_token_trees(&target.attrs, &target.tokens, &mut res);
                 }
             }
         }
         res
     }
 }
 
+// Converts multiple attributes and the tokens for a target AST node into token trees, and appends
+// them to `res`.
+//
+// Example: if the AST node is "fn f() { blah(); }", then:
+// - Simple if no attributes are present, e.g. "fn f() { blah(); }"
+// - Simple if only outer attribute are present, e.g. "#[outer1] #[outer2] fn f() { blah(); }"
+// - Trickier if inner attributes are present, because they must be moved within the AST node's
+//   tokens, e.g. "#[outer] fn f() { #![inner] blah() }"
+fn attrs_and_tokens_to_token_trees(
+    attrs: &[Attribute],
+    target_tokens: &LazyAttrTokenStream,
+    res: &mut Vec<TokenTree>,
+) {
+    let idx = attrs.partition_point(|attr| matches!(attr.style, crate::AttrStyle::Outer));
+    let (outer_attrs, inner_attrs) = attrs.split_at(idx);
+
+    // Add outer attribute tokens.
+    for attr in outer_attrs {
+        res.extend(attr.token_trees());
+    }
+
+    // Add target AST node tokens.
+    res.extend(target_tokens.to_attr_token_stream().to_token_trees());
+
+    // Insert inner attribute tokens.
+    if !inner_attrs.is_empty() {
+        let mut found = false;
+        // Check the last two trees (to account for a trailing semi)
+        for tree in res.iter_mut().rev().take(2) {
+            if let TokenTree::Delimited(span, spacing, delim, delim_tokens) = tree {
+                // Inner attributes are only supported on extern blocks, functions,
+                // impls, and modules. All of these have their inner attributes
+                // placed at the beginning of the rightmost outermost braced group:
+                // e.g. fn foo() { #![my_attr] }
+                //
+                // Therefore, we can insert them back into the right location
+                // without needing to do any extra position tracking.
+                //
+                // Note: Outline modules are an exception - they can
+                // have attributes like `#![my_attr]` at the start of a file.
+                // Support for custom attributes in this position is not
+                // properly implemented - we always synthesize fake tokens,
+                // so we never reach this code.
+                let mut tts = vec![];
+                for inner_attr in inner_attrs {
+                    tts.extend(inner_attr.token_trees());
+                }
+                tts.extend(delim_tokens.0.iter().cloned());
+                let stream = TokenStream::new(tts);
+                *tree = TokenTree::Delimited(*span, *spacing, *delim, stream);
+                found = true;
+                break;
+            }
+        }
+        assert!(found, "Failed to find trailing delimited group in: {res:?}");
+    }
+}
+
 /// Stores the tokens for an attribute target, along
 /// with its attributes.
 ///
 /// This is constructed during parsing when we need to capture
-/// tokens.
+/// tokens, for `cfg` and `cfg_attr` attributes.
 ///
 /// For example, `#[cfg(FALSE)] struct Foo {}` would
 /// have an `attrs` field containing the `#[cfg(FALSE)]` attr,
 /// and a `tokens` field storing the (unparsed) tokens `struct Foo {}`
+///
+/// The `cfg`/`cfg_attr` processing occurs in
+/// `StripUnconfigured::configure_tokens`.
 #[derive(Clone, Debug, Encodable, Decodable)]
 pub struct AttrsTarget {
     /// Attributes, both outer and inner.
@@ -437,18 +453,10 @@ impl TokenStream {
     }
 
     pub fn from_ast(node: &(impl HasAttrs + HasTokens + fmt::Debug)) -> TokenStream {
-        let Some(tokens) = node.tokens() else {
-            panic!("missing tokens for node: {:?}", node);
-        };
-        let attrs = node.attrs();
-        let attr_stream = if attrs.is_empty() {
-            tokens.to_attr_token_stream()
-        } else {
-            let target =
-                AttrsTarget { attrs: attrs.iter().cloned().collect(), tokens: tokens.clone() };
-            AttrTokenStream::new(vec![AttrTokenTree::AttrsTarget(target)])
-        };
-        TokenStream::new(attr_stream.to_token_trees())
+        let tokens = node.tokens().unwrap_or_else(|| panic!("missing tokens for node: {:?}", node));
+        let mut tts = vec![];
+        attrs_and_tokens_to_token_trees(node.attrs(), tokens, &mut tts);
+        TokenStream::new(tts)
     }
 
     pub fn from_nonterminal_ast(nt: &Nonterminal) -> TokenStream {

compiler/rustc_expand/src/config.rs

@@ -187,6 +187,7 @@ impl<'a> StripUnconfigured<'a> {
             .iter()
             .filter_map(|tree| match tree.clone() {
                 AttrTokenTree::AttrsTarget(mut target) => {
+                    // Expand any `cfg_attr` attributes.
                     target.attrs.flat_map_in_place(|attr| self.process_cfg_attr(&attr));
 
                     if self.in_cfg(&target.attrs) {
@@ -195,6 +196,8 @@ impl<'a> StripUnconfigured<'a> {
                         );
                         Some(AttrTokenTree::AttrsTarget(target))
                     } else {
+                        // Remove the target if there's a `cfg` attribute and
+                        // the condition isn't satisfied.
                         None
                     }
                 }
@@ -253,9 +256,9 @@ impl<'a> StripUnconfigured<'a> {
     /// Gives a compiler warning when the `cfg_attr` contains no attributes and
     /// is in the original source file. Gives a compiler error if the syntax of
     /// the attribute is incorrect.
-    pub(crate) fn expand_cfg_attr(&self, attr: &Attribute, recursive: bool) -> Vec<Attribute> {
+    pub(crate) fn expand_cfg_attr(&self, cfg_attr: &Attribute, recursive: bool) -> Vec<Attribute> {
         let Some((cfg_predicate, expanded_attrs)) =
-            rustc_parse::parse_cfg_attr(attr, &self.sess.psess)
+            rustc_parse::parse_cfg_attr(cfg_attr, &self.sess.psess)
         else {
             return vec![];
         };
@@ -264,7 +267,7 @@ impl<'a> StripUnconfigured<'a> {
         if expanded_attrs.is_empty() {
             self.sess.psess.buffer_lint(
                 rustc_lint_defs::builtin::UNUSED_ATTRIBUTES,
-                attr.span,
+                cfg_attr.span,
                 ast::CRATE_NODE_ID,
                 BuiltinLintDiag::CfgAttrNoAttributes,
             );
@@ -280,20 +283,21 @@ impl<'a> StripUnconfigured<'a> {
             //  `#[cfg_attr(false, cfg_attr(true, some_attr))]`.
             expanded_attrs
                 .into_iter()
-                .flat_map(|item| self.process_cfg_attr(&self.expand_cfg_attr_item(attr, item)))
+                .flat_map(|item| self.process_cfg_attr(&self.expand_cfg_attr_item(cfg_attr, item)))
                 .collect()
         } else {
-            expanded_attrs.into_iter().map(|item| self.expand_cfg_attr_item(attr, item)).collect()
+            expanded_attrs
+                .into_iter()
+                .map(|item| self.expand_cfg_attr_item(cfg_attr, item))
+                .collect()
         }
     }
 
     fn expand_cfg_attr_item(
         &self,
-        attr: &Attribute,
+        cfg_attr: &Attribute,
         (item, item_span): (ast::AttrItem, Span),
     ) -> Attribute {
-        let orig_tokens = attr.get_tokens();
-
         // We are taking an attribute of the form `#[cfg_attr(pred, attr)]`
         // and producing an attribute of the form `#[attr]`. We
         // have captured tokens for `attr` itself, but we need to
@@ -302,11 +306,11 @@ impl<'a> StripUnconfigured<'a> {
 
         // Use the `#` in `#[cfg_attr(pred, attr)]` as the `#` token
         // for `attr` when we expand it to `#[attr]`
-        let mut orig_trees = orig_tokens.trees();
+        let mut orig_trees = cfg_attr.token_trees().into_iter();
         let TokenTree::Token(pound_token @ Token { kind: TokenKind::Pound, .. }, _) =
             orig_trees.next().unwrap().clone()
         else {
-            panic!("Bad tokens for attribute {attr:?}");
+            panic!("Bad tokens for attribute {cfg_attr:?}");
         };
 
         // We don't really have a good span to use for the synthesized `[]`
@@ -320,12 +324,12 @@ impl<'a> StripUnconfigured<'a> {
                 .unwrap_or_else(|| panic!("Missing tokens for {item:?}"))
                 .to_attr_token_stream(),
         );
-        let trees = if attr.style == AttrStyle::Inner {
+        let trees = if cfg_attr.style == AttrStyle::Inner {
             // For inner attributes, we do the same thing for the `!` in `#![some_attr]`
             let TokenTree::Token(bang_token @ Token { kind: TokenKind::Not, .. }, _) =
                 orig_trees.next().unwrap().clone()
             else {
-                panic!("Bad tokens for attribute {attr:?}");
+                panic!("Bad tokens for attribute {cfg_attr:?}");
             };
             vec![
                 AttrTokenTree::Token(pound_token, Spacing::Joint),
@@ -340,7 +344,7 @@ impl<'a> StripUnconfigured<'a> {
             &self.sess.psess.attr_id_generator,
             item,
             tokens,
-            attr.style,
+            cfg_attr.style,
             item_span,
         );
         if attr.has_name(sym::crate_type) {

compiler/rustc_parse/src/lib.rs

@@ -157,14 +157,14 @@ pub fn fake_token_stream_for_crate(psess: &ParseSess, krate: &ast::Crate) -> Tok
 }
 
 pub fn parse_cfg_attr(
-    attr: &Attribute,
+    cfg_attr: &Attribute,
     psess: &ParseSess,
 ) -> Option<(MetaItem, Vec<(AttrItem, Span)>)> {
     const CFG_ATTR_GRAMMAR_HELP: &str = "#[cfg_attr(condition, attribute, other_attribute, ...)]";
     const CFG_ATTR_NOTE_REF: &str = "for more information, visit \
         <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute>";
 
-    match attr.get_normal_item().args {
+    match cfg_attr.get_normal_item().args {
         ast::AttrArgs::Delimited(ast::DelimArgs { dspan, delim, ref tokens })
             if !tokens.is_empty() =>
         {
@@ -180,7 +180,7 @@ pub fn parse_cfg_attr(
         }
         _ => {
             psess.dcx().emit_err(errors::MalformedCfgAttr {
-                span: attr.span,
+                span: cfg_attr.span,
                 sugg: CFG_ATTR_GRAMMAR_HELP,
             });
         }