From 4bb39966a670864de0078cafc1e5b623ca1b2ec3 Mon Sep 17 00:00:00 2001 From: F001 Date: Sat, 19 May 2018 00:17:13 +0800 Subject: [PATCH 1/4] re-implement --- src/librustc/lint/builtin.rs | 7 ++++++ src/librustc_typeck/astconv.rs | 21 +++++++++++++---- .../issue-50589-multiple-associated-types.rs | 23 +++++++++++++++++++ ...sue-50589-multiple-associated-types.stderr | 14 +++++++++++ 4 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/lint/issue-50589-multiple-associated-types.rs create mode 100644 src/test/ui/lint/issue-50589-multiple-associated-types.stderr diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index ac2fa8515bc1d..1cb00abbb7e39 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -279,6 +279,12 @@ declare_lint! { "detects labels that are never used" } +declare_lint! { + pub DUPLICATE_ASSOCIATED_TYPE_BINDING, + Warn, + "warns about duplicate associated type bindings in generics" +} + /// Does nothing as a lint pass, but registers some `Lint`s /// which are used by other parts of the compiler. #[derive(Copy, Clone)] @@ -330,6 +336,7 @@ impl LintPass for HardwiredLints { BARE_TRAIT_OBJECT, ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE, UNSTABLE_NAME_COLLISION, + DUPLICATE_ASSOCIATED_TYPE_BINDING, ) } } diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index c1868467503f8..091ed0a4b367f 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -27,7 +27,7 @@ use rustc_target::spec::abi; use std::slice; use require_c_abi_if_variadic; use util::common::ErrorReported; -use util::nodemap::FxHashSet; +use util::nodemap::{FxHashSet, FxHashMap}; use errors::FatalError; use std::iter; @@ -398,13 +398,24 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { trait_ref.path.segments.last().unwrap()); let poly_trait_ref = ty::Binder::bind(ty::TraitRef::new(trait_def_id, substs)); + let mut dup_bindings = FxHashMap::default(); poly_projections.extend(assoc_bindings.iter().filter_map(|binding| { // specify type to assert that error was already reported in Err case: let predicate: Result<_, ErrorReported> = - self.ast_type_binding_to_poly_projection_predicate(trait_ref.ref_id, poly_trait_ref, - binding, speculative); + self.ast_type_binding_to_poly_projection_predicate( + trait_ref.ref_id, poly_trait_ref, binding, speculative, &mut dup_bindings); predicate.ok() // ok to ignore Err() because ErrorReported (see above) })); + for (_id, spans) in dup_bindings { + if spans.len() > 1 { + self.tcx().struct_span_lint_node( + ::rustc::lint::builtin::DUPLICATE_ASSOCIATED_TYPE_BINDING, + trait_ref.ref_id, + spans, + "duplicate associated type binding" + ).emit(); + } + } debug!("ast_path_to_poly_trait_ref({:?}, projections={:?}) -> {:?}", trait_ref, poly_projections, poly_trait_ref); @@ -487,7 +498,8 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { ref_id: ast::NodeId, trait_ref: ty::PolyTraitRef<'tcx>, binding: &ConvertedBinding<'tcx>, - speculative: bool) + speculative: bool, + dup_bindings: &mut FxHashMap>) -> Result, ErrorReported> { let tcx = self.tcx(); @@ -565,6 +577,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { tcx.sess.span_err(binding.span, &msg); } tcx.check_stability(assoc_ty.def_id, Some(ref_id), binding.span); + dup_bindings.entry(assoc_ty.def_id).or_insert(Vec::new()).push(binding.span); Ok(candidate.map_bound(|trait_ref| { ty::ProjectionPredicate { diff --git a/src/test/ui/lint/issue-50589-multiple-associated-types.rs b/src/test/ui/lint/issue-50589-multiple-associated-types.rs new file mode 100644 index 0000000000000..2c789a139cd3a --- /dev/null +++ b/src/test/ui/lint/issue-50589-multiple-associated-types.rs @@ -0,0 +1,23 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-pass + +use std::iter::Iterator; + +type Unit = (); + +fn test() -> Box> { + Box::new(None.into_iter()) +} + +fn main() { + test(); +} diff --git a/src/test/ui/lint/issue-50589-multiple-associated-types.stderr b/src/test/ui/lint/issue-50589-multiple-associated-types.stderr new file mode 100644 index 0000000000000..05e02879f1f50 --- /dev/null +++ b/src/test/ui/lint/issue-50589-multiple-associated-types.stderr @@ -0,0 +1,14 @@ +warning: duplicate associated type binding + --> $DIR/issue-50589-multiple-associated-types.rs:17:28 + | +LL | fn test() -> Box> { + | ^^^^^^^^^ ^^^^^^^^^^^ + | + = note: #[warn(duplicate_associated_type_binding)] on by default + +warning: duplicate associated type binding + --> $DIR/issue-50589-multiple-associated-types.rs:17:28 + | +LL | fn test() -> Box> { + | ^^^^^^^^^ ^^^^^^^^^^^ + From c3322556f50fbdcdbd235e5d15406f1dea77048c Mon Sep 17 00:00:00 2001 From: F001 Date: Sat, 19 May 2018 22:47:34 +0800 Subject: [PATCH 2/4] Fix according to comments --- src/librustc/lint/builtin.rs | 4 +-- src/librustc_lint/lib.rs | 5 ++++ src/librustc_typeck/astconv.rs | 28 +++++++++++-------- ...sue-50589-multiple-associated-types.stderr | 23 ++++++++++----- 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 1cb00abbb7e39..5d7d2f0f9e698 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -280,7 +280,7 @@ declare_lint! { } declare_lint! { - pub DUPLICATE_ASSOCIATED_TYPE_BINDING, + pub DUPLICATE_ASSOCIATED_TYPE_BINDINGS, Warn, "warns about duplicate associated type bindings in generics" } @@ -336,7 +336,7 @@ impl LintPass for HardwiredLints { BARE_TRAIT_OBJECT, ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE, UNSTABLE_NAME_COLLISION, - DUPLICATE_ASSOCIATED_TYPE_BINDING, + DUPLICATE_ASSOCIATED_TYPE_BINDINGS, ) } } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index cfd5cf5a0f98c..e35537459b70f 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -283,6 +283,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { reference: "issue TBD", edition: Some(Edition::Edition2018), }, + FutureIncompatibleInfo { + id: LintId::of(DUPLICATE_ASSOCIATED_TYPE_BINDINGS), + reference: "issue #50589 ", + edition: None, + }, ]); // Register renamed and removed lints diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 091ed0a4b367f..677244b612a82 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -406,16 +406,6 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { trait_ref.ref_id, poly_trait_ref, binding, speculative, &mut dup_bindings); predicate.ok() // ok to ignore Err() because ErrorReported (see above) })); - for (_id, spans) in dup_bindings { - if spans.len() > 1 { - self.tcx().struct_span_lint_node( - ::rustc::lint::builtin::DUPLICATE_ASSOCIATED_TYPE_BINDING, - trait_ref.ref_id, - spans, - "duplicate associated type binding" - ).emit(); - } - } debug!("ast_path_to_poly_trait_ref({:?}, projections={:?}) -> {:?}", trait_ref, poly_projections, poly_trait_ref); @@ -499,7 +489,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { trait_ref: ty::PolyTraitRef<'tcx>, binding: &ConvertedBinding<'tcx>, speculative: bool, - dup_bindings: &mut FxHashMap>) + dup_bindings: &mut FxHashMap) -> Result, ErrorReported> { let tcx = self.tcx(); @@ -577,7 +567,21 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { tcx.sess.span_err(binding.span, &msg); } tcx.check_stability(assoc_ty.def_id, Some(ref_id), binding.span); - dup_bindings.entry(assoc_ty.def_id).or_insert(Vec::new()).push(binding.span); + + dup_bindings.entry(assoc_ty.def_id) + .and_modify(|prev_span| { + let mut err = self.tcx().struct_span_lint_node( + ::rustc::lint::builtin::DUPLICATE_ASSOCIATED_TYPE_BINDINGS, + ref_id, + binding.span, + &format!("associated type binding `{}` specified more than once", + binding.item_name) + ); + err.span_label(binding.span, "used more than once"); + err.span_label(*prev_span, format!("first use of `{}`", binding.item_name)); + err.emit(); + }) + .or_insert(binding.span); Ok(candidate.map_bound(|trait_ref| { ty::ProjectionPredicate { diff --git a/src/test/ui/lint/issue-50589-multiple-associated-types.stderr b/src/test/ui/lint/issue-50589-multiple-associated-types.stderr index 05e02879f1f50..7f0a1ee1f3307 100644 --- a/src/test/ui/lint/issue-50589-multiple-associated-types.stderr +++ b/src/test/ui/lint/issue-50589-multiple-associated-types.stderr @@ -1,14 +1,23 @@ -warning: duplicate associated type binding - --> $DIR/issue-50589-multiple-associated-types.rs:17:28 +warning: associated type binding `Item` specified more than once + --> $DIR/issue-50589-multiple-associated-types.rs:17:39 | LL | fn test() -> Box> { - | ^^^^^^^^^ ^^^^^^^^^^^ + | --------- ^^^^^^^^^^^ used more than once + | | + | first use of `Item` | - = note: #[warn(duplicate_associated_type_binding)] on by default + = note: #[warn(duplicate_associated_type_bindings)] 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 #50589 -warning: duplicate associated type binding - --> $DIR/issue-50589-multiple-associated-types.rs:17:28 +warning: associated type binding `Item` specified more than once + --> $DIR/issue-50589-multiple-associated-types.rs:17:39 | LL | fn test() -> Box> { - | ^^^^^^^^^ ^^^^^^^^^^^ + | --------- ^^^^^^^^^^^ used more than once + | | + | first use of `Item` + | + = 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 #50589 From f837c34a865c46faef122e2c65b5f78c3fb954f8 Mon Sep 17 00:00:00 2001 From: F001 Date: Sun, 20 May 2018 10:08:54 +0800 Subject: [PATCH 3/4] avoid reporting twice --- src/librustc_typeck/astconv.rs | 30 ++++++++++--------- ...sue-50589-multiple-associated-types.stderr | 11 ------- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 677244b612a82..35d5619969dbb 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -568,20 +568,22 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { } tcx.check_stability(assoc_ty.def_id, Some(ref_id), binding.span); - dup_bindings.entry(assoc_ty.def_id) - .and_modify(|prev_span| { - let mut err = self.tcx().struct_span_lint_node( - ::rustc::lint::builtin::DUPLICATE_ASSOCIATED_TYPE_BINDINGS, - ref_id, - binding.span, - &format!("associated type binding `{}` specified more than once", - binding.item_name) - ); - err.span_label(binding.span, "used more than once"); - err.span_label(*prev_span, format!("first use of `{}`", binding.item_name)); - err.emit(); - }) - .or_insert(binding.span); + if speculative { + dup_bindings.entry(assoc_ty.def_id) + .and_modify(|prev_span| { + let mut err = self.tcx().struct_span_lint_node( + ::rustc::lint::builtin::DUPLICATE_ASSOCIATED_TYPE_BINDINGS, + ref_id, + binding.span, + &format!("associated type binding `{}` specified more than once", + binding.item_name) + ); + err.span_label(binding.span, "used more than once"); + err.span_label(*prev_span, format!("first use of `{}`", binding.item_name)); + err.emit(); + }) + .or_insert(binding.span); + } Ok(candidate.map_bound(|trait_ref| { ty::ProjectionPredicate { diff --git a/src/test/ui/lint/issue-50589-multiple-associated-types.stderr b/src/test/ui/lint/issue-50589-multiple-associated-types.stderr index 7f0a1ee1f3307..e115e523d8735 100644 --- a/src/test/ui/lint/issue-50589-multiple-associated-types.stderr +++ b/src/test/ui/lint/issue-50589-multiple-associated-types.stderr @@ -10,14 +10,3 @@ LL | fn test() -> Box> { = 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 #50589 -warning: associated type binding `Item` specified more than once - --> $DIR/issue-50589-multiple-associated-types.rs:17:39 - | -LL | fn test() -> Box> { - | --------- ^^^^^^^^^^^ used more than once - | | - | first use of `Item` - | - = 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 #50589 - From 88f810f52e628795875260a31d479af4f28a5edb Mon Sep 17 00:00:00 2001 From: F001 Date: Wed, 23 May 2018 08:59:08 +0800 Subject: [PATCH 4/4] inverting speculative flag --- src/librustc_typeck/astconv.rs | 2 +- .../lint/issue-50589-multiple-associated-types.stderr | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 35d5619969dbb..8fe5b74ccb95b 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -568,7 +568,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { } tcx.check_stability(assoc_ty.def_id, Some(ref_id), binding.span); - if speculative { + if !speculative { dup_bindings.entry(assoc_ty.def_id) .and_modify(|prev_span| { let mut err = self.tcx().struct_span_lint_node( diff --git a/src/test/ui/lint/issue-50589-multiple-associated-types.stderr b/src/test/ui/lint/issue-50589-multiple-associated-types.stderr index e115e523d8735..7f0a1ee1f3307 100644 --- a/src/test/ui/lint/issue-50589-multiple-associated-types.stderr +++ b/src/test/ui/lint/issue-50589-multiple-associated-types.stderr @@ -10,3 +10,14 @@ LL | fn test() -> Box> { = 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 #50589 +warning: associated type binding `Item` specified more than once + --> $DIR/issue-50589-multiple-associated-types.rs:17:39 + | +LL | fn test() -> Box> { + | --------- ^^^^^^^^^^^ used more than once + | | + | first use of `Item` + | + = 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 #50589 +