Rollup merge of #104559 - nnethercote:split-MacArgs, r=petrochenkov

Split `MacArgs` in two.

`MacArgs` is an enum with three variants: `Empty`, `Delimited`, and `Eq`. It's used in two ways:
- For representing attribute macro arguments (e.g. in `AttrItem`), where all three variants are used.
- For representing function-like macros (e.g. in `MacCall` and `MacroDef`), where only the `Delimited` variant is used.

In other words, `MacArgs` is used in two quite different places due to them having partial overlap. I find this makes the code hard to read. It also leads to various unreachable code paths, and allows invalid values (such as accidentally using `MacArgs::Empty` in a `MacCall`).

This commit splits `MacArgs` in two:
- `DelimArgs` is a new struct just for the "delimited arguments" case. It is now used in `MacCall` and `MacroDef`.
- `AttrArgs` is a renaming of the old `MacArgs` enum for the attribute macro case. Its `Delimited` variant now contains a `DelimArgs`.

Various other related things are renamed as well.

These changes make the code clearer, avoids several unreachable paths, and disallows the invalid values.

r? `@petrochenkov`

Author: matthiaskrgr

compiler/rustc_ast/src/ast.rs

@@ -1544,55 +1544,48 @@ pub enum ClosureBinder {
 #[derive(Clone, Encodable, Decodable, Debug)]
 pub struct MacCall {
     pub path: Path,
-    pub args: P<MacArgs>,
+    pub args: P<DelimArgs>,
     pub prior_type_ascription: Option<(Span, bool)>,
 }
 
 impl MacCall {
     pub fn span(&self) -> Span {
-        self.path.span.to(self.args.span().unwrap_or(self.path.span))
+        self.path.span.to(self.args.dspan.entire())
     }
 }
 
-/// Arguments passed to an attribute or a function-like macro.
+/// Arguments passed to an attribute macro.
 #[derive(Clone, Encodable, Decodable, Debug)]
-pub enum MacArgs {
-    /// No arguments - `#[attr]`.
+pub enum AttrArgs {
+    /// No arguments: `#[attr]`.
     Empty,
-    /// Delimited arguments - `#[attr()/[]/{}]` or `mac!()/[]/{}`.
-    Delimited(DelimSpan, MacDelimiter, TokenStream),
-    /// Arguments of a key-value attribute - `#[attr = "value"]`.
+    /// Delimited arguments: `#[attr()/[]/{}]`.
+    Delimited(DelimArgs),
+    /// Arguments of a key-value attribute: `#[attr = "value"]`.
     Eq(
         /// Span of the `=` token.
         Span,
         /// The "value".
-        MacArgsEq,
+        AttrArgsEq,
     ),
 }
 
-// The RHS of a `MacArgs::Eq` starts out as an expression. Once macro expansion
-// is completed, all cases end up either as a literal, which is the form used
-// after lowering to HIR, or as an error.
+// The RHS of an `AttrArgs::Eq` starts out as an expression. Once macro
+// expansion is completed, all cases end up either as a literal, which is the
+// form used after lowering to HIR, or as an error.
 #[derive(Clone, Encodable, Decodable, Debug)]
-pub enum MacArgsEq {
+pub enum AttrArgsEq {
     Ast(P<Expr>),
     Hir(Lit),
 }
 
-impl MacArgs {
-    pub fn delim(&self) -> Option<Delimiter> {
-        match self {
-            MacArgs::Delimited(_, delim, _) => Some(delim.to_token()),
-            MacArgs::Empty | MacArgs::Eq(..) => None,
-        }
-    }
-
+impl AttrArgs {
     pub fn span(&self) -> Option<Span> {
         match self {
-            MacArgs::Empty => None,
-            MacArgs::Delimited(dspan, ..) => Some(dspan.entire()),
-            MacArgs::Eq(eq_span, MacArgsEq::Ast(expr)) => Some(eq_span.to(expr.span)),
-            MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
+            AttrArgs::Empty => None,
+            AttrArgs::Delimited(args) => Some(args.dspan.entire()),
+            AttrArgs::Eq(eq_span, AttrArgsEq::Ast(expr)) => Some(eq_span.to(expr.span)),
+            AttrArgs::Eq(_, AttrArgsEq::Hir(lit)) => {
                 unreachable!("in literal form when getting span: {:?}", lit);
             }
         }
@@ -1602,46 +1595,64 @@ impl MacArgs {
     /// Proc macros see these tokens, for example.
     pub fn inner_tokens(&self) -> TokenStream {
         match self {
-            MacArgs::Empty => TokenStream::default(),
-            MacArgs::Delimited(.., tokens) => tokens.clone(),
-            MacArgs::Eq(_, MacArgsEq::Ast(expr)) => TokenStream::from_ast(expr),
-            MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
+            AttrArgs::Empty => TokenStream::default(),
+            AttrArgs::Delimited(args) => args.tokens.clone(),
+            AttrArgs::Eq(_, AttrArgsEq::Ast(expr)) => TokenStream::from_ast(expr),
+            AttrArgs::Eq(_, AttrArgsEq::Hir(lit)) => {
                 unreachable!("in literal form when getting inner tokens: {:?}", lit)
             }
         }
     }
-
-    /// Whether a macro with these arguments needs a semicolon
-    /// when used as a standalone item or statement.
-    pub fn need_semicolon(&self) -> bool {
-        !matches!(self, MacArgs::Delimited(_, MacDelimiter::Brace, _))
-    }
 }
 
-impl<CTX> HashStable<CTX> for MacArgs
+impl<CTX> HashStable<CTX> for AttrArgs
 where
     CTX: crate::HashStableContext,
 {
     fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
         mem::discriminant(self).hash_stable(ctx, hasher);
         match self {
-            MacArgs::Empty => {}
-            MacArgs::Delimited(dspan, delim, tokens) => {
-                dspan.hash_stable(ctx, hasher);
-                delim.hash_stable(ctx, hasher);
-                tokens.hash_stable(ctx, hasher);
-            }
-            MacArgs::Eq(_eq_span, MacArgsEq::Ast(expr)) => {
+            AttrArgs::Empty => {}
+            AttrArgs::Delimited(args) => args.hash_stable(ctx, hasher),
+            AttrArgs::Eq(_eq_span, AttrArgsEq::Ast(expr)) => {
                 unreachable!("hash_stable {:?}", expr);
             }
-            MacArgs::Eq(eq_span, MacArgsEq::Hir(lit)) => {
+            AttrArgs::Eq(eq_span, AttrArgsEq::Hir(lit)) => {
                 eq_span.hash_stable(ctx, hasher);
                 lit.hash_stable(ctx, hasher);
             }
         }
     }
 }
 
+/// Delimited arguments, as used in `#[attr()/[]/{}]` or `mac!()/[]/{}`.
+#[derive(Clone, Encodable, Decodable, Debug)]
+pub struct DelimArgs {
+    pub dspan: DelimSpan,
+    pub delim: MacDelimiter,
+    pub tokens: TokenStream,
+}
+
+impl DelimArgs {
+    /// Whether a macro with these arguments needs a semicolon
+    /// when used as a standalone item or statement.
+    pub fn need_semicolon(&self) -> bool {
+        !matches!(self, DelimArgs { delim: MacDelimiter::Brace, .. })
+    }
+}
+
+impl<CTX> HashStable<CTX> for DelimArgs
+where
+    CTX: crate::HashStableContext,
+{
+    fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
+        let DelimArgs { dspan, delim, tokens } = self;
+        dspan.hash_stable(ctx, hasher);
+        delim.hash_stable(ctx, hasher);
+        tokens.hash_stable(ctx, hasher);
+    }
+}
+
 #[derive(Copy, Clone, PartialEq, Eq, Encodable, Decodable, Debug, HashStable_Generic)]
 pub enum MacDelimiter {
     Parenthesis,
@@ -1671,7 +1682,7 @@ impl MacDelimiter {
 /// Represents a macro definition.
 #[derive(Clone, Encodable, Decodable, Debug, HashStable_Generic)]
 pub struct MacroDef {
-    pub body: P<MacArgs>,
+    pub body: P<DelimArgs>,
     /// `true` if macro was defined with `macro_rules`.
     pub macro_rules: bool,
 }
@@ -2534,7 +2545,7 @@ impl<D: Decoder> Decodable<D> for AttrId {
 #[derive(Clone, Encodable, Decodable, Debug, HashStable_Generic)]
 pub struct AttrItem {
     pub path: Path,
-    pub args: MacArgs,
+    pub args: AttrArgs,
     pub tokens: Option<LazyAttrTokenStream>,
 }
 

compiler/rustc_ast/src/attr/mod.rs

@@ -1,9 +1,9 @@
 //! Functions dealing with attributes and meta items.
 
 use crate::ast;
-use crate::ast::{AttrId, AttrItem, AttrKind, AttrStyle, Attribute};
-use crate::ast::{Lit, LitKind};
-use crate::ast::{MacArgs, MacArgsEq, MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem};
+use crate::ast::{AttrArgs, AttrArgsEq, AttrId, AttrItem, AttrKind, AttrStyle, Attribute};
+use crate::ast::{DelimArgs, Lit, LitKind};
+use crate::ast::{MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem};
 use crate::ast::{Path, PathSegment};
 use crate::ptr::P;
 use crate::token::{self, CommentKind, Delimiter, Token};
@@ -158,7 +158,7 @@ impl Attribute {
 
     pub fn is_word(&self) -> bool {
         if let AttrKind::Normal(normal) = &self.kind {
-            matches!(normal.item.args, MacArgs::Empty)
+            matches!(normal.item.args, AttrArgs::Empty)
         } else {
             false
         }
@@ -223,13 +223,13 @@ impl AttrItem {
     pub fn meta(&self, span: Span) -> Option<MetaItem> {
         Some(MetaItem {
             path: self.path.clone(),
-            kind: MetaItemKind::from_mac_args(&self.args)?,
+            kind: MetaItemKind::from_attr_args(&self.args)?,
             span,
         })
     }
 
     pub fn meta_kind(&self) -> Option<MetaItemKind> {
-        MetaItemKind::from_mac_args(&self.args)
+        MetaItemKind::from_attr_args(&self.args)
     }
 }
 
@@ -390,7 +390,7 @@ pub fn mk_attr(
     g: &AttrIdGenerator,
     style: AttrStyle,
     path: Path,
-    args: MacArgs,
+    args: AttrArgs,
     span: Span,
 ) -> Attribute {
     mk_attr_from_item(g, AttrItem { path, args, tokens: None }, None, style, span)
@@ -413,12 +413,12 @@ pub fn mk_attr_from_item(
 
 /// Returns an inner attribute with the given value and span.
 pub fn mk_attr_inner(g: &AttrIdGenerator, item: MetaItem) -> Attribute {
-    mk_attr(g, AttrStyle::Inner, item.path, item.kind.mac_args(item.span), item.span)
+    mk_attr(g, AttrStyle::Inner, item.path, item.kind.attr_args(item.span), item.span)
 }
 
 /// Returns an outer attribute with the given value and span.
 pub fn mk_attr_outer(g: &AttrIdGenerator, item: MetaItem) -> Attribute {
-    mk_attr(g, AttrStyle::Outer, item.path, item.kind.mac_args(item.span), item.span)
+    mk_attr(g, AttrStyle::Outer, item.path, item.kind.attr_args(item.span), item.span)
 }
 
 pub fn mk_doc_comment(
@@ -524,9 +524,9 @@ impl MetaItemKind {
         }
     }
 
-    pub fn mac_args(&self, span: Span) -> MacArgs {
+    pub fn attr_args(&self, span: Span) -> AttrArgs {
         match self {
-            MetaItemKind::Word => MacArgs::Empty,
+            MetaItemKind::Word => AttrArgs::Empty,
             MetaItemKind::NameValue(lit) => {
                 let expr = P(ast::Expr {
                     id: ast::DUMMY_NODE_ID,
@@ -535,7 +535,7 @@ impl MetaItemKind {
                     attrs: ast::AttrVec::new(),
                     tokens: None,
                 });
-                MacArgs::Eq(span, MacArgsEq::Ast(expr))
+                AttrArgs::Eq(span, AttrArgsEq::Ast(expr))
             }
             MetaItemKind::List(list) => {
                 let mut tts = Vec::new();
@@ -545,11 +545,11 @@ impl MetaItemKind {
                     }
                     tts.extend(item.token_trees())
                 }
-                MacArgs::Delimited(
-                    DelimSpan::from_single(span),
-                    MacDelimiter::Parenthesis,
-                    TokenStream::new(tts),
-                )
+                AttrArgs::Delimited(DelimArgs {
+                    dspan: DelimSpan::from_single(span),
+                    delim: MacDelimiter::Parenthesis,
+                    tokens: TokenStream::new(tts),
+                })
             }
         }
     }
@@ -608,20 +608,22 @@ impl MetaItemKind {
         }
     }
 
-    fn from_mac_args(args: &MacArgs) -> Option<MetaItemKind> {
+    fn from_attr_args(args: &AttrArgs) -> Option<MetaItemKind> {
         match args {
-            MacArgs::Empty => Some(MetaItemKind::Word),
-            MacArgs::Delimited(_, MacDelimiter::Parenthesis, tokens) => {
-                MetaItemKind::list_from_tokens(tokens.clone())
-            }
-            MacArgs::Delimited(..) => None,
-            MacArgs::Eq(_, MacArgsEq::Ast(expr)) => match expr.kind {
+            AttrArgs::Empty => Some(MetaItemKind::Word),
+            AttrArgs::Delimited(DelimArgs {
+                dspan: _,
+                delim: MacDelimiter::Parenthesis,
+                tokens,
+            }) => MetaItemKind::list_from_tokens(tokens.clone()),
+            AttrArgs::Delimited(..) => None,
+            AttrArgs::Eq(_, AttrArgsEq::Ast(expr)) => match expr.kind {
                 ast::ExprKind::Lit(token_lit) => Some(MetaItemKind::NameValue(
-                    Lit::from_token_lit(token_lit, expr.span).expect("token_lit in from_mac_args"),
+                    Lit::from_token_lit(token_lit, expr.span).expect("token_lit in from_attr_args"),
                 )),
                 _ => None,
             },
-            MacArgs::Eq(_, MacArgsEq::Hir(lit)) => Some(MetaItemKind::NameValue(lit.clone())),
+            AttrArgs::Eq(_, AttrArgsEq::Hir(lit)) => Some(MetaItemKind::NameValue(lit.clone())),
         }
     }
 

compiler/rustc_ast/src/mut_visit.rs

@@ -367,23 +367,27 @@ pub fn visit_fn_sig<T: MutVisitor>(FnSig { header, decl, span }: &mut FnSig, vis
 }
 
 // No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
-pub fn visit_mac_args<T: MutVisitor>(args: &mut MacArgs, vis: &mut T) {
+pub fn visit_attr_args<T: MutVisitor>(args: &mut AttrArgs, vis: &mut T) {
     match args {
-        MacArgs::Empty => {}
-        MacArgs::Delimited(dspan, _delim, tokens) => {
-            visit_delim_span(dspan, vis);
-            visit_tts(tokens, vis);
-        }
-        MacArgs::Eq(eq_span, MacArgsEq::Ast(expr)) => {
+        AttrArgs::Empty => {}
+        AttrArgs::Delimited(args) => visit_delim_args(args, vis),
+        AttrArgs::Eq(eq_span, AttrArgsEq::Ast(expr)) => {
             vis.visit_span(eq_span);
             vis.visit_expr(expr);
         }
-        MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
+        AttrArgs::Eq(_, AttrArgsEq::Hir(lit)) => {
             unreachable!("in literal form when visiting mac args eq: {:?}", lit)
         }
     }
 }
 
+// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
+pub fn visit_delim_args<T: MutVisitor>(args: &mut DelimArgs, vis: &mut T) {
+    let DelimArgs { dspan, delim: _, tokens } = args;
+    visit_delim_span(dspan, vis);
+    visit_tts(tokens, vis);
+}
+
 pub fn visit_delim_span<T: MutVisitor>(dspan: &mut DelimSpan, vis: &mut T) {
     vis.visit_span(&mut dspan.open);
     vis.visit_span(&mut dspan.close);
@@ -601,7 +605,7 @@ pub fn noop_visit_attribute<T: MutVisitor>(attr: &mut Attribute, vis: &mut T) {
             let NormalAttr { item: AttrItem { path, args, tokens }, tokens: attr_tokens } =
                 &mut **normal;
             vis.visit_path(path);
-            visit_mac_args(args, vis);
+            visit_attr_args(args, vis);
             visit_lazy_tts(tokens, vis);
             visit_lazy_tts(attr_tokens, vis);
         }
@@ -613,12 +617,12 @@ pub fn noop_visit_attribute<T: MutVisitor>(attr: &mut Attribute, vis: &mut T) {
 pub fn noop_visit_mac<T: MutVisitor>(mac: &mut MacCall, vis: &mut T) {
     let MacCall { path, args, prior_type_ascription: _ } = mac;
     vis.visit_path(path);
-    visit_mac_args(args, vis);
+    visit_delim_args(args, vis);
 }
 
 pub fn noop_visit_macro_def<T: MutVisitor>(macro_def: &mut MacroDef, vis: &mut T) {
     let MacroDef { body, macro_rules: _ } = macro_def;
-    visit_mac_args(body, vis);
+    visit_delim_args(body, vis);
 }
 
 pub fn noop_visit_meta_list_item<T: MutVisitor>(li: &mut NestedMetaItem, vis: &mut T) {
@@ -792,7 +796,7 @@ pub fn visit_nonterminal<T: MutVisitor>(nt: &mut token::Nonterminal, vis: &mut T
         token::NtMeta(item) => {
             let AttrItem { path, args, tokens } = item.deref_mut();
             vis.visit_path(path);
-            visit_mac_args(args, vis);
+            visit_attr_args(args, vis);
             visit_lazy_tts(tokens, vis);
         }
         token::NtPath(path) => vis.visit_path(path),