From 802b256283fe7dd0f0e72499ba77d18f2838e817 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Thu, 17 Jan 2019 21:19:56 -0600 Subject: [PATCH] Make it an incompatibility lint for now --- src/librustc/lint/builtin.rs | 6 ++ src/librustc/lint/mod.rs | 3 +- src/librustc_lint/lib.rs | 5 ++ src/libsyntax/early_buffered_lints.rs | 2 + src/libsyntax/ext/tt/macro_rules.rs | 31 +++++--- .../macros/macro-multiple-matcher-bindings.rs | 10 +-- .../macro-multiple-matcher-bindings.stderr | 71 ++++++++----------- 7 files changed, 72 insertions(+), 56 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 6ae7448645a..3ff76e98d7b 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -352,6 +352,12 @@ declare_lint! { "outlives requirements can be inferred" } +declare_lint! { + pub DUPLICATE_MATCHER_BINDING_NAME, + Warn, + "duplicate macro matcher binding name" +} + /// Some lints that are buffered from `libsyntax`. See `syntax::early_buffered_lints`. pub mod parser { declare_lint! { diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 4e6bf753b01..8952ae98e59 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -27,7 +27,7 @@ use crate::errors::{DiagnosticBuilder, DiagnosticId}; use crate::hir::def_id::{CrateNum, LOCAL_CRATE}; use crate::hir::intravisit; use crate::hir; -use crate::lint::builtin::BuiltinLintDiagnostics; +use crate::lint::builtin::{BuiltinLintDiagnostics, DUPLICATE_MATCHER_BINDING_NAME}; use crate::lint::builtin::parser::{QUESTION_MARK_MACRO_SEP, ILL_FORMED_ATTRIBUTE_INPUT}; use crate::session::{Session, DiagnosticMessageId}; use std::{hash, ptr}; @@ -82,6 +82,7 @@ impl Lint { match lint_id { BufferedEarlyLintId::QuestionMarkMacroSep => QUESTION_MARK_MACRO_SEP, BufferedEarlyLintId::IllFormedAttributeInput => ILL_FORMED_ATTRIBUTE_INPUT, + BufferedEarlyLintId::DuplicateMacroMatcherBindingName => DUPLICATE_MATCHER_BINDING_NAME, } } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index fd5e68d5ae6..460cb977346 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -354,6 +354,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { reference: "issue #57644 ", edition: None, }, + FutureIncompatibleInfo { + id: LintId::of(DUPLICATE_MATCHER_BINDING_NAME), + reference: "issue #57593 ", + edition: None, + }, ]); // Register renamed and removed lints. diff --git a/src/libsyntax/early_buffered_lints.rs b/src/libsyntax/early_buffered_lints.rs index 977e6d45877..29cb9cd7f30 100644 --- a/src/libsyntax/early_buffered_lints.rs +++ b/src/libsyntax/early_buffered_lints.rs @@ -12,6 +12,8 @@ pub enum BufferedEarlyLintId { /// Usage of `?` as a macro separator is deprecated. QuestionMarkMacroSep, IllFormedAttributeInput, + /// Usage of a duplicate macro matcher binding name. + DuplicateMacroMatcherBindingName, } /// Stores buffered lint info which can later be passed to `librustc`. diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index 61a5b2cb0d2..33ea675f9d1 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -360,8 +360,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, &[lhs.clone()]); - valid &= - check_lhs_duplicate_matcher_bindings(sess, &[lhs.clone()], &mut FxHashMap::default()); + valid &= check_lhs_duplicate_matcher_bindings( + sess, + &[lhs.clone()], + &mut FxHashMap::default(), + def.id + ); } let expander: Box<_> = Box::new(MacroRulesMacroExpander { @@ -467,29 +471,38 @@ fn check_lhs_no_empty_seq(sess: &ParseSess, tts: &[quoted::TokenTree]) -> bool { fn check_lhs_duplicate_matcher_bindings( sess: &ParseSess, tts: &[quoted::TokenTree], - metavar_names: &mut FxHashMap + metavar_names: &mut FxHashMap, + node_id: ast::NodeId, ) -> bool { use self::quoted::TokenTree; + use crate::early_buffered_lints::BufferedEarlyLintId; 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(); + // FIXME(mark-i-m): in a few cycles, make this a hard error. + // sess.span_diagnostic + // .struct_span_err(span, "duplicate matcher binding") + // .span_note(prev_span, "previous declaration was here") + // .emit(); + sess.buffer_lint( + BufferedEarlyLintId::DuplicateMacroMatcherBindingName, + crate::source_map::MultiSpan::from(vec![prev_span, span]), + node_id, + "duplicate matcher binding" + ); return false; } else { metavar_names.insert(name, span); } } TokenTree::Delimited(_, ref del) => { - if !check_lhs_duplicate_matcher_bindings(sess, &del.tts, metavar_names) { + 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) { + if !check_lhs_duplicate_matcher_bindings(sess, &seq.tts, metavar_names, node_id) { return false; } } diff --git a/src/test/ui/macros/macro-multiple-matcher-bindings.rs b/src/test/ui/macros/macro-multiple-matcher-bindings.rs index ea57d66b565..3deae3eacec 100644 --- a/src/test/ui/macros/macro-multiple-matcher-bindings.rs +++ b/src/test/ui/macros/macro-multiple-matcher-bindings.rs @@ -1,9 +1,11 @@ // Test that duplicate matcher binding names are caught at declaration time, rather than at macro // invocation time. +#![allow(unused_macros)] + macro_rules! foo1 { - ($a:ident, $a:ident) => {}; //~ERROR duplicate matcher binding - ($a:ident, $a:path) => {}; //~ERROR duplicate matcher binding + ($a:ident, $a:ident) => {}; //~WARN duplicate matcher binding + ($a:ident, $a:path) => {}; //~WARN duplicate matcher binding } macro_rules! foo2 { @@ -12,8 +14,8 @@ macro_rules! foo2 { } macro_rules! foo3 { - ($a:ident, $($a:ident),*) => {}; //~ERROR duplicate matcher binding - ($($a:ident)+ # $($($a:path),+);*) => {}; //~ERROR duplicate matcher binding + ($a:ident, $($a:ident),*) => {}; //~WARN duplicate matcher binding + ($($a:ident)+ # $($($a:path),+);*) => {}; //~WARN duplicate matcher binding } fn main() {} diff --git a/src/test/ui/macros/macro-multiple-matcher-bindings.stderr b/src/test/ui/macros/macro-multiple-matcher-bindings.stderr index 3e3e1024587..04b27e88a1f 100644 --- a/src/test/ui/macros/macro-multiple-matcher-bindings.stderr +++ b/src/test/ui/macros/macro-multiple-matcher-bindings.stderr @@ -1,50 +1,37 @@ -error: duplicate matcher binding - --> $DIR/macro-multiple-matcher-bindings.rs:5:16 - | -LL | ($a:ident, $a:ident) => {}; //~ERROR duplicate matcher binding - | ^^^^^^^^ - | -note: previous declaration was here - --> $DIR/macro-multiple-matcher-bindings.rs:5:6 - | -LL | ($a:ident, $a:ident) => {}; //~ERROR duplicate matcher binding - | ^^^^^^^^ +warning: duplicate matcher binding + --> src/test/ui/macros/macro-multiple-matcher-bindings.rs:7:6 + | +7 | ($a:ident, $a:ident) => {}; //~WARN duplicate matcher binding + | ^^^^^^^^ ^^^^^^^^ + | + = note: #[warn(duplicate_matcher_binding_name)] on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57593 -error: duplicate matcher binding - --> $DIR/macro-multiple-matcher-bindings.rs:6:16 - | -LL | ($a:ident, $a:path) => {}; //~ERROR duplicate matcher binding - | ^^^^^^^ - | -note: previous declaration was here - --> $DIR/macro-multiple-matcher-bindings.rs:6:6 - | -LL | ($a:ident, $a:path) => {}; //~ERROR duplicate matcher binding - | ^^^^^^^^ +warning: duplicate matcher binding + --> src/test/ui/macros/macro-multiple-matcher-bindings.rs:8:6 + | +8 | ($a:ident, $a:path) => {}; //~WARN duplicate matcher binding + | ^^^^^^^^ ^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57593 -error: duplicate matcher binding - --> $DIR/macro-multiple-matcher-bindings.rs:15:18 +warning: duplicate matcher binding + --> src/test/ui/macros/macro-multiple-matcher-bindings.rs:17:6 | -LL | ($a:ident, $($a:ident),*) => {}; //~ERROR duplicate matcher binding - | ^^^^^^^^ +LL | ($a:ident, $($a:ident),*) => {}; //~WARN duplicate matcher binding + | ^^^^^^^^ ^^^^^^^^ | -note: previous declaration was here - --> $DIR/macro-multiple-matcher-bindings.rs:15:6 - | -LL | ($a:ident, $($a:ident),*) => {}; //~ERROR duplicate matcher binding - | ^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57593 -error: duplicate matcher binding - --> $DIR/macro-multiple-matcher-bindings.rs:16:25 +warning: duplicate matcher binding + --> src/test/ui/macros/macro-multiple-matcher-bindings.rs:18:8 | -LL | ($($a:ident)+ # $($($a:path),+);*) => {}; //~ERROR duplicate matcher binding - | ^^^^^^^ +LL | ($($a:ident)+ # $($($a:path),+);*) => {}; //~WARN duplicate matcher binding + | ^^^^^^^^ ^^^^^^^ | -note: previous declaration was here - --> $DIR/macro-multiple-matcher-bindings.rs:16:8 - | -LL | ($($a:ident)+ # $($($a:path),+);*) => {}; //~ERROR duplicate matcher binding - | ^^^^^^^^ - -error: aborting due to 4 previous errors + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57593