From e446a562ab2984c8085d30d57403a56bbc925883 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 29 May 2019 20:29:51 +0200 Subject: [PATCH 1/3] Remember the span of the Kleene operator in macros This is needed for having complete error messages where reporting macro variable errors. Here is what they would look like: error: meta-variable repeats with different kleene operator --> $DIR/issue-61053-different-kleene.rs:3:57 | LL | ( $( $i:ident = $($j:ident),+ );* ) => { $( $( $i = $j; )* )* }; | - expected repetition ^^ - conflicting repetition --- src/libsyntax/ext/tt/macro_parser.rs | 6 +++--- src/libsyntax/ext/tt/macro_rules.rs | 16 +++++++-------- src/libsyntax/ext/tt/quoted.rs | 30 +++++++++++++++++++--------- src/libsyntax/ext/tt/transcribe.rs | 2 +- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/libsyntax/ext/tt/macro_parser.rs b/src/libsyntax/ext/tt/macro_parser.rs index 92ce3779a3c81..e931a97f8b641 100644 --- a/src/libsyntax/ext/tt/macro_parser.rs +++ b/src/libsyntax/ext/tt/macro_parser.rs @@ -557,8 +557,8 @@ fn inner_parse_loop<'root, 'tt>( // implicitly disallowing OneOrMore from having 0 matches here. Thus, that will // result in a "no rules expected token" error by virtue of this matcher not // working. - if seq.op == quoted::KleeneOp::ZeroOrMore - || seq.op == quoted::KleeneOp::ZeroOrOne + if seq.kleene.op == quoted::KleeneOp::ZeroOrMore + || seq.kleene.op == quoted::KleeneOp::ZeroOrOne { let mut new_item = item.clone(); new_item.match_cur += seq.num_captures; @@ -573,7 +573,7 @@ fn inner_parse_loop<'root, 'tt>( cur_items.push(MatcherPosHandle::Box(Box::new(MatcherPos { stack: smallvec![], sep: seq.separator.clone(), - seq_op: Some(seq.op), + seq_op: Some(seq.kleene.op), idx: 0, matches, match_lo: item.match_cur, diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index 1a448cb2a43cf..0ed40de041272 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -276,7 +276,7 @@ pub fn compile( if body.legacy { token::Semi } else { token::Comma }, def.span, )), - op: quoted::KleeneOp::OneOrMore, + kleene: quoted::KleeneToken::new(quoted::KleeneOp::OneOrMore, def.span), num_captures: 2, }), ), @@ -286,7 +286,7 @@ pub fn compile( Lrc::new(quoted::SequenceRepetition { tts: vec![quoted::TokenTree::token(token::Semi, def.span)], separator: None, - op: quoted::KleeneOp::ZeroOrMore, + kleene: quoted::KleeneToken::new(quoted::KleeneOp::ZeroOrMore, def.span), num_captures: 0, }), ), @@ -484,8 +484,8 @@ fn check_lhs_no_empty_seq(sess: &ParseSess, tts: &[quoted::TokenTree]) -> bool { && seq.tts.iter().all(|seq_tt| match *seq_tt { TokenTree::MetaVarDecl(_, _, id) => id.name == sym::vis, TokenTree::Sequence(_, ref sub_seq) => { - sub_seq.op == quoted::KleeneOp::ZeroOrMore - || sub_seq.op == quoted::KleeneOp::ZeroOrOne + sub_seq.kleene.op == quoted::KleeneOp::ZeroOrMore + || sub_seq.kleene.op == quoted::KleeneOp::ZeroOrOne } _ => false, }) @@ -635,8 +635,8 @@ impl FirstSets { // Reverse scan: Sequence comes before `first`. if subfirst.maybe_empty - || seq_rep.op == quoted::KleeneOp::ZeroOrMore - || seq_rep.op == quoted::KleeneOp::ZeroOrOne + || seq_rep.kleene.op == quoted::KleeneOp::ZeroOrMore + || seq_rep.kleene.op == quoted::KleeneOp::ZeroOrOne { // If sequence is potentially empty, then // union them (preserving first emptiness). @@ -684,8 +684,8 @@ impl FirstSets { assert!(first.maybe_empty); first.add_all(subfirst); if subfirst.maybe_empty - || seq_rep.op == quoted::KleeneOp::ZeroOrMore - || seq_rep.op == quoted::KleeneOp::ZeroOrOne + || seq_rep.kleene.op == quoted::KleeneOp::ZeroOrMore + || seq_rep.kleene.op == quoted::KleeneOp::ZeroOrOne { // continue scanning for more first // tokens, but also make sure we diff --git a/src/libsyntax/ext/tt/quoted.rs b/src/libsyntax/ext/tt/quoted.rs index ccf9db842ab6e..eff6b9dd429e4 100644 --- a/src/libsyntax/ext/tt/quoted.rs +++ b/src/libsyntax/ext/tt/quoted.rs @@ -50,11 +50,23 @@ pub struct SequenceRepetition { /// The optional separator pub separator: Option, /// Whether the sequence can be repeated zero (*), or one or more times (+) - pub op: KleeneOp, + pub kleene: KleeneToken, /// The number of `Match`s that appear in the sequence (and subsequences) pub num_captures: usize, } +#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, Copy)] +pub struct KleeneToken { + pub span: Span, + pub op: KleeneOp, +} + +impl KleeneToken { + pub fn new(op: KleeneOp, span: Span) -> KleeneToken { + KleeneToken { span, op } + } +} + /// A Kleene-style [repetition operator](http://en.wikipedia.org/wiki/Kleene_star) /// for token sequences. #[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] @@ -273,7 +285,7 @@ fn parse_tree( macro_node_id, ); // Get the Kleene operator and optional separator - let (separator, op) = parse_sep_and_kleene_op(trees, span.entire(), sess); + let (separator, kleene) = parse_sep_and_kleene_op(trees, span.entire(), sess); // Count the number of captured "names" (i.e., named metavars) let name_captures = macro_parser::count_names(&sequence); TokenTree::Sequence( @@ -281,7 +293,7 @@ fn parse_tree( Lrc::new(SequenceRepetition { tts: sequence, separator, - op, + kleene, num_captures: name_captures, }), ) @@ -379,16 +391,16 @@ fn parse_sep_and_kleene_op( input: &mut Peekable>, span: Span, sess: &ParseSess, -) -> (Option, KleeneOp) { +) -> (Option, KleeneToken) { // We basically look at two token trees here, denoted as #1 and #2 below let span = match parse_kleene_op(input, span) { // #1 is a `?`, `+`, or `*` KleeneOp - Ok(Ok((op, _))) => return (None, op), + Ok(Ok((op, span))) => return (None, KleeneToken::new(op, span)), // #1 is a separator followed by #2, a KleeneOp Ok(Err(token)) => match parse_kleene_op(input, token.span) { // #2 is the `?` Kleene op, which does not take a separator (error) - Ok(Ok((KleeneOp::ZeroOrOne, _))) => { + Ok(Ok((KleeneOp::ZeroOrOne, span))) => { // Error! sess.span_diagnostic.span_err( token.span, @@ -396,11 +408,11 @@ fn parse_sep_and_kleene_op( ); // Return a dummy - return (None, KleeneOp::ZeroOrMore); + return (None, KleeneToken::new(KleeneOp::ZeroOrMore, span)); } // #2 is a KleeneOp :D - Ok(Ok((op, _))) => return (Some(token), op), + Ok(Ok((op, span))) => return (Some(token), KleeneToken::new(op, span)), // #2 is a random token or not a token at all :( Ok(Err(Token { span, .. })) | Err(span) => span, @@ -414,5 +426,5 @@ fn parse_sep_and_kleene_op( sess.span_diagnostic.span_err(span, "expected one of: `*`, `+`, or `?`"); // Return a dummy - (None, KleeneOp::ZeroOrMore) + (None, KleeneToken::new(KleeneOp::ZeroOrMore, span)) } diff --git a/src/libsyntax/ext/tt/transcribe.rs b/src/libsyntax/ext/tt/transcribe.rs index ea7f8e356aa63..d3c84e55e769d 100644 --- a/src/libsyntax/ext/tt/transcribe.rs +++ b/src/libsyntax/ext/tt/transcribe.rs @@ -183,7 +183,7 @@ pub fn transcribe( // Is the repetition empty? if len == 0 { - if seq.op == quoted::KleeneOp::OneOrMore { + if seq.kleene.op == quoted::KleeneOp::OneOrMore { // FIXME: this really ought to be caught at macro definition // time... It happens when the Kleene operator in the matcher and // the body for the same meta-variable do not match. From 98a6587d28ad42b73a591525a6cabe3cf9c13f43 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 30 May 2019 12:53:27 +0200 Subject: [PATCH 2/3] Implement checks for meta-variables in macros --- src/librustc/lint/builtin.rs | 7 + src/librustc/lint/mod.rs | 3 +- src/libsyntax/early_buffered_lints.rs | 1 + src/libsyntax/ext/tt/macro_check.rs | 620 ++++++++++++++++++ src/libsyntax/ext/tt/macro_rules.rs | 52 +- src/libsyntax/ext/tt/quoted.rs | 16 + src/libsyntax/lib.rs | 1 + .../run-pass/macros/meta-variable-misuse.rs | 34 + .../ui/macros/issue-61053-different-kleene.rs | 30 + .../issue-61053-different-kleene.stderr | 45 ++ .../ui/macros/issue-61053-duplicate-binder.rs | 14 + .../issue-61053-duplicate-binder.stderr | 16 + .../macros/issue-61053-missing-repetition.rs | 28 + .../issue-61053-missing-repetition.stderr | 33 + src/test/ui/macros/issue-61053-unbound.rs | 28 + src/test/ui/macros/issue-61053-unbound.stderr | 26 + 16 files changed, 907 insertions(+), 47 deletions(-) create mode 100644 src/libsyntax/ext/tt/macro_check.rs create mode 100644 src/test/run-pass/macros/meta-variable-misuse.rs create mode 100644 src/test/ui/macros/issue-61053-different-kleene.rs create mode 100644 src/test/ui/macros/issue-61053-different-kleene.stderr create mode 100644 src/test/ui/macros/issue-61053-duplicate-binder.rs create mode 100644 src/test/ui/macros/issue-61053-duplicate-binder.stderr create mode 100644 src/test/ui/macros/issue-61053-missing-repetition.rs create mode 100644 src/test/ui/macros/issue-61053-missing-repetition.stderr create mode 100644 src/test/ui/macros/issue-61053-unbound.rs create mode 100644 src/test/ui/macros/issue-61053-unbound.stderr diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 45e598531b969..c881d92a471f5 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -353,6 +353,12 @@ pub mod parser { Warn, "ill-formed attribute inputs that were previously accepted and used in practice" } + + declare_lint! { + pub META_VARIABLE_MISUSE, + Allow, + "possible meta-variable misuse at macro definition" + } } declare_lint! { @@ -439,6 +445,7 @@ declare_lint_pass! { MACRO_USE_EXTERN_CRATE, MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS, parser::ILL_FORMED_ATTRIBUTE_INPUT, + parser::META_VARIABLE_MISUSE, DEPRECATED_IN_FUTURE, AMBIGUOUS_ASSOCIATED_ITEMS, NESTED_IMPL_TRAIT, diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 309af4b72c127..56d29ec28c134 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -27,7 +27,7 @@ use crate::hir::def_id::{CrateNum, LOCAL_CRATE}; use crate::hir::intravisit; use crate::hir; use crate::lint::builtin::BuiltinLintDiagnostics; -use crate::lint::builtin::parser::ILL_FORMED_ATTRIBUTE_INPUT; +use crate::lint::builtin::parser::{ILL_FORMED_ATTRIBUTE_INPUT, META_VARIABLE_MISUSE}; use crate::session::{Session, DiagnosticMessageId}; use crate::ty::TyCtxt; use crate::ty::query::Providers; @@ -81,6 +81,7 @@ impl Lint { pub fn from_parser_lint_id(lint_id: BufferedEarlyLintId) -> &'static Self { match lint_id { BufferedEarlyLintId::IllFormedAttributeInput => ILL_FORMED_ATTRIBUTE_INPUT, + BufferedEarlyLintId::MetaVariableMisuse => META_VARIABLE_MISUSE, } } diff --git a/src/libsyntax/early_buffered_lints.rs b/src/libsyntax/early_buffered_lints.rs index b26a1165fed1d..36c1da2929975 100644 --- a/src/libsyntax/early_buffered_lints.rs +++ b/src/libsyntax/early_buffered_lints.rs @@ -10,6 +10,7 @@ use syntax_pos::MultiSpan; /// passed to `rustc::lint::Lint::from_parser_lint_id` to get a `rustc::lint::Lint`. pub enum BufferedEarlyLintId { IllFormedAttributeInput, + MetaVariableMisuse, } /// Stores buffered lint info which can later be passed to `librustc`. diff --git a/src/libsyntax/ext/tt/macro_check.rs b/src/libsyntax/ext/tt/macro_check.rs new file mode 100644 index 0000000000000..a1fc965dec002 --- /dev/null +++ b/src/libsyntax/ext/tt/macro_check.rs @@ -0,0 +1,620 @@ +//! Checks that meta-variables in macro definition are correctly declared and used. +//! +//! # What is checked +//! +//! ## Meta-variables must not be bound twice +//! +//! ``` +//! macro_rules! foo { ($x:tt $x:tt) => { $x }; } +//! ``` +//! +//! This check is sound (no false-negative) and complete (no false-positive). +//! +//! ## Meta-variables must not be free +//! +//! ``` +//! macro_rules! foo { () => { $x }; } +//! ``` +//! +//! This check is also done at macro instantiation but only if the branch is taken. +//! +//! ## Meta-variables must repeat at least as many times as their binder +//! +//! ``` +//! macro_rules! foo { ($($x:tt)*) => { $x }; } +//! ``` +//! +//! This check is also done at macro instantiation but only if the branch is taken. +//! +//! ## Meta-variables must repeat with the same Kleene operators as their binder +//! +//! ``` +//! macro_rules! foo { ($($x:tt)+) => { $($x)* }; } +//! ``` +//! +//! This check is not done at macro instantiation. +//! +//! # Disclaimer +//! +//! In the presence of nested macros (a macro defined in a macro), those checks may have false +//! positives and false negatives. We try to detect those cases by recognizing potential macro +//! definitions in RHSes, but nested macros may be hidden through the use of particular values of +//! meta-variables. +//! +//! ## Examples of false positive +//! +//! False positives can come from cases where we don't recognize a nested macro, because it depends +//! on particular values of meta-variables. In the following example, we think both instances of +//! `$x` are free, which is a correct statement if `$name` is anything but `macro_rules`. But when +//! `$name` is `macro_rules`, like in the instantiation below, then `$x:tt` is actually a binder of +//! the nested macro and `$x` is bound to it. +//! +//! ``` +//! macro_rules! foo { ($name:ident) => { $name! bar { ($x:tt) => { $x }; } }; } +//! foo!(macro_rules); +//! ``` +//! +//! False positives can also come from cases where we think there is a nested macro while there +//! isn't. In the following example, we think `$x` is free, which is incorrect because `bar` is not +//! a nested macro since it is not evaluated as code by `stringify!`. +//! +//! ``` +//! macro_rules! foo { () => { stringify!(macro_rules! bar { () => { $x }; }) }; } +//! ``` +//! +//! ## Examples of false negative +//! +//! False negatives can come from cases where we don't recognize a meta-variable, because it depends +//! on particular values of meta-variables. In the following examples, we don't see that if `$d` is +//! instantiated with `$` then `$d z` becomes `$z` in the nested macro definition and is thus a free +//! meta-variable. Note however, that if `foo` is instantiated, then we would check the definition +//! of `bar` and would see the issue. +//! +//! ``` +//! macro_rules! foo { ($d:tt) => { macro_rules! bar { ($y:tt) => { $d z }; } }; } +//! ``` +//! +//! # How it is checked +//! +//! There are 3 main functions: `check_binders`, `check_occurrences`, and `check_nested_macro`. They +//! all need some kind of environment. +//! +//! ## Environments +//! +//! Environments are used to pass information. +//! +//! ### From LHS to RHS +//! +//! When checking a LHS with `check_binders`, we produce (and use) an environment for binders, +//! namely `Binders`. This is a mapping from binder name to information about that binder: the span +//! of the binder for error messages and the stack of Kleene operators under which it was bound in +//! the LHS. +//! +//! This environment is used by both the LHS and RHS. The LHS uses it to detect duplicate binders. +//! The RHS uses it to detect the other errors. +//! +//! ### From outer macro to inner macro +//! +//! When checking the RHS of an outer macro and we detect a nested macro definition, we push the +//! current state, namely `MacroState`, to an environment of nested macro definitions. Each state +//! stores the LHS binders when entering the macro definition as well as the stack of Kleene +//! operators under which the inner macro is defined in the RHS. +//! +//! This environment is a stack representing the nesting of macro definitions. As such, the stack of +//! Kleene operators under which a meta-variable is repeating is the concatenation of the stacks +//! stored when entering a macro definition starting from the state in which the meta-variable is +//! bound. +use crate::ast::NodeId; +use crate::early_buffered_lints::BufferedEarlyLintId; +use crate::ext::tt::quoted::{KleeneToken, TokenTree}; +use crate::parse::token::TokenKind; +use crate::parse::token::{DelimToken, Token}; +use crate::parse::ParseSess; +use crate::symbol::{kw, sym}; + +use rustc_data_structures::fx::FxHashMap; +use smallvec::SmallVec; +use syntax_pos::{symbol::Ident, MultiSpan, Span}; + +/// Stack represented as linked list. +/// +/// Those are used for environments because they grow incrementally and are not mutable. +enum Stack<'a, T> { + /// Empty stack. + Empty, + /// A non-empty stack. + Push { + /// The top element. + top: T, + /// The previous elements. + prev: &'a Stack<'a, T>, + }, +} + +impl<'a, T> Stack<'a, T> { + /// Returns whether a stack is empty. + fn is_empty(&self) -> bool { + match *self { + Stack::Empty => true, + _ => false, + } + } + + /// Returns a new stack with an element of top. + fn push(&'a self, top: T) -> Stack<'a, T> { + Stack::Push { top, prev: self } + } +} + +impl<'a, T> Iterator for &'a Stack<'a, T> { + type Item = &'a T; + + // Iterates from top to bottom of the stack. + fn next(&mut self) -> Option<&'a T> { + match *self { + Stack::Empty => None, + Stack::Push { ref top, ref prev } => { + *self = prev; + Some(top) + } + } + } +} + +impl From<&Stack<'_, KleeneToken>> for SmallVec<[KleeneToken; 1]> { + fn from(ops: &Stack<'_, KleeneToken>) -> SmallVec<[KleeneToken; 1]> { + let mut ops: SmallVec<[KleeneToken; 1]> = ops.cloned().collect(); + // The stack is innermost on top. We want outermost first. + ops.reverse(); + ops + } +} + +/// Information attached to a meta-variable binder in LHS. +struct BinderInfo { + /// The span of the meta-variable in LHS. + span: Span, + /// The stack of Kleene operators (outermost first). + ops: SmallVec<[KleeneToken; 1]>, +} + +/// An environment of meta-variables to their binder information. +type Binders = FxHashMap; + +/// The state at which we entered a macro definition in the RHS of another macro definition. +struct MacroState<'a> { + /// The binders of the branch where we entered the macro definition. + binders: &'a Binders, + /// The stack of Kleene operators (outermost first) where we entered the macro definition. + ops: SmallVec<[KleeneToken; 1]>, +} + +/// Checks that meta-variables are used correctly in a macro definition. +/// +/// Arguments: +/// - `sess` is used to emit diagnostics and lints +/// - `node_id` is used to emit lints +/// - `span` is used when no spans are available +/// - `lhses` and `rhses` should have the same length and represent the macro definition +pub fn check_meta_variables( + sess: &ParseSess, + node_id: NodeId, + span: Span, + lhses: &[TokenTree], + rhses: &[TokenTree], +) -> bool { + if lhses.len() != rhses.len() { + sess.span_diagnostic.span_bug(span, "length mismatch between LHSes and RHSes") + } + let mut valid = true; + for (lhs, rhs) in lhses.iter().zip(rhses.iter()) { + let mut binders = Binders::default(); + check_binders(sess, node_id, lhs, &Stack::Empty, &mut binders, &Stack::Empty, &mut valid); + check_occurrences(sess, node_id, rhs, &Stack::Empty, &binders, &Stack::Empty, &mut valid); + } + valid +} + +/// Checks `lhs` as part of the LHS of a macro definition, extends `binders` with new binders, and +/// sets `valid` to false in case of errors. +/// +/// Arguments: +/// - `sess` is used to emit diagnostics and lints +/// - `node_id` is used to emit lints +/// - `lhs` is checked as part of a LHS +/// - `macros` is the stack of possible outer macros +/// - `binders` contains the binders of the LHS +/// - `ops` is the stack of Kleene operators from the LHS +/// - `valid` is set in case of errors +fn check_binders( + sess: &ParseSess, + node_id: NodeId, + lhs: &TokenTree, + macros: &Stack<'_, MacroState<'_>>, + binders: &mut Binders, + ops: &Stack<'_, KleeneToken>, + valid: &mut bool, +) { + match *lhs { + TokenTree::Token(..) => {} + // This can only happen when checking a nested macro because this LHS is then in the RHS of + // the outer macro. See run-pass/macros/macro-of-higher-order.rs where $y:$fragment in the + // LHS of the nested macro (and RHS of the outer macro) is parsed as MetaVar(y) Colon + // MetaVar(fragment) and not as MetaVarDecl(y, fragment). + TokenTree::MetaVar(span, name) => { + if macros.is_empty() { + sess.span_diagnostic.span_bug(span, "unexpected MetaVar in lhs"); + } + // There are 3 possibilities: + if let Some(prev_info) = binders.get(&name) { + // 1. The meta-variable is already bound in the current LHS: This is an error. + let mut span = MultiSpan::from_span(span); + span.push_span_label(prev_info.span, "previous declaration".into()); + buffer_lint(sess, span, node_id, "duplicate matcher binding"); + } else if get_binder_info(macros, binders, name).is_none() { + // 2. The meta-variable is free: This is a binder. + binders.insert(name, BinderInfo { span, ops: ops.into() }); + } else { + // 3. The meta-variable is bound: This is an occurrence. + check_occurrences(sess, node_id, lhs, macros, binders, ops, valid); + } + } + // Similarly, this can only happen when checking a toplevel macro. + TokenTree::MetaVarDecl(span, name, _kind) => { + if !macros.is_empty() { + sess.span_diagnostic.span_bug(span, "unexpected MetaVarDecl in nested lhs"); + } + if let Some(prev_info) = get_binder_info(macros, binders, name) { + // Duplicate binders at the top-level macro definition are errors. The lint is only + // for nested macro definitions. + sess.span_diagnostic + .struct_span_err(span, "duplicate matcher binding") + .span_note(prev_info.span, "previous declaration was here") + .emit(); + *valid = false; + } else { + binders.insert(name, BinderInfo { span, ops: ops.into() }); + } + } + TokenTree::Delimited(_, ref del) => { + for tt in &del.tts { + check_binders(sess, node_id, tt, macros, binders, ops, valid); + } + } + TokenTree::Sequence(_, ref seq) => { + let ops = ops.push(seq.kleene); + for tt in &seq.tts { + check_binders(sess, node_id, tt, macros, binders, &ops, valid); + } + } + } +} + +/// Returns the binder information of a meta-variable. +/// +/// Arguments: +/// - `macros` is the stack of possible outer macros +/// - `binders` contains the current binders +/// - `name` is the name of the meta-variable we are looking for +fn get_binder_info<'a>( + mut macros: &'a Stack<'a, MacroState<'a>>, + binders: &'a Binders, + name: Ident, +) -> Option<&'a BinderInfo> { + binders.get(&name).or_else(|| macros.find_map(|state| state.binders.get(&name))) +} + +/// Checks `rhs` as part of the RHS of a macro definition and sets `valid` to false in case of +/// errors. +/// +/// Arguments: +/// - `sess` is used to emit diagnostics and lints +/// - `node_id` is used to emit lints +/// - `rhs` is checked as part of a RHS +/// - `macros` is the stack of possible outer macros +/// - `binders` contains the binders of the associated LHS +/// - `ops` is the stack of Kleene operators from the RHS +/// - `valid` is set in case of errors +fn check_occurrences( + sess: &ParseSess, + node_id: NodeId, + rhs: &TokenTree, + macros: &Stack<'_, MacroState<'_>>, + binders: &Binders, + ops: &Stack<'_, KleeneToken>, + valid: &mut bool, +) { + match *rhs { + TokenTree::Token(..) => {} + TokenTree::MetaVarDecl(span, _name, _kind) => { + sess.span_diagnostic.span_bug(span, "unexpected MetaVarDecl in rhs") + } + TokenTree::MetaVar(span, name) => { + check_ops_is_prefix(sess, node_id, macros, binders, ops, span, name); + } + TokenTree::Delimited(_, ref del) => { + check_nested_occurrences(sess, node_id, &del.tts, macros, binders, ops, valid); + } + TokenTree::Sequence(_, ref seq) => { + let ops = ops.push(seq.kleene); + check_nested_occurrences(sess, node_id, &seq.tts, macros, binders, &ops, valid); + } + } +} + +/// Represents the processed prefix of a nested macro. +#[derive(Clone, Copy)] +enum NestedMacroState { + /// Nothing that matches a nested macro definition was processed yet. + Empty, + /// The token `macro_rules` was processed. + MacroRules, + /// The tokens `macro_rules!` were processed. + MacroRulesNot, + /// The tokens `macro_rules!` followed by a name were processed. The name may be either directly + /// an identifier or a meta-variable (that hopefully would be instantiated by an identifier). + MacroRulesNotName, + /// The keyword `macro` was processed. + Macro, + /// The keyword `macro` followed by a name was processed. + MacroName, + /// The keyword `macro` followed by a name and a token delimited by parentheses was processed. + MacroNameParen, +} + +/// Checks `tts` as part of the RHS of a macro definition, tries to recognize nested macro +/// definitions, and sets `valid` to false in case of errors. +/// +/// Arguments: +/// - `sess` is used to emit diagnostics and lints +/// - `node_id` is used to emit lints +/// - `tts` is checked as part of a RHS and may contain macro definitions +/// - `macros` is the stack of possible outer macros +/// - `binders` contains the binders of the associated LHS +/// - `ops` is the stack of Kleene operators from the RHS +/// - `valid` is set in case of errors +fn check_nested_occurrences( + sess: &ParseSess, + node_id: NodeId, + tts: &[TokenTree], + macros: &Stack<'_, MacroState<'_>>, + binders: &Binders, + ops: &Stack<'_, KleeneToken>, + valid: &mut bool, +) { + let mut state = NestedMacroState::Empty; + let nested_macros = macros.push(MacroState { binders, ops: ops.into() }); + let mut nested_binders = Binders::default(); + for tt in tts { + match (state, tt) { + ( + NestedMacroState::Empty, + &TokenTree::Token(Token { kind: TokenKind::Ident(name, false), .. }), + ) => { + if name == sym::macro_rules { + state = NestedMacroState::MacroRules; + } else if name == kw::Macro { + state = NestedMacroState::Macro; + } + } + ( + NestedMacroState::MacroRules, + &TokenTree::Token(Token { kind: TokenKind::Not, .. }), + ) => { + state = NestedMacroState::MacroRulesNot; + } + ( + NestedMacroState::MacroRulesNot, + &TokenTree::Token(Token { kind: TokenKind::Ident(..), .. }), + ) => { + state = NestedMacroState::MacroRulesNotName; + } + (NestedMacroState::MacroRulesNot, &TokenTree::MetaVar(..)) => { + state = NestedMacroState::MacroRulesNotName; + // We check that the meta-variable is correctly used. + check_occurrences(sess, node_id, tt, macros, binders, ops, valid); + } + (NestedMacroState::MacroRulesNotName, &TokenTree::Delimited(_, ref del)) + | (NestedMacroState::MacroName, &TokenTree::Delimited(_, ref del)) + if del.delim == DelimToken::Brace => + { + state = NestedMacroState::Empty; + let rest = check_nested_macro(sess, node_id, &del.tts, &nested_macros, valid); + // If we did not check the whole macro definition, then check the rest as if outside + // the macro definition. + check_nested_occurrences( + sess, + node_id, + &del.tts[rest..], + macros, + binders, + ops, + valid, + ); + } + ( + NestedMacroState::Macro, + &TokenTree::Token(Token { kind: TokenKind::Ident(..), .. }), + ) => { + state = NestedMacroState::MacroName; + } + (NestedMacroState::Macro, &TokenTree::MetaVar(..)) => { + state = NestedMacroState::MacroName; + // We check that the meta-variable is correctly used. + check_occurrences(sess, node_id, tt, macros, binders, ops, valid); + } + (NestedMacroState::MacroName, &TokenTree::Delimited(_, ref del)) + if del.delim == DelimToken::Paren => + { + state = NestedMacroState::MacroNameParen; + nested_binders = Binders::default(); + check_binders( + sess, + node_id, + tt, + &nested_macros, + &mut nested_binders, + &Stack::Empty, + valid, + ); + } + (NestedMacroState::MacroNameParen, &TokenTree::Delimited(_, ref del)) + if del.delim == DelimToken::Brace => + { + state = NestedMacroState::Empty; + check_occurrences( + sess, + node_id, + tt, + &nested_macros, + &nested_binders, + &Stack::Empty, + valid, + ); + } + (_, ref tt) => { + state = NestedMacroState::Empty; + check_occurrences(sess, node_id, tt, macros, binders, ops, valid); + } + } + } +} + +/// Checks the body of nested macro, returns where the check stopped, and sets `valid` to false in +/// case of errors. +/// +/// The token trees are checked as long as they look like a list of (LHS) => {RHS} token trees. This +/// check is a best-effort to detect a macro definition. It returns the position in `tts` where we +/// stopped checking because we detected we were not in a macro definition anymore. +/// +/// Arguments: +/// - `sess` is used to emit diagnostics and lints +/// - `node_id` is used to emit lints +/// - `tts` is checked as a list of (LHS) => {RHS}; +/// - `macros` is the stack of outer macros +/// - `valid` is set in case of errors +fn check_nested_macro( + sess: &ParseSess, + node_id: NodeId, + tts: &[TokenTree], + macros: &Stack<'_, MacroState<'_>>, + valid: &mut bool, +) -> usize { + let n = tts.len(); + let mut i = 0; + loop { + // We expect 3 token trees: `(LHS) => {RHS}`. The semicolon is checked after. + if i + 2 >= n + || !tts[i].is_delimited() + || !tts[i + 1].is_token(&TokenKind::FatArrow) + || !tts[i + 2].is_delimited() + { + break; + } + let lhs = &tts[i]; + let rhs = &tts[i + 2]; + let mut binders = Binders::default(); + check_binders(sess, node_id, lhs, macros, &mut binders, &Stack::Empty, valid); + check_occurrences(sess, node_id, rhs, macros, &binders, &Stack::Empty, valid); + // Since the last semicolon is optional, we increment our checked position by how many token + // trees we already checked (the 3 above) before checking for the semicolon. + i += 3; + if i == n || !tts[i].is_token(&TokenKind::Semi) { + break; + } + // We increment our checked position for the semicolon. + i += 1; + } + i +} + +/// Checks that a meta-variable occurrence is valid. +/// +/// Arguments: +/// - `sess` is used to emit diagnostics and lints +/// - `node_id` is used to emit lints +/// - `macros` is the stack of possible outer macros +/// - `binders` contains the binders of the associated LHS +/// - `ops` is the stack of Kleene operators from the RHS +/// - `span` is the span of the meta-variable to check +/// - `name` is the name of the meta-variable to check +fn check_ops_is_prefix( + sess: &ParseSess, + node_id: NodeId, + macros: &Stack<'_, MacroState<'_>>, + binders: &Binders, + ops: &Stack<'_, KleeneToken>, + span: Span, + name: Ident, +) { + let macros = macros.push(MacroState { binders, ops: ops.into() }); + // Accumulates the stacks the operators of each state until (and including when) the + // meta-variable is found. The innermost stack is first. + let mut acc: SmallVec<[&SmallVec<[KleeneToken; 1]>; 1]> = SmallVec::new(); + for state in ¯os { + acc.push(&state.ops); + if let Some(binder) = state.binders.get(&name) { + // This variable concatenates the stack of operators from the RHS of the LHS where the + // meta-variable was defined to where it is used (in possibly nested macros). The + // outermost operator is first. + let mut occurrence_ops: SmallVec<[KleeneToken; 2]> = SmallVec::new(); + // We need to iterate from the end to start with outermost stack. + for ops in acc.iter().rev() { + occurrence_ops.extend_from_slice(ops); + } + ops_is_prefix(sess, node_id, span, name, &binder.ops, &occurrence_ops); + return; + } + } + buffer_lint(sess, span.into(), node_id, &format!("unknown macro variable `{}`", name)); +} + +/// Returns whether `binder_ops` is a prefix of `occurrence_ops`. +/// +/// The stack of Kleene operators of a meta-variable occurrence just needs to have the stack of +/// Kleene operators of its binder as a prefix. +/// +/// Consider $i in the following example: +/// +/// ( $( $i:ident = $($j:ident),+ );* ) => { $($( $i += $j; )+)* } +/// +/// It occurs under the Kleene stack ["*", "+"] and is bound under ["*"] only. +/// +/// Arguments: +/// - `sess` is used to emit diagnostics and lints +/// - `node_id` is used to emit lints +/// - `span` is the span of the meta-variable being check +/// - `name` is the name of the meta-variable being check +/// - `binder_ops` is the stack of Kleene operators for the binder +/// - `occurrence_ops` is the stack of Kleene operators for the occurrence +fn ops_is_prefix( + sess: &ParseSess, + node_id: NodeId, + span: Span, + name: Ident, + binder_ops: &[KleeneToken], + occurrence_ops: &[KleeneToken], +) { + for (i, binder) in binder_ops.iter().enumerate() { + if i >= occurrence_ops.len() { + let mut span = MultiSpan::from_span(span); + span.push_span_label(binder.span, "expected repetition".into()); + let message = &format!("variable '{}' is still repeating at this depth", name); + buffer_lint(sess, span, node_id, message); + return; + } + let occurrence = &occurrence_ops[i]; + if occurrence.op != binder.op { + let mut span = MultiSpan::from_span(span); + span.push_span_label(binder.span, "expected repetition".into()); + span.push_span_label(occurrence.span, "conflicting repetition".into()); + let message = "meta-variable repeats with different Kleene operator"; + buffer_lint(sess, span, node_id, message); + return; + } + } +} + +fn buffer_lint(sess: &ParseSess, span: MultiSpan, node_id: NodeId, message: &str) { + sess.buffer_lint(BufferedEarlyLintId::MetaVariableMisuse, span, node_id, message); +} diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index 0ed40de041272..84508bf77f2fb 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -3,6 +3,7 @@ use crate::ext::base::{DummyResult, ExtCtxt, MacResult, TTMacroExpander}; use crate::ext::base::{SyntaxExtension, SyntaxExtensionKind}; use crate::ext::expand::{AstFragment, AstFragmentKind}; use crate::ext::hygiene::Transparency; +use crate::ext::tt::macro_check; use crate::ext::tt::macro_parser::{parse, parse_failure_msg}; use crate::ext::tt::macro_parser::{Error, Failure, Success}; use crate::ext::tt::macro_parser::{MatchedNonterminal, MatchedSeq}; @@ -19,7 +20,7 @@ use crate::{ast, attr}; use errors::FatalError; use log::debug; -use syntax_pos::{symbol::Ident, Span}; +use syntax_pos::Span; use rustc_data_structures::fx::FxHashMap; use std::borrow::Cow; @@ -369,14 +370,12 @@ pub fn compile( // don't abort iteration early, so that errors for multiple lhses can be reported for lhs in &lhses { valid &= check_lhs_no_empty_seq(sess, slice::from_ref(lhs)); - valid &= check_lhs_duplicate_matcher_bindings( - sess, - slice::from_ref(lhs), - &mut FxHashMap::default(), - def.id, - ); } + // We use CRATE_NODE_ID instead of `def.id` otherwise we may emit buffered lints for a node id + // that is not lint-checked and trigger the "failed to process buffered lint here" bug. + valid &= macro_check::check_meta_variables(sess, ast::CRATE_NODE_ID, def.span, &lhses, &rhses); + let expander: Box<_> = Box::new(MacroRulesMacroExpander { name: def.ident, lhses, rhses, valid }); @@ -504,45 +503,6 @@ fn check_lhs_no_empty_seq(sess: &ParseSess, tts: &[quoted::TokenTree]) -> bool { true } -/// Check that the LHS contains no duplicate matcher bindings. e.g. `$a:expr, $a:expr` would be -/// illegal, since it would be ambiguous which `$a` to use if we ever needed to. -fn check_lhs_duplicate_matcher_bindings( - sess: &ParseSess, - tts: &[quoted::TokenTree], - metavar_names: &mut FxHashMap, - node_id: ast::NodeId, -) -> bool { - use self::quoted::TokenTree; - for tt in tts { - match *tt { - TokenTree::MetaVarDecl(span, name, _kind) => { - if let Some(&prev_span) = metavar_names.get(&name) { - sess.span_diagnostic - .struct_span_err(span, "duplicate matcher binding") - .span_note(prev_span, "previous declaration was here") - .emit(); - return false; - } else { - metavar_names.insert(name, span); - } - } - TokenTree::Delimited(_, ref del) => { - if !check_lhs_duplicate_matcher_bindings(sess, &del.tts, metavar_names, node_id) { - return false; - } - } - TokenTree::Sequence(_, ref seq) => { - if !check_lhs_duplicate_matcher_bindings(sess, &seq.tts, metavar_names, node_id) { - return false; - } - } - _ => {} - } - } - - true -} - fn check_rhs(sess: &ParseSess, rhs: "ed::TokenTree) -> bool { match *rhs { quoted::TokenTree::Delimited(..) => return true, diff --git a/src/libsyntax/ext/tt/quoted.rs b/src/libsyntax/ext/tt/quoted.rs index eff6b9dd429e4..f67e4d368cc8f 100644 --- a/src/libsyntax/ext/tt/quoted.rs +++ b/src/libsyntax/ext/tt/quoted.rs @@ -123,6 +123,22 @@ impl TokenTree { } } + /// Returns `true` if the given token tree is delimited. + pub fn is_delimited(&self) -> bool { + match *self { + TokenTree::Delimited(..) => true, + _ => false, + } + } + + /// Returns `true` if the given token tree is a token of the given kind. + pub fn is_token(&self, expected_kind: &TokenKind) -> bool { + match self { + TokenTree::Token(Token { kind: actual_kind, .. }) => actual_kind == expected_kind, + _ => false, + } + } + /// Gets the `index`-th sub-token-tree. This only makes sense for delimited trees and sequences. pub fn get_tt(&self, index: usize) -> TokenTree { match (self, index) { diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs index 337b84247361d..c92b0740f33ba 100644 --- a/src/libsyntax/lib.rs +++ b/src/libsyntax/lib.rs @@ -174,6 +174,7 @@ pub mod ext { pub mod tt { pub mod transcribe; + pub mod macro_check; pub mod macro_parser; pub mod macro_rules; pub mod quoted; diff --git a/src/test/run-pass/macros/meta-variable-misuse.rs b/src/test/run-pass/macros/meta-variable-misuse.rs new file mode 100644 index 0000000000000..99a2f940176bd --- /dev/null +++ b/src/test/run-pass/macros/meta-variable-misuse.rs @@ -0,0 +1,34 @@ +// run-pass +#![deny(meta_variable_misuse)] + +macro_rules! foo { + ($($m:ident $($f:ident $v:tt)+),*) => { + $($(macro_rules! $f { () => { $v } })+)* + $(macro_rules! $m { () => { $(fn $f() -> i32 { $v })+ } })* + } +} + +foo!(m a 1 b 2, n c 3); +m!(); +n!(); + +macro_rules! no_shadow { + ($x:tt) => { macro_rules! bar { ($x:tt) => { 42 }; } }; +} +no_shadow!(z); + +macro_rules! make_plus { + ($n: ident $x:expr) => { macro_rules! $n { ($y:expr) => { $x + $y }; } }; +} +make_plus!(add3 3); + +fn main() { + assert_eq!(a!(), 1); + assert_eq!(b!(), 2); + assert_eq!(c!(), 3); + assert_eq!(a(), 1); + assert_eq!(b(), 2); + assert_eq!(c(), 3); + assert_eq!(bar!(z:tt), 42); + assert_eq!(add3!(9), 12); +} diff --git a/src/test/ui/macros/issue-61053-different-kleene.rs b/src/test/ui/macros/issue-61053-different-kleene.rs new file mode 100644 index 0000000000000..9b7babdbb70ad --- /dev/null +++ b/src/test/ui/macros/issue-61053-different-kleene.rs @@ -0,0 +1,30 @@ +#![deny(meta_variable_misuse)] + +macro_rules! foo { + () => {}; + ( $( $i:ident = $($j:ident),+ );* ) => { $( $( $i = $j; )* )* }; + //~^ ERROR meta-variable repeats with + ( $( $($j:ident),+ );* ) => { $( $( $j; )+ )+ }; //~ERROR meta-variable repeats with +} + +macro_rules! bar { + () => {}; + (test) => { + macro_rules! nested { + () => {}; + ( $( $i:ident = $($j:ident),+ );* ) => { $( $( $i = $j; )* )* }; + //~^ ERROR meta-variable repeats with + ( $( $($j:ident),+ );* ) => { $( $( $j; )+ )+ }; //~ERROR meta-variable repeats with + } + }; + ( $( $i:ident = $($j:ident),+ );* ) => { + $(macro_rules! $i { + () => { 0 $( + $j )* }; //~ ERROR meta-variable repeats with + })* + }; +} + +fn main() { + foo!(); + bar!(); +} diff --git a/src/test/ui/macros/issue-61053-different-kleene.stderr b/src/test/ui/macros/issue-61053-different-kleene.stderr new file mode 100644 index 0000000000000..86474822a0c67 --- /dev/null +++ b/src/test/ui/macros/issue-61053-different-kleene.stderr @@ -0,0 +1,45 @@ +error: meta-variable repeats with different Kleene operator + --> $DIR/issue-61053-different-kleene.rs:5:57 + | +LL | ( $( $i:ident = $($j:ident),+ );* ) => { $( $( $i = $j; )* )* }; + | - expected repetition ^^ - conflicting repetition + | +note: lint level defined here + --> $DIR/issue-61053-different-kleene.rs:1:9 + | +LL | #![deny(meta_variable_misuse)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: meta-variable repeats with different Kleene operator + --> $DIR/issue-61053-different-kleene.rs:7:41 + | +LL | ( $( $($j:ident),+ );* ) => { $( $( $j; )+ )+ }; + | - ^^ - conflicting repetition + | | + | expected repetition + +error: meta-variable repeats with different Kleene operator + --> $DIR/issue-61053-different-kleene.rs:15:65 + | +LL | ( $( $i:ident = $($j:ident),+ );* ) => { $( $( $i = $j; )* )* }; + | - expected repetition ^^ - conflicting repetition + +error: meta-variable repeats with different Kleene operator + --> $DIR/issue-61053-different-kleene.rs:17:49 + | +LL | ( $( $($j:ident),+ );* ) => { $( $( $j; )+ )+ }; + | - ^^ - conflicting repetition + | | + | expected repetition + +error: meta-variable repeats with different Kleene operator + --> $DIR/issue-61053-different-kleene.rs:22:28 + | +LL | ( $( $i:ident = $($j:ident),+ );* ) => { + | - expected repetition +LL | $(macro_rules! $i { +LL | () => { 0 $( + $j )* }; + | ^^ - conflicting repetition + +error: aborting due to 5 previous errors + diff --git a/src/test/ui/macros/issue-61053-duplicate-binder.rs b/src/test/ui/macros/issue-61053-duplicate-binder.rs new file mode 100644 index 0000000000000..34aa571c11ee6 --- /dev/null +++ b/src/test/ui/macros/issue-61053-duplicate-binder.rs @@ -0,0 +1,14 @@ +#![deny(meta_variable_misuse)] + +macro_rules! foo { + () => {}; + (error) => { + macro_rules! bar { + ($x:tt $x:tt) => { $x }; //~ ERROR duplicate matcher binding + } + }; +} + +fn main() { + foo!(); +} diff --git a/src/test/ui/macros/issue-61053-duplicate-binder.stderr b/src/test/ui/macros/issue-61053-duplicate-binder.stderr new file mode 100644 index 0000000000000..fbd67b6c1e9c0 --- /dev/null +++ b/src/test/ui/macros/issue-61053-duplicate-binder.stderr @@ -0,0 +1,16 @@ +error: duplicate matcher binding + --> $DIR/issue-61053-duplicate-binder.rs:7:20 + | +LL | ($x:tt $x:tt) => { $x }; + | -- ^^ + | | + | previous declaration + | +note: lint level defined here + --> $DIR/issue-61053-duplicate-binder.rs:1:9 + | +LL | #![deny(meta_variable_misuse)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/macros/issue-61053-missing-repetition.rs b/src/test/ui/macros/issue-61053-missing-repetition.rs new file mode 100644 index 0000000000000..6b36c730b8254 --- /dev/null +++ b/src/test/ui/macros/issue-61053-missing-repetition.rs @@ -0,0 +1,28 @@ +#![deny(meta_variable_misuse)] + +macro_rules! foo { + () => {}; + ($( $i:ident = $($j:ident),+ );*) => { $( $i = $j; )* }; + //~^ ERROR variable 'j' is still repeating +} + +macro_rules! bar { + () => {}; + (test) => { + macro_rules! nested { + () => {}; + ($( $i:ident = $($j:ident),+ );*) => { $( $i = $j; )* }; + //~^ ERROR variable 'j' is still repeating + } + }; + ( $( $i:ident = $($j:ident),+ );* ) => { + $(macro_rules! $i { + () => { $j }; //~ ERROR variable 'j' is still repeating + })* + }; +} + +fn main() { + foo!(); + bar!(); +} diff --git a/src/test/ui/macros/issue-61053-missing-repetition.stderr b/src/test/ui/macros/issue-61053-missing-repetition.stderr new file mode 100644 index 0000000000000..6f89e276c169f --- /dev/null +++ b/src/test/ui/macros/issue-61053-missing-repetition.stderr @@ -0,0 +1,33 @@ +error: variable 'j' is still repeating at this depth + --> $DIR/issue-61053-missing-repetition.rs:5:52 + | +LL | ($( $i:ident = $($j:ident),+ );*) => { $( $i = $j; )* }; + | - ^^ + | | + | expected repetition + | +note: lint level defined here + --> $DIR/issue-61053-missing-repetition.rs:1:9 + | +LL | #![deny(meta_variable_misuse)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: variable 'j' is still repeating at this depth + --> $DIR/issue-61053-missing-repetition.rs:14:60 + | +LL | ($( $i:ident = $($j:ident),+ );*) => { $( $i = $j; )* }; + | - ^^ + | | + | expected repetition + +error: variable 'j' is still repeating at this depth + --> $DIR/issue-61053-missing-repetition.rs:20:21 + | +LL | ( $( $i:ident = $($j:ident),+ );* ) => { + | - expected repetition +LL | $(macro_rules! $i { +LL | () => { $j }; + | ^^ + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/macros/issue-61053-unbound.rs b/src/test/ui/macros/issue-61053-unbound.rs new file mode 100644 index 0000000000000..b75cdce0cf4bd --- /dev/null +++ b/src/test/ui/macros/issue-61053-unbound.rs @@ -0,0 +1,28 @@ +#![deny(meta_variable_misuse)] + +macro_rules! foo { + () => {}; + ($( $i:ident = $($j:ident),+ );*) => { $( $( $i = $k; )+ )* }; + //~^ ERROR unknown macro variable +} + +macro_rules! bar { + () => {}; + (test) => { + macro_rules! nested { + () => {}; + ($( $i:ident = $($j:ident),+ );*) => { $( $( $i = $k; )+ )* }; + //~^ ERROR unknown macro variable + } + }; + ( $( $i:ident = $($j:ident),+ );* ) => { + $(macro_rules! $i { + () => { $( $i = $k)+ }; //~ ERROR unknown macro variable + })* + }; +} + +fn main() { + foo!(); + bar!(); +} diff --git a/src/test/ui/macros/issue-61053-unbound.stderr b/src/test/ui/macros/issue-61053-unbound.stderr new file mode 100644 index 0000000000000..0fc0a7e283e92 --- /dev/null +++ b/src/test/ui/macros/issue-61053-unbound.stderr @@ -0,0 +1,26 @@ +error: unknown macro variable `k` + --> $DIR/issue-61053-unbound.rs:5:55 + | +LL | ($( $i:ident = $($j:ident),+ );*) => { $( $( $i = $k; )+ )* }; + | ^^ + | +note: lint level defined here + --> $DIR/issue-61053-unbound.rs:1:9 + | +LL | #![deny(meta_variable_misuse)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: unknown macro variable `k` + --> $DIR/issue-61053-unbound.rs:14:63 + | +LL | ($( $i:ident = $($j:ident),+ );*) => { $( $( $i = $k; )+ )* }; + | ^^ + +error: unknown macro variable `k` + --> $DIR/issue-61053-unbound.rs:20:29 + | +LL | () => { $( $i = $k)+ }; + | ^^ + +error: aborting due to 3 previous errors + From 390133e669be5dd57ce8b2fc8c369d38c04fa2f4 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Sat, 29 Jun 2019 10:00:34 +0200 Subject: [PATCH 3/3] DO NOT SUBMIT: Switch to deny-by-default for crater --- src/librustc/lint/builtin.rs | 2 +- src/test/ui/issues/issue-6596-1.rs | 2 +- src/test/ui/issues/issue-6596-1.stderr | 10 +++++++++- src/test/ui/issues/issue-6596-2.rs | 2 +- src/test/ui/issues/issue-6596-2.stderr | 10 +++++++++- 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index c881d92a471f5..5ffb04fc50a44 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -356,7 +356,7 @@ pub mod parser { declare_lint! { pub META_VARIABLE_MISUSE, - Allow, + Deny, "possible meta-variable misuse at macro definition" } } diff --git a/src/test/ui/issues/issue-6596-1.rs b/src/test/ui/issues/issue-6596-1.rs index 5da54451346a5..46a904903a23e 100644 --- a/src/test/ui/issues/issue-6596-1.rs +++ b/src/test/ui/issues/issue-6596-1.rs @@ -1,6 +1,6 @@ macro_rules! e { ($inp:ident) => ( - $nonexistent + $nonexistent //~ ERROR unknown macro variable `nonexistent` //~^ ERROR unknown macro variable `nonexistent` ); } diff --git a/src/test/ui/issues/issue-6596-1.stderr b/src/test/ui/issues/issue-6596-1.stderr index 2a4ece2f2425f..a2e24ce5353bd 100644 --- a/src/test/ui/issues/issue-6596-1.stderr +++ b/src/test/ui/issues/issue-6596-1.stderr @@ -7,5 +7,13 @@ LL | $nonexistent LL | e!(foo); | -------- in this macro invocation -error: aborting due to previous error +error: unknown macro variable `nonexistent` + --> $DIR/issue-6596-1.rs:3:9 + | +LL | $nonexistent + | ^^^^^^^^^^^^ + | + = note: #[deny(meta_variable_misuse)] on by default + +error: aborting due to 2 previous errors diff --git a/src/test/ui/issues/issue-6596-2.rs b/src/test/ui/issues/issue-6596-2.rs index b19700efe5ad3..ff0d0045db47a 100644 --- a/src/test/ui/issues/issue-6596-2.rs +++ b/src/test/ui/issues/issue-6596-2.rs @@ -2,7 +2,7 @@ macro_rules! g { ($inp:ident) => ( - { $inp $nonexistent } + { $inp $nonexistent } //~ ERROR unknown macro variable `nonexistent` //~^ ERROR unknown macro variable `nonexistent` ); } diff --git a/src/test/ui/issues/issue-6596-2.stderr b/src/test/ui/issues/issue-6596-2.stderr index 20fbe0fab0112..4cf4963a49169 100644 --- a/src/test/ui/issues/issue-6596-2.stderr +++ b/src/test/ui/issues/issue-6596-2.stderr @@ -7,5 +7,13 @@ LL | { $inp $nonexistent } LL | g!(foo); | -------- in this macro invocation -error: aborting due to previous error +error: unknown macro variable `nonexistent` + --> $DIR/issue-6596-2.rs:5:16 + | +LL | { $inp $nonexistent } + | ^^^^^^^^^^^^ + | + = note: #[deny(meta_variable_misuse)] on by default + +error: aborting due to 2 previous errors