Rollup merge of rust-lang#59858 - mark-i-m:dup-matcher-bindings-3, r=…

…Centril Make duplicate matcher bindings a hard error r? @Centril Closes rust-lang#57742
Centril · Apr 13, 2019 · abfca1e · abfca1e
2 parents 8af5c8f + e149dc0
commit abfca1e
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 61 deletions.
diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs
@@ -352,12 +352,6 @@ declare_lint! {
     "outlives requirements can be inferred"
 }
 
-declare_lint! {
-    pub DUPLICATE_MATCHER_BINDING_NAME,
-    Deny,
-    "duplicate macro matcher binding name"
-}
-
 /// Some lints that are buffered from `libsyntax`. See `syntax::early_buffered_lints`.
 pub mod parser {
     declare_lint! {
@@ -462,7 +456,6 @@ declare_lint_pass! {
         DEPRECATED_IN_FUTURE,
         AMBIGUOUS_ASSOCIATED_ITEMS,
         NESTED_IMPL_TRAIT,
-        DUPLICATE_MATCHER_BINDING_NAME,
         MUTABLE_BORROW_RESERVATION_CONFLICT,
     ]
 }

diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs
@@ -26,7 +26,7 @@ use rustc_data_structures::sync::{self, Lrc};
 use crate::hir::def_id::{CrateNum, LOCAL_CRATE};
 use crate::hir::intravisit;
 use crate::hir;
-use crate::lint::builtin::{BuiltinLintDiagnostics, DUPLICATE_MATCHER_BINDING_NAME};
+use crate::lint::builtin::BuiltinLintDiagnostics;
 use crate::lint::builtin::parser::{QUESTION_MARK_MACRO_SEP, ILL_FORMED_ATTRIBUTE_INPUT};
 use crate::session::{Session, DiagnosticMessageId};
 use crate::ty::TyCtxt;
@@ -82,7 +82,6 @@ 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
@@ -428,11 +428,6 @@ 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,
-        },
         FutureIncompatibleInfo {
             id: LintId::of(NESTED_IMPL_TRAIT),
             reference: "issue #59014 <https://github.com/rust-lang/rust/issues/59014>",
@@ -494,6 +489,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
         "no longer a warning, #[no_mangle] statics always exported");
     store.register_removed("bad_repr",
         "replaced with a generic attribute input check");
+    store.register_removed("duplicate_matcher_binding_name",
+        "converted into hard error, see https://github.com/rust-lang/rust/issues/57742");
 }
 
 pub fn register_internals(store: &mut lint::LintStore, sess: Option<&Session>) {

diff --git a/src/libsyntax/early_buffered_lints.rs b/src/libsyntax/early_buffered_lints.rs
@@ -12,8 +12,6 @@ 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
@@ -497,22 +497,14 @@ fn check_lhs_duplicate_matcher_bindings(
     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) {
-                    // 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"
-                    );
+                    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);

diff --git a/src/test/ui/macros/macro-multiple-matcher-bindings.rs b/src/test/ui/macros/macro-multiple-matcher-bindings.rs
@@ -1,16 +1,11 @@
 // Test that duplicate matcher binding names are caught at declaration time, rather than at macro
 // invocation time.
-//
-// FIXME(mark-i-m): Update this when it becomes a hard error.
-
-// compile-pass
 
 #![allow(unused_macros)]
-#![warn(duplicate_matcher_binding_name)]
 
 macro_rules! foo1 {
-    ($a:ident, $a:ident) => {}; //~WARNING duplicate matcher binding
-    ($a:ident, $a:path) => {};  //~WARNING duplicate matcher binding
+    ($a:ident, $a:ident) => {}; //~ERROR duplicate matcher binding
+    ($a:ident, $a:path) => {};  //~ERROR duplicate matcher binding
 }
 
 macro_rules! foo2 {
@@ -19,8 +14,8 @@ macro_rules! foo2 {
 }
 
 macro_rules! foo3 {
-    ($a:ident, $($a:ident),*) => {}; //~WARNING duplicate matcher binding
-    ($($a:ident)+ # $($($a:path),+);*) => {}; //~WARNING duplicate matcher binding
+    ($a:ident, $($a:ident),*) => {}; //~ERROR duplicate matcher binding
+    ($($a:ident)+ # $($($a:path),+);*) => {}; //~ERROR 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,41 +1,50 @@
-warning: duplicate matcher binding
-  --> $DIR/macro-multiple-matcher-bindings.rs:12:6
+error: duplicate matcher binding
+  --> $DIR/macro-multiple-matcher-bindings.rs:7:16
    |
 LL |     ($a:ident, $a:ident) => {};
-   |      ^^^^^^^^  ^^^^^^^^
+   |                ^^^^^^^^
    |
-note: lint level defined here
-  --> $DIR/macro-multiple-matcher-bindings.rs:9:9
+note: previous declaration was here
+  --> $DIR/macro-multiple-matcher-bindings.rs:7:6
    |
-LL | #![warn(duplicate_matcher_binding_name)]
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = 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>
+LL |     ($a:ident, $a:ident) => {};
+   |      ^^^^^^^^
 
-warning: duplicate matcher binding
-  --> $DIR/macro-multiple-matcher-bindings.rs:13:6
+error: duplicate matcher binding
+  --> $DIR/macro-multiple-matcher-bindings.rs:8:16
    |
 LL |     ($a:ident, $a:path) => {};
-   |      ^^^^^^^^  ^^^^^^^
+   |                ^^^^^^^
+   |
+note: previous declaration was here
+  --> $DIR/macro-multiple-matcher-bindings.rs:8:6
    |
-   = 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>
+LL |     ($a:ident, $a:path) => {};
+   |      ^^^^^^^^
 
-warning: duplicate matcher binding
-  --> $DIR/macro-multiple-matcher-bindings.rs:22:6
+error: duplicate matcher binding
+  --> $DIR/macro-multiple-matcher-bindings.rs:17:18
    |
 LL |     ($a:ident, $($a:ident),*) => {};
-   |      ^^^^^^^^    ^^^^^^^^
+   |                  ^^^^^^^^
+   |
+note: previous declaration was here
+  --> $DIR/macro-multiple-matcher-bindings.rs:17:6
    |
-   = 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>
+LL |     ($a:ident, $($a:ident),*) => {};
+   |      ^^^^^^^^
 
-warning: duplicate matcher binding
-  --> $DIR/macro-multiple-matcher-bindings.rs:23:8
+error: duplicate matcher binding
+  --> $DIR/macro-multiple-matcher-bindings.rs:18:25
    |
 LL |     ($($a:ident)+ # $($($a:path),+);*) => {};
-   |        ^^^^^^^^         ^^^^^^^
+   |                         ^^^^^^^
+   |
+note: previous declaration was here
+  --> $DIR/macro-multiple-matcher-bindings.rs:18:8
    |
-   = 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>
+LL |     ($($a:ident)+ # $($($a:path),+);*) => {};
+   |        ^^^^^^^^
+
+error: aborting due to 4 previous errors