Skip to content

Commit

Permalink
make duplicate matcher bindings a hard error
Browse files Browse the repository at this point in the history
  • Loading branch information
mark-i-m committed Apr 11, 2019
1 parent 96d700f commit d7f5c50
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 59 deletions.
6 changes: 0 additions & 6 deletions src/librustc/lint/builtin.rs
Expand Up @@ -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! {
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/lint/mod.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
}

Expand Down
7 changes: 2 additions & 5 deletions src/librustc_lint/lib.rs
Expand Up @@ -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>",
Expand Down Expand Up @@ -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>) {
Expand Down
2 changes: 0 additions & 2 deletions src/libsyntax/early_buffered_lints.rs
Expand Up @@ -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`.
Expand Down
16 changes: 4 additions & 12 deletions src/libsyntax/ext/tt/macro_rules.rs
Expand Up @@ -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);
Expand Down
12 changes: 4 additions & 8 deletions src/test/ui/macros/macro-multiple-matcher-bindings.rs
@@ -1,16 +1,12 @@
// 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 {
Expand All @@ -19,8 +15,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() {}
57 changes: 33 additions & 24 deletions 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

0 comments on commit d7f5c50

Please sign in to comment.