Make it an incompatibility lint for now

rust-lang · Feb 7, 2019 · 802b256 · 802b256
1 parent 3e790a7
commit 802b256
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 56 deletions.
diff --git 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
@@ -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
@@ -354,6 +354,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
             reference: "issue #57644 <https://github.com/rust-lang/rust/issues/57644>",
             edition: None,
         },
+        FutureIncompatibleInfo {
+            id: LintId::of(DUPLICATE_MATCHER_BINDING_NAME),
+            reference: "issue #57593 <https://github.com/rust-lang/rust/issues/57593>",
+            edition: None,
+        },
         ]);
 
     // Register renamed and removed lints.

diff --git 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
@@ -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<Ident, Span>
+    metavar_names: &mut FxHashMap<Ident, Span>,
+    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
@@ -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
@@ -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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/57593>
 
-error: duplicate matcher binding
-  --> $DIR/macro-multiple-matcher-bindings.rs:15:18
-   |
-LL |     ($a:ident, $($a:ident),*) => {}; //~ERROR duplicate matcher binding
-   |                  ^^^^^^^^
+warning: duplicate matcher binding
+  --> src/test/ui/macros/macro-multiple-matcher-bindings.rs:17:6
    |
-note: previous declaration was here
-  --> $DIR/macro-multiple-matcher-bindings.rs:15:6
+LL |     ($a:ident, $($a:ident),*) => {}; //~WARN duplicate matcher binding
+   |      ^^^^^^^^    ^^^^^^^^
    |
-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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/57593>