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.

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,39 +1595,29 @@ 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);
             }
@@ -1642,6 +1625,34 @@ where
     }
 }
 
+/// 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>,
 }