about summary refs log tree commit diff
path: root/compiler/rustc_expand
diff options
context:
space:
mode:
Diffstat (limited to 'compiler/rustc_expand')
-rw-r--r--compiler/rustc_expand/messages.ftl26
-rw-r--r--compiler/rustc_expand/src/base.rs32
-rw-r--r--compiler/rustc_expand/src/config.rs62
-rw-r--r--compiler/rustc_expand/src/errors.rs44
-rw-r--r--compiler/rustc_expand/src/expand.rs38
-rw-r--r--compiler/rustc_expand/src/mbe/macro_rules.rs111
-rw-r--r--compiler/rustc_expand/src/mbe/metavar_expr.rs98
7 files changed, 304 insertions, 107 deletions
diff --git a/compiler/rustc_expand/messages.ftl b/compiler/rustc_expand/messages.ftl
index 3fc0fa06191..89d6e62834d 100644
--- a/compiler/rustc_expand/messages.ftl
+++ b/compiler/rustc_expand/messages.ftl
@@ -133,6 +133,32 @@ expand_module_multiple_candidates =
 expand_must_repeat_once =
     this must repeat at least once
 
+expand_mve_extra_tokens =
+    unexpected trailing tokens
+    .label = for this metavariable expression
+    .range = the `{$name}` metavariable expression takes between {$min_or_exact_args} and {$max_args} arguments
+    .exact = the `{$name}` metavariable expression takes {$min_or_exact_args ->
+        [zero] no arguments
+        [one] a single argument
+        *[other] {$min_or_exact_args} arguments
+    }
+    .suggestion = try removing {$extra_count ->
+        [one] this token
+        *[other] these tokens
+    }
+
+expand_mve_missing_paren =
+    expected `(`
+    .label = for this this metavariable expression
+    .unexpected = unexpected token
+    .note = metavariable expressions use function-like parentheses syntax
+    .suggestion = try adding parentheses
+
+expand_mve_unrecognized_expr =
+    unrecognized metavariable expression
+    .label = not a valid metavariable expression
+    .note = valid metavariable expressions are {$valid_expr_list}
+
 expand_mve_unrecognized_var =
     variable `{$key}` is not recognized in meta-variable expression
 
diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs
index d6d89808839..1928cfd9048 100644
--- a/compiler/rustc_expand/src/base.rs
+++ b/compiler/rustc_expand/src/base.rs
@@ -11,7 +11,7 @@ use rustc_ast::token::MetaVarKind;
 use rustc_ast::tokenstream::TokenStream;
 use rustc_ast::visit::{AssocCtxt, Visitor};
 use rustc_ast::{self as ast, AttrVec, Attribute, HasAttrs, Item, NodeId, PatKind};
-use rustc_attr_data_structures::{AttributeKind, Deprecation, Stability, find_attr};
+use rustc_attr_data_structures::{AttributeKind, CfgEntry, Deprecation, Stability, find_attr};
 use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
 use rustc_data_structures::sync;
 use rustc_errors::{DiagCtxtHandle, ErrorGuaranteed, PResult};
@@ -34,6 +34,7 @@ use thin_vec::ThinVec;
 use crate::base::ast::MetaItemInner;
 use crate::errors;
 use crate::expand::{self, AstFragment, Invocation};
+use crate::mbe::macro_rules::ParserAnyMacro;
 use crate::module::DirOwnership;
 use crate::stats::MacroStat;
 
@@ -262,6 +263,25 @@ impl<T, U> ExpandResult<T, U> {
     }
 }
 
+impl<'cx> MacroExpanderResult<'cx> {
+    /// Creates a [`MacroExpanderResult::Ready`] from a [`TokenStream`].
+    ///
+    /// The `TokenStream` is forwarded without any expansion.
+    pub fn from_tts(
+        cx: &'cx mut ExtCtxt<'_>,
+        tts: TokenStream,
+        site_span: Span,
+        arm_span: Span,
+        macro_ident: Ident,
+    ) -> Self {
+        // Emit the SEMICOLON_IN_EXPRESSIONS_FROM_MACROS deprecation lint.
+        let is_local = true;
+
+        let parser = ParserAnyMacro::from_tts(cx, tts, site_span, arm_span, is_local, macro_ident);
+        ExpandResult::Ready(Box::new(parser))
+    }
+}
+
 pub trait MultiItemModifier {
     /// `meta_item` is the attribute, and `item` is the item being modified.
     fn expand(
@@ -1042,7 +1062,7 @@ pub trait ResolverExpand {
     fn next_node_id(&mut self) -> NodeId;
     fn invocation_parent(&self, id: LocalExpnId) -> LocalDefId;
 
-    fn resolve_dollar_crates(&mut self);
+    fn resolve_dollar_crates(&self);
     fn visit_ast_fragment_with_placeholders(
         &mut self,
         expn_id: LocalExpnId,
@@ -1108,7 +1128,13 @@ pub trait ResolverExpand {
     /// HIR proc macros items back to their harness items.
     fn declare_proc_macro(&mut self, id: NodeId);
 
-    fn append_stripped_cfg_item(&mut self, parent_node: NodeId, ident: Ident, cfg: ast::MetaItem);
+    fn append_stripped_cfg_item(
+        &mut self,
+        parent_node: NodeId,
+        ident: Ident,
+        cfg: CfgEntry,
+        cfg_span: Span,
+    );
 
     /// Tools registered with `#![register_tool]` and used by tool attributes and lints.
     fn registered_tools(&self) -> &RegisteredTools;
diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs
index 170ac39d1ec..83a8d601afe 100644
--- a/compiler/rustc_expand/src/config.rs
+++ b/compiler/rustc_expand/src/config.rs
@@ -11,6 +11,9 @@ use rustc_ast::{
     NodeId, NormalAttr,
 };
 use rustc_attr_parsing as attr;
+use rustc_attr_parsing::{
+    AttributeParser, CFG_TEMPLATE, EvalConfigResult, ShouldEmit, eval_config_entry, parse_cfg_attr,
+};
 use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
 use rustc_feature::{
     ACCEPTED_LANG_FEATURES, AttributeSafety, EnabledLangFeature, EnabledLibFeature, Features,
@@ -18,6 +21,7 @@ use rustc_feature::{
 };
 use rustc_lint_defs::BuiltinLintDiag;
 use rustc_parse::validate_attr;
+use rustc_parse::validate_attr::deny_builtin_meta_unsafety;
 use rustc_session::Session;
 use rustc_session::parse::feature_err;
 use rustc_span::{STDLIB_STABLE_CRATES, Span, Symbol, sym};
@@ -161,7 +165,12 @@ pub fn pre_configure_attrs(sess: &Session, attrs: &[Attribute]) -> ast::AttrVec
     attrs
         .iter()
         .flat_map(|attr| strip_unconfigured.process_cfg_attr(attr))
-        .take_while(|attr| !is_cfg(attr) || strip_unconfigured.cfg_true(attr).0)
+        .take_while(|attr| {
+            !is_cfg(attr)
+                || strip_unconfigured
+                    .cfg_true(attr, strip_unconfigured.lint_node_id, ShouldEmit::Nothing)
+                    .as_bool()
+        })
         .collect()
 }
 
@@ -394,26 +403,50 @@ impl<'a> StripUnconfigured<'a> {
 
     /// Determines if a node with the given attributes should be included in this configuration.
     fn in_cfg(&self, attrs: &[Attribute]) -> bool {
-        attrs.iter().all(|attr| !is_cfg(attr) || self.cfg_true(attr).0)
+        attrs.iter().all(|attr| {
+            !is_cfg(attr)
+                || self.cfg_true(attr, self.lint_node_id, ShouldEmit::ErrorsAndLints).as_bool()
+        })
     }
 
-    pub(crate) fn cfg_true(&self, attr: &Attribute) -> (bool, Option<MetaItem>) {
-        let meta_item = match validate_attr::parse_meta(&self.sess.psess, attr) {
-            Ok(meta_item) => meta_item,
+    pub(crate) fn cfg_true(
+        &self,
+        attr: &Attribute,
+        node: NodeId,
+        emit_errors: ShouldEmit,
+    ) -> EvalConfigResult {
+        // We need to run this to do basic validation of the attribute, such as that lits are valid, etc
+        // FIXME(jdonszelmann) this should not be necessary in the future
+        match validate_attr::parse_meta(&self.sess.psess, attr) {
+            Ok(_) => {}
             Err(err) => {
                 err.emit();
-                return (true, None);
+                return EvalConfigResult::True;
             }
-        };
+        }
+
+        // Unsafety check needs to be done explicitly here because this attribute will be removed before the normal check
+        deny_builtin_meta_unsafety(
+            self.sess.dcx(),
+            attr.get_normal_item().unsafety,
+            &rustc_ast::Path::from_ident(attr.ident().unwrap()),
+        );
 
-        validate_attr::deny_builtin_meta_unsafety(&self.sess.psess, &meta_item);
+        let Some(cfg) = AttributeParser::parse_single(
+            self.sess,
+            attr,
+            attr.span,
+            node,
+            self.features,
+            emit_errors,
+            parse_cfg_attr,
+            &CFG_TEMPLATE,
+        ) else {
+            // Cfg attribute was not parsable, give up
+            return EvalConfigResult::True;
+        };
 
-        (
-            parse_cfg(&meta_item, self.sess).is_none_or(|meta_item| {
-                attr::cfg_matches(meta_item, &self.sess, self.lint_node_id, self.features)
-            }),
-            Some(meta_item),
-        )
+        eval_config_entry(self.sess, &cfg, self.lint_node_id, self.features, emit_errors)
     }
 
     /// If attributes are not allowed on expressions, emit an error for `attr`
@@ -465,6 +498,7 @@ impl<'a> StripUnconfigured<'a> {
     }
 }
 
+/// FIXME: Still used by Rustdoc, should be removed after
 pub fn parse_cfg<'a>(meta_item: &'a MetaItem, sess: &Session) -> Option<&'a MetaItemInner> {
     let span = meta_item.span;
     match meta_item.meta_item_list() {
diff --git a/compiler/rustc_expand/src/errors.rs b/compiler/rustc_expand/src/errors.rs
index fdbc65aff68..3ac5d213053 100644
--- a/compiler/rustc_expand/src/errors.rs
+++ b/compiler/rustc_expand/src/errors.rs
@@ -496,6 +496,50 @@ pub(crate) use metavar_exprs::*;
 mod metavar_exprs {
     use super::*;
 
+    #[derive(Diagnostic, Default)]
+    #[diag(expand_mve_extra_tokens)]
+    pub(crate) struct MveExtraTokens {
+        #[primary_span]
+        #[suggestion(code = "", applicability = "machine-applicable")]
+        pub span: Span,
+        #[label]
+        pub ident_span: Span,
+        pub extra_count: usize,
+
+        // The rest is only used for specific diagnostics and can be default if neither
+        // `note` is `Some`.
+        #[note(expand_exact)]
+        pub exact_args_note: Option<()>,
+        #[note(expand_range)]
+        pub range_args_note: Option<()>,
+        pub min_or_exact_args: usize,
+        pub max_args: usize,
+        pub name: String,
+    }
+
+    #[derive(Diagnostic)]
+    #[note]
+    #[diag(expand_mve_missing_paren)]
+    pub(crate) struct MveMissingParen {
+        #[primary_span]
+        #[label]
+        pub ident_span: Span,
+        #[label(expand_unexpected)]
+        pub unexpected_span: Option<Span>,
+        #[suggestion(code = "( /* ... */ )", applicability = "has-placeholders")]
+        pub insert_span: Option<Span>,
+    }
+
+    #[derive(Diagnostic)]
+    #[note]
+    #[diag(expand_mve_unrecognized_expr)]
+    pub(crate) struct MveUnrecognizedExpr {
+        #[primary_span]
+        #[label]
+        pub span: Span,
+        pub valid_expr_list: &'static str,
+    }
+
     #[derive(Diagnostic)]
     #[diag(expand_mve_unrecognized_var)]
     pub(crate) struct MveUnrecognizedVar {
diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs
index 2de09aa1a28..79ec79a2fdf 100644
--- a/compiler/rustc_expand/src/expand.rs
+++ b/compiler/rustc_expand/src/expand.rs
@@ -13,6 +13,7 @@ use rustc_ast::{
     MetaItemKind, ModKind, NodeId, PatKind, StmtKind, TyKind, token,
 };
 use rustc_ast_pretty::pprust;
+use rustc_attr_parsing::{EvalConfigResult, ShouldEmit};
 use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
 use rustc_errors::PResult;
 use rustc_feature::Features;
@@ -2166,19 +2167,19 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
 
     fn expand_cfg_true(
         &mut self,
-        node: &mut impl HasAttrs,
+        node: &mut (impl HasAttrs + HasNodeId),
         attr: ast::Attribute,
         pos: usize,
-    ) -> (bool, Option<ast::MetaItem>) {
-        let (res, meta_item) = self.cfg().cfg_true(&attr);
-        if res {
+    ) -> EvalConfigResult {
+        let res = self.cfg().cfg_true(&attr, node.node_id(), ShouldEmit::ErrorsAndLints);
+        if res.as_bool() {
             // 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)
+        res
     }
 
     fn expand_cfg_attr(&self, node: &mut impl HasAttrs, attr: &ast::Attribute, pos: usize) {
@@ -2199,20 +2200,21 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
             return match self.take_first_attr(&mut node) {
                 Some((attr, pos, derives)) => match attr.name() {
                     Some(sym::cfg) => {
-                        let (res, meta_item) = self.expand_cfg_true(&mut node, attr, pos);
-                        if res {
-                            continue;
-                        }
-
-                        if let Some(meta_item) = meta_item {
-                            for ident in node.declared_idents() {
-                                self.cx.resolver.append_stripped_cfg_item(
-                                    self.cx.current_expansion.lint_node_id,
-                                    ident,
-                                    meta_item.clone(),
-                                )
+                        let res = self.expand_cfg_true(&mut node, attr, pos);
+                        match res {
+                            EvalConfigResult::True => continue,
+                            EvalConfigResult::False { reason, reason_span } => {
+                                for ident in node.declared_idents() {
+                                    self.cx.resolver.append_stripped_cfg_item(
+                                        self.cx.current_expansion.lint_node_id,
+                                        ident,
+                                        reason.clone(),
+                                        reason_span,
+                                    )
+                                }
                             }
                         }
+
                         Default::default()
                     }
                     Some(sym::cfg_attr) => {
@@ -2291,7 +2293,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                 Some((attr, pos, derives)) => match attr.name() {
                     Some(sym::cfg) => {
                         let span = attr.span;
-                        if self.expand_cfg_true(node, attr, pos).0 {
+                        if self.expand_cfg_true(node, attr, pos).as_bool() {
                             continue;
                         }
 
diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs
index 52cdcc5c747..2d792355b27 100644
--- a/compiler/rustc_expand/src/mbe/macro_rules.rs
+++ b/compiler/rustc_expand/src/mbe/macro_rules.rs
@@ -96,6 +96,30 @@ impl<'a> ParserAnyMacro<'a> {
         ensure_complete_parse(parser, &path, kind.name(), site_span);
         fragment
     }
+
+    #[instrument(skip(cx, tts))]
+    pub(crate) fn from_tts<'cx>(
+        cx: &'cx mut ExtCtxt<'a>,
+        tts: TokenStream,
+        site_span: Span,
+        arm_span: Span,
+        is_local: bool,
+        macro_ident: Ident,
+    ) -> Self {
+        Self {
+            parser: Parser::new(&cx.sess.psess, tts, None),
+
+            // Pass along the original expansion site and the name of the macro
+            // so we can print a useful error message if the parse of the expanded
+            // macro leaves unparsed tokens.
+            site_span,
+            macro_ident,
+            lint_node_id: cx.current_expansion.lint_node_id,
+            is_trailing_mac: cx.current_expansion.is_trailing_mac,
+            arm_span,
+            is_local,
+        }
+    }
 }
 
 pub(super) struct MacroRule {
@@ -207,9 +231,6 @@ fn expand_macro<'cx>(
     rules: &[MacroRule],
 ) -> Box<dyn MacResult + 'cx> {
     let psess = &cx.sess.psess;
-    // Macros defined in the current crate have a real node id,
-    // whereas macros from an external crate have a dummy id.
-    let is_local = node_id != DUMMY_NODE_ID;
 
     if cx.trace_macros() {
         let msg = format!("expanding `{}! {{ {} }}`", name, pprust::tts_to_string(&arg));
@@ -220,7 +241,7 @@ fn expand_macro<'cx>(
     let try_success_result = try_match_macro(psess, name, &arg, rules, &mut NoopTracker);
 
     match try_success_result {
-        Ok((i, rule, named_matches)) => {
+        Ok((rule_index, rule, named_matches)) => {
             let mbe::TokenTree::Delimited(rhs_span, _, ref rhs) = rule.rhs else {
                 cx.dcx().span_bug(sp, "malformed macro rhs");
             };
@@ -241,27 +262,13 @@ fn expand_macro<'cx>(
                 trace_macros_note(&mut cx.expansions, sp, msg);
             }
 
-            let p = Parser::new(psess, tts, None);
-
+            let is_local = is_defined_in_current_crate(node_id);
             if is_local {
-                cx.resolver.record_macro_rule_usage(node_id, i);
+                cx.resolver.record_macro_rule_usage(node_id, rule_index);
             }
 
-            // Let the context choose how to interpret the result.
-            // Weird, but useful for X-macros.
-            Box::new(ParserAnyMacro {
-                parser: p,
-
-                // Pass along the original expansion site and the name of the macro
-                // so we can print a useful error message if the parse of the expanded
-                // macro leaves unparsed tokens.
-                site_span: sp,
-                macro_ident: name,
-                lint_node_id: cx.current_expansion.lint_node_id,
-                is_trailing_mac: cx.current_expansion.is_trailing_mac,
-                arm_span,
-                is_local,
-            })
+            // Let the context choose how to interpret the result. Weird, but useful for X-macros.
+            Box::new(ParserAnyMacro::from_tts(cx, tts, sp, arm_span, is_local, name))
         }
         Err(CanRetry::No(guar)) => {
             debug!("Will not retry matching as an error was emitted already");
@@ -374,16 +381,9 @@ pub fn compile_declarative_macro(
     edition: Edition,
 ) -> (SyntaxExtension, usize) {
     let mk_syn_ext = |expander| {
-        SyntaxExtension::new(
-            sess,
-            SyntaxExtensionKind::LegacyBang(expander),
-            span,
-            Vec::new(),
-            edition,
-            ident.name,
-            attrs,
-            node_id != DUMMY_NODE_ID,
-        )
+        let kind = SyntaxExtensionKind::LegacyBang(expander);
+        let is_local = is_defined_in_current_crate(node_id);
+        SyntaxExtension::new(sess, kind, span, Vec::new(), edition, ident.name, attrs, is_local)
     };
     let dummy_syn_ext = |guar| (mk_syn_ext(Arc::new(DummyExpander(guar))), 0);
 
@@ -393,7 +393,8 @@ pub fn compile_declarative_macro(
     let body = macro_def.body.tokens.clone();
     let mut p = Parser::new(&sess.psess, body, rustc_parse::MACRO_ARGUMENTS);
 
-    // Don't abort iteration early, so that multiple errors can be reported.
+    // Don't abort iteration early, so that multiple errors can be reported. We only abort early on
+    // parse failures we can't recover from.
     let mut guar = None;
     let mut check_emission = |ret: Result<(), ErrorGuaranteed>| guar = guar.or(ret.err());
 
@@ -402,20 +403,11 @@ pub fn compile_declarative_macro(
     while p.token != token::Eof {
         let lhs_tt = p.parse_token_tree();
         let lhs_tt = parse_one_tt(lhs_tt, RulePart::Pattern, sess, node_id, features, edition);
-        // We don't handle errors here, the driver will abort after parsing/expansion. We can
-        // report every error in every macro this way.
-        check_emission(check_lhs_nt_follows(sess, node_id, &lhs_tt));
-        check_emission(check_lhs_no_empty_seq(sess, slice::from_ref(&lhs_tt)));
+        check_emission(check_lhs(sess, node_id, &lhs_tt));
         if let Err(e) = p.expect(exp!(FatArrow)) {
             return dummy_syn_ext(e.emit());
         }
-        if p.token == token::Eof {
-            let err_sp = p.token.span.shrink_to_hi();
-            let guar = sess
-                .dcx()
-                .struct_span_err(err_sp, "macro definition ended unexpectedly")
-                .with_span_label(err_sp, "expected right-hand side of macro rule")
-                .emit();
+        if let Some(guar) = check_no_eof(sess, &p, "expected right-hand side of macro rule") {
             return dummy_syn_ext(guar);
         }
         let rhs_tt = p.parse_token_tree();
@@ -454,13 +446,32 @@ pub fn compile_declarative_macro(
     }
 
     // Return the number of rules for unused rule linting, if this is a local macro.
-    let nrules = if node_id != DUMMY_NODE_ID { rules.len() } else { 0 };
+    let nrules = if is_defined_in_current_crate(node_id) { rules.len() } else { 0 };
 
     let expander =
         Arc::new(MacroRulesMacroExpander { name: ident, span, node_id, transparency, rules });
     (mk_syn_ext(expander), nrules)
 }
 
+fn check_no_eof(sess: &Session, p: &Parser<'_>, msg: &'static str) -> Option<ErrorGuaranteed> {
+    if p.token == token::Eof {
+        let err_sp = p.token.span.shrink_to_hi();
+        let guar = sess
+            .dcx()
+            .struct_span_err(err_sp, "macro definition ended unexpectedly")
+            .with_span_label(err_sp, msg)
+            .emit();
+        return Some(guar);
+    }
+    None
+}
+
+fn check_lhs(sess: &Session, node_id: NodeId, lhs: &mbe::TokenTree) -> Result<(), ErrorGuaranteed> {
+    let e1 = check_lhs_nt_follows(sess, node_id, lhs);
+    let e2 = check_lhs_no_empty_seq(sess, slice::from_ref(lhs));
+    e1.and(e2)
+}
+
 fn check_lhs_nt_follows(
     sess: &Session,
     node_id: NodeId,
@@ -1030,9 +1041,7 @@ fn check_matcher_core<'tt>(
                     // definition of this macro_rules, not while (re)parsing
                     // the macro when compiling another crate that is using the
                     // macro. (See #86567.)
-                    // Macros defined in the current crate have a real node id,
-                    // whereas macros from an external crate have a dummy id.
-                    if node_id != DUMMY_NODE_ID
+                    if is_defined_in_current_crate(node_id)
                         && matches!(kind, NonterminalKind::Pat(PatParam { inferred: true }))
                         && matches!(
                             next_token,
@@ -1292,6 +1301,12 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String {
     }
 }
 
+fn is_defined_in_current_crate(node_id: NodeId) -> bool {
+    // Macros defined in the current crate have a real node id,
+    // whereas macros from an external crate have a dummy id.
+    node_id != DUMMY_NODE_ID
+}
+
 pub(super) fn parser_from_cx(
     psess: &ParseSess,
     mut tts: TokenStream,
diff --git a/compiler/rustc_expand/src/mbe/metavar_expr.rs b/compiler/rustc_expand/src/mbe/metavar_expr.rs
index ffd3548019a..d2b275ad20a 100644
--- a/compiler/rustc_expand/src/mbe/metavar_expr.rs
+++ b/compiler/rustc_expand/src/mbe/metavar_expr.rs
@@ -7,6 +7,8 @@ use rustc_macros::{Decodable, Encodable};
 use rustc_session::parse::ParseSess;
 use rustc_span::{Ident, Span, Symbol};
 
+use crate::errors;
+
 pub(crate) const RAW_IDENT_ERR: &str = "`${concat(..)}` currently does not support raw identifiers";
 pub(crate) const UNSUPPORTED_CONCAT_ELEM_ERR: &str = "expected identifier or string literal";
 
@@ -40,11 +42,32 @@ impl MetaVarExpr {
     ) -> PResult<'psess, MetaVarExpr> {
         let mut iter = input.iter();
         let ident = parse_ident(&mut iter, psess, outer_span)?;
-        let Some(TokenTree::Delimited(.., Delimiter::Parenthesis, args)) = iter.next() else {
-            let msg = "meta-variable expression parameter must be wrapped in parentheses";
-            return Err(psess.dcx().struct_span_err(ident.span, msg));
+        let next = iter.next();
+        let Some(TokenTree::Delimited(.., Delimiter::Parenthesis, args)) = next else {
+            // No `()`; wrong or no delimiters. Point at a problematic span or a place to
+            // add parens if it makes sense.
+            let (unexpected_span, insert_span) = match next {
+                Some(TokenTree::Delimited(..)) => (None, None),
+                Some(tt) => (Some(tt.span()), None),
+                None => (None, Some(ident.span.shrink_to_hi())),
+            };
+            let err =
+                errors::MveMissingParen { ident_span: ident.span, unexpected_span, insert_span };
+            return Err(psess.dcx().create_err(err));
         };
-        check_trailing_token(&mut iter, psess)?;
+
+        // Ensure there are no trailing tokens in the braces, e.g. `${foo() extra}`
+        if iter.peek().is_some() {
+            let span = iter_span(&iter).expect("checked is_some above");
+            let err = errors::MveExtraTokens {
+                span,
+                ident_span: ident.span,
+                extra_count: iter.count(),
+                ..Default::default()
+            };
+            return Err(psess.dcx().create_err(err));
+        }
+
         let mut iter = args.iter();
         let rslt = match ident.as_str() {
             "concat" => parse_concat(&mut iter, psess, outer_span, ident.span)?,
@@ -56,18 +79,14 @@ impl MetaVarExpr {
             "index" => MetaVarExpr::Index(parse_depth(&mut iter, psess, ident.span)?),
             "len" => MetaVarExpr::Len(parse_depth(&mut iter, psess, ident.span)?),
             _ => {
-                let err_msg = "unrecognized meta-variable expression";
-                let mut err = psess.dcx().struct_span_err(ident.span, err_msg);
-                err.span_suggestion(
-                    ident.span,
-                    "supported expressions are count, ignore, index and len",
-                    "",
-                    Applicability::MachineApplicable,
-                );
-                return Err(err);
+                let err = errors::MveUnrecognizedExpr {
+                    span: ident.span,
+                    valid_expr_list: "`count`, `ignore`, `index`, `len`, and `concat`",
+                };
+                return Err(psess.dcx().create_err(err));
             }
         };
-        check_trailing_token(&mut iter, psess)?;
+        check_trailing_tokens(&mut iter, psess, ident)?;
         Ok(rslt)
     }
 
@@ -87,20 +106,51 @@ impl MetaVarExpr {
     }
 }
 
-// Checks if there are any remaining tokens. For example, `${ignore(ident ... a b c ...)}`
-fn check_trailing_token<'psess>(
+/// Checks if there are any remaining tokens (for example, `${ignore($valid, extra)}`) and create
+/// a diag with the correct arg count if so.
+fn check_trailing_tokens<'psess>(
     iter: &mut TokenStreamIter<'_>,
     psess: &'psess ParseSess,
+    ident: Ident,
 ) -> PResult<'psess, ()> {
-    if let Some(tt) = iter.next() {
-        let mut diag = psess
-            .dcx()
-            .struct_span_err(tt.span(), format!("unexpected token: {}", pprust::tt_to_string(tt)));
-        diag.span_note(tt.span(), "meta-variable expression must not have trailing tokens");
-        Err(diag)
-    } else {
-        Ok(())
+    if iter.peek().is_none() {
+        // All tokens consumed, as expected
+        return Ok(());
     }
+
+    // `None` for max indicates the arg count must be exact, `Some` indicates a range is accepted.
+    let (min_or_exact_args, max_args) = match ident.as_str() {
+        "concat" => panic!("concat takes unlimited tokens but didn't eat them all"),
+        "ignore" => (1, None),
+        // 1 or 2 args
+        "count" => (1, Some(2)),
+        // 0 or 1 arg
+        "index" => (0, Some(1)),
+        "len" => (0, Some(1)),
+        other => unreachable!("unknown MVEs should be rejected earlier (got `{other}`)"),
+    };
+
+    let err = errors::MveExtraTokens {
+        span: iter_span(iter).expect("checked is_none above"),
+        ident_span: ident.span,
+        extra_count: iter.count(),
+
+        exact_args_note: if max_args.is_some() { None } else { Some(()) },
+        range_args_note: if max_args.is_some() { Some(()) } else { None },
+        min_or_exact_args,
+        max_args: max_args.unwrap_or_default(),
+        name: ident.to_string(),
+    };
+    Err(psess.dcx().create_err(err))
+}
+
+/// Returns a span encompassing all tokens in the iterator if there is at least one item.
+fn iter_span(iter: &TokenStreamIter<'_>) -> Option<Span> {
+    let mut iter = iter.clone(); // cloning is cheap
+    let first_sp = iter.next()?.span();
+    let last_sp = iter.last().map(TokenTree::span).unwrap_or(first_sp);
+    let span = first_sp.with_hi(last_sp.hi());
+    Some(span)
 }
 
 /// Indicates what is placed in a `concat` parameter. For example, literals