From 47a0dfeb8846fe5d5293ea9b10e0a886624ecb96 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Sat, 26 Jan 2019 02:13:24 +0000 Subject: [PATCH] Lint duplicate trait and lifetime bounds. --- src/libcore/array.rs | 2 + src/librustc/lint/builtin.rs | 15 +++ src/librustc/traits/util.rs | 1 + src/librustc/ty/mod.rs | 22 +++- src/librustc_lint/builtin.rs | 2 +- src/librustc_lint/lib.rs | 2 +- src/librustc_typeck/astconv.rs | 13 ++- src/librustc_typeck/collect.rs | 103 +++++++++++++++--- .../lint-incoherent-auto-trait-objects.rs | 2 + .../lint-incoherent-auto-trait-objects.stderr | 18 +++ .../traits/trait-object-auto-dedup-in-impl.rs | 15 ++- .../trait-object-auto-dedup-in-impl.stderr | 36 +++++- 12 files changed, 203 insertions(+), 28 deletions(-) diff --git a/src/libcore/array.rs b/src/libcore/array.rs index 3a27a39af4ace..c0b7d41c24ebb 100644 --- a/src/libcore/array.rs +++ b/src/libcore/array.rs @@ -9,6 +9,8 @@ integer constants", issue = "27778")] +#![cfg_attr(not(stage0), allow(duplicate_bounds))] + use borrow::{Borrow, BorrowMut}; use cmp::Ordering; use convert::TryFrom; diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index a0bd4f01cd231..ca0bb4c78c8d1 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -346,6 +346,19 @@ declare_lint! { "outlives requirements can be inferred" } +declare_lint! { + pub DEPRECATED_IN_FUTURE, + Allow, + "detects use of items that will be deprecated in a future version", + report_in_external_macro: true +} + +declare_lint! { + pub DUPLICATE_BOUNDS, + Warn, + "detects duplicate bounds on type parameters, lifetime parameters, and projections" +} + /// Some lints that are buffered from `libsyntax`. See `syntax::early_buffered_lints`. pub mod parser { declare_lint! { @@ -440,6 +453,8 @@ impl LintPass for HardwiredLints { parser::ILL_FORMED_ATTRIBUTE_INPUT, DEPRECATED_IN_FUTURE, AMBIGUOUS_ASSOCIATED_ITEMS, + DUPLICATE_BOUNDS, + parser::QUESTION_MARK_MACRO_SEP, ) } } diff --git a/src/librustc/traits/util.rs b/src/librustc/traits/util.rs index 5b7ba5386725e..e4d7566dbba4f 100644 --- a/src/librustc/traits/util.rs +++ b/src/librustc/traits/util.rs @@ -1,3 +1,4 @@ +use errors::DiagnosticBuilder; use hir; use hir::def_id::DefId; use traits::specialize::specialization_graph::NodeItem; diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index c9089428b2324..08a382680e3cf 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1212,11 +1212,15 @@ impl<'tcx> PolyTraitPredicate<'tcx> { // ok to skip binder since trait def-id does not care about regions self.skip_binder().def_id() } + + pub fn self_ty(&self) -> Binder> { + self.map_bound(|p| p.self_ty()) + } } #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, RustcEncodable, RustcDecodable)] -pub struct OutlivesPredicate(pub A, pub B); // `A: B` -pub type PolyOutlivesPredicate = ty::Binder>; +pub struct OutlivesPredicate(pub A, pub B); // `A: B` +pub type PolyOutlivesPredicate = ty::Binder>; pub type RegionOutlivesPredicate<'tcx> = OutlivesPredicate, ty::Region<'tcx>>; pub type TypeOutlivesPredicate<'tcx> = OutlivesPredicate, @@ -1224,6 +1228,16 @@ pub type TypeOutlivesPredicate<'tcx> = OutlivesPredicate, pub type PolyRegionOutlivesPredicate<'tcx> = ty::Binder>; pub type PolyTypeOutlivesPredicate<'tcx> = ty::Binder>; +impl PolyOutlivesPredicate { + pub fn var(&self) -> Binder { + self.map_bound(|pred| pred.0) + } + + pub fn value(&self) -> Binder { + self.map_bound(|pred| pred.1) + } +} + #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)] pub struct SubtypePredicate<'tcx> { pub a_is_expected: bool, @@ -1265,11 +1279,11 @@ impl<'tcx> PolyProjectionPredicate<'tcx> { // This is because here `self` has a `Binder` and so does our // return value, so we are preserving the number of binding // levels. - self.map_bound(|predicate| predicate.projection_ty.trait_ref(tcx)) + self.map_bound(|pred| pred.projection_ty.trait_ref(tcx)) } pub fn ty(&self) -> Binder> { - self.map_bound(|predicate| predicate.ty) + self.map_bound(|pred| pred.ty) } /// The `DefId` of the `TraitItem` for the associated type. diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index f6c381ff74cc9..6b8187b522ead 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -47,7 +47,7 @@ use rustc::hir::{self, GenericParamKind, PatKind}; use nonstandard_style::{MethodLateContext, method_context}; -// hardwired lints from librustc +// Hardwired lints from librustc. pub use lint::builtin::*; declare_lint! { diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index d3d17184a0181..9e49eacefddf1 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -345,7 +345,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { reference: "issue #57644 ", edition: None, }, - ]); + ]); // Register renamed and removed lints. store.register_renamed("single_use_lifetime", "single_use_lifetimes"); diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index e89506aaf99f9..fed4c45e4d434 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -32,6 +32,7 @@ use std::collections::BTreeSet; use std::iter; use std::slice; +use crate::collect::{lint_duplicate_bounds, LintedBoundVar, LintedBoundValue}; use super::{check_type_alias_enum_variants_enabled}; use rustc_data_structures::fx::FxHashSet; @@ -940,6 +941,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { } fn conv_object_ty_poly_trait_ref(&self, + node_id: ast::NodeId, span: Span, trait_bounds: &[hir::PolyTraitRef], lifetime: &hir::Lifetime) @@ -980,6 +982,15 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { .emit(); } + // Lint duplicate bounds. + { + let bounds = trait_bounds.iter().filter_map(|b| { + let trait_did = b.trait_ref.trait_def_id(); + Some((LintedBoundVar::SelfTy, LintedBoundValue::DefId(trait_did), b.span)) + }); + lint_duplicate_bounds(tcx, node_id, bounds); + } + // Check that there are no gross object safety violations; // most importantly, that the supertraits don't contain `Self`, // to avoid ICEs. @@ -1767,7 +1778,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { tcx.mk_fn_ptr(self.ty_of_fn(bf.unsafety, bf.abi, &bf.decl)) } hir::TyKind::TraitObject(ref bounds, ref lifetime) => { - self.conv_object_ty_poly_trait_ref(ast_ty.span, bounds, lifetime) + self.conv_object_ty_poly_trait_ref(ast_ty.id, ast_ty.span, bounds, lifetime) } hir::TyKind::Path(hir::QPath::Resolved(ref maybe_qself, ref path)) => { debug!("ast_ty_to_ty: maybe_qself={:?} path={:?}", maybe_qself, path); diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 93f14a2ea6f64..d82f52ecdd90c 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -22,12 +22,11 @@ use middle::lang_items::SizedTraitLangItem; use middle::resolve_lifetime as rl; use middle::weak_lang_items; use rustc::mir::mono::Linkage; +use rustc::ty::{self, AdtKind, Binder, Predicate, ToPolyTraitRef, Ty, TyCtxt}; use rustc::ty::query::Providers; use rustc::ty::query::queries; use rustc::ty::subst::Substs; -use rustc::ty::util::Discr; -use rustc::ty::util::IntTypeExt; -use rustc::ty::{self, AdtKind, ToPolyTraitRef, Ty, TyCtxt}; +use rustc::ty::util::{Discr, IntTypeExt}; use rustc::ty::{ReprOptions, ToPredicate}; use rustc::util::captures::Captures; use rustc::util::nodemap::FxHashMap; @@ -50,9 +49,65 @@ use rustc::hir::GenericParamKind; use rustc::hir::{self, CodegenFnAttrFlags, CodegenFnAttrs, Unsafety}; use std::iter; +use std::ops::Range; struct OnlySelfBounds(bool); +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum LintedBoundVar<'tcx> { + DefId(DefId), + SelfTy, + Ty(Binder>), + Region(Binder>), +} + +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum LintedBoundValue<'tcx> { + DefId(DefId), + Ty(Binder>), + Region(Binder>), +} + +pub fn lint_duplicate_bounds<'a, 'gcx, 'tcx>( + tcx: TyCtxt<'a, 'gcx, 'tcx>, + node_id: ast::NodeId, + bounds: impl IntoIterator, LintedBoundValue<'tcx>, Span)>, +) { + let mut bounds: Vec<_> = bounds.into_iter().collect(); + bounds.sort_unstable(); + + let emit_lint = |range: Range| { + let bound_name = tcx.sess.source_map() + .span_to_snippet(bounds[range.start].2).unwrap(); + let mut err = tcx.struct_span_lint_node( + lint::builtin::DUPLICATE_BOUNDS, + node_id, + bounds[range.clone()].iter().map(|(_, _, sp)| *sp).collect::>(), + &format!("duplicate bound `{}` found", + bound_name)); + debug!("zzz: 1: {:?} / {:?}", bounds[range.start].0, bounds[range.start].1); + err.span_label(bounds[range.start].2, "first use of bound"); + for i in (range.start + 1)..range.end { + debug!("zzz: 2: {:?} / {:?}", bounds[i].0, bounds[i].1); + err.span_label(bounds[i].2, "subsequent use of bound"); + } + err.emit(); + }; + + let mut seq_start = 0; + for i in 1..bounds.len() { + if (&bounds[i].0, &bounds[i].1) != (&bounds[i - 1].0, &bounds[i - 1].1) { + if i - seq_start > 1 { + emit_lint(seq_start..i); + } + seq_start = i; + } + } + if bounds.len() - seq_start > 1 { + emit_lint(seq_start..bounds.len()); + } +} + /////////////////////////////////////////////////////////////////////////// // Main entry point @@ -336,7 +391,7 @@ impl<'a, 'tcx> ItemCtxt<'a, 'tcx> { param_id: ast::NodeId, ty: Ty<'tcx>, only_self_bounds: OnlySelfBounds, - ) -> Vec<(ty::Predicate<'tcx>, Span)> { + ) -> Vec<(Predicate<'tcx>, Span)> { let from_ty_params = ast_generics .params .iter() @@ -726,7 +781,7 @@ fn super_predicates_of<'a, 'tcx>( // which will, in turn, reach indirect supertraits. for &(pred, span) in &superbounds { debug!("superbound: {:?}", pred); - if let ty::Predicate::Trait(bound) = pred { + if let Predicate::Trait(bound) = pred { tcx.at(span).super_predicates_of(bound.def_id()); } } @@ -1672,8 +1727,8 @@ fn explicit_predicates_of<'a, 'tcx>( /// Preserving the order of insertion is important here so as not to break /// compile-fail UI tests. struct UniquePredicates<'tcx> { - predicates: Vec<(ty::Predicate<'tcx>, Span)>, - uniques: FxHashSet<(ty::Predicate<'tcx>, Span)>, + predicates: Vec<(Predicate<'tcx>, Span)>, + uniques: FxHashSet<(Predicate<'tcx>, Span)>, } impl<'tcx> UniquePredicates<'tcx> { @@ -1684,13 +1739,13 @@ fn explicit_predicates_of<'a, 'tcx>( } } - fn push(&mut self, value: (ty::Predicate<'tcx>, Span)) { + fn push(&mut self, value: (Predicate<'tcx>, Span)) { if self.uniques.insert(value) { self.predicates.push(value); } } - fn extend, Span)>>(&mut self, iter: I) { + fn extend, Span)>>(&mut self, iter: I) { for value in iter { self.push(value); } @@ -1884,7 +1939,7 @@ fn explicit_predicates_of<'a, 'tcx>( let span = bound_pred.bounded_ty.span; let predicate = ty::OutlivesPredicate(ty, tcx.mk_region(ty::ReEmpty)); predicates.push( - (ty::Predicate::TypeOutlives(ty::Binder::dummy(predicate)), span) + (Predicate::TypeOutlives(ty::Binder::dummy(predicate)), span) ); } } @@ -1910,7 +1965,7 @@ fn explicit_predicates_of<'a, 'tcx>( &hir::GenericBound::Outlives(ref lifetime) => { let region = AstConv::ast_region_to_region(&icx, lifetime, None); let pred = ty::Binder::bind(ty::OutlivesPredicate(ty, region)); - predicates.push((ty::Predicate::TypeOutlives(pred), lifetime.span)) + predicates.push((Predicate::TypeOutlives(pred), lifetime.span)) } } } @@ -1927,7 +1982,7 @@ fn explicit_predicates_of<'a, 'tcx>( }; let pred = ty::Binder::bind(ty::OutlivesPredicate(r1, r2)); - (ty::Predicate::RegionOutlives(pred), span) + (Predicate::RegionOutlives(pred), span) })) } @@ -1983,6 +2038,26 @@ fn explicit_predicates_of<'a, 'tcx>( ); } + // Lint duplicate bounds. + let bounds = predicates.iter().filter_map(|(pred, sp)| { + match pred { + Predicate::Trait(ref pred) => + Some((LintedBoundVar::Ty(pred.self_ty()), + LintedBoundValue::DefId(pred.def_id()), *sp)), + Predicate::RegionOutlives(ref pred) => + Some((LintedBoundVar::Region(pred.var()), + LintedBoundValue::Region(pred.value()), *sp)), + Predicate::TypeOutlives(ref pred) => + Some((LintedBoundVar::Ty(pred.var()), + LintedBoundValue::Region(pred.value()), *sp)), + Predicate::Projection(ref pred) => + Some((LintedBoundVar::DefId(pred.item_def_id()), + LintedBoundValue::Ty(pred.ty()), *sp)), + _ => None, + } + }); + lint_duplicate_bounds(tcx, node_id, bounds); + let result = Lrc::new(ty::GenericPredicates { parent: generics.parent, predicates, @@ -2062,7 +2137,7 @@ fn predicates_from_bound<'tcx>( astconv: &dyn AstConv<'tcx, 'tcx>, param_ty: Ty<'tcx>, bound: &hir::GenericBound, -) -> Vec<(ty::Predicate<'tcx>, Span)> { +) -> Vec<(Predicate<'tcx>, Span)> { match *bound { hir::GenericBound::Trait(ref tr, hir::TraitBoundModifier::None) => { let mut projections = Vec::new(); @@ -2076,7 +2151,7 @@ fn predicates_from_bound<'tcx>( hir::GenericBound::Outlives(ref lifetime) => { let region = astconv.ast_region_to_region(lifetime, None); let pred = ty::Binder::bind(ty::OutlivesPredicate(param_ty, region)); - vec![(ty::Predicate::TypeOutlives(pred), lifetime.span)] + vec![(Predicate::TypeOutlives(pred), lifetime.span)] } hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe) => vec![], } diff --git a/src/test/ui/lint/lint-incoherent-auto-trait-objects.rs b/src/test/ui/lint/lint-incoherent-auto-trait-objects.rs index 0d18965ee7338..38a95ce317a86 100644 --- a/src/test/ui/lint/lint-incoherent-auto-trait-objects.rs +++ b/src/test/ui/lint/lint-incoherent-auto-trait-objects.rs @@ -7,6 +7,7 @@ impl Foo for dyn Send {} impl Foo for dyn Send + Send {} //~^ ERROR conflicting implementations //~| hard error +//~^^^ WARNING duplicate auto trait `Send` found in type parameter bounds [duplicate_auto_traits_in_bounds] impl Foo for dyn Send + Sync {} @@ -17,5 +18,6 @@ impl Foo for dyn Sync + Send {} impl Foo for dyn Send + Sync + Send {} //~^ ERROR conflicting implementations //~| hard error +//~^^^ WARNING duplicate auto trait `Send` found in type parameter bounds [duplicate_auto_traits_in_bounds] fn main() {} diff --git a/src/test/ui/lint/lint-incoherent-auto-trait-objects.stderr b/src/test/ui/lint/lint-incoherent-auto-trait-objects.stderr index 928c92ef91655..9239a18dd003a 100644 --- a/src/test/ui/lint/lint-incoherent-auto-trait-objects.stderr +++ b/src/test/ui/lint/lint-incoherent-auto-trait-objects.stderr @@ -1,3 +1,21 @@ +warning: duplicate auto trait `Send` found in type parameter bounds + --> $DIR/lint-incoherent-auto-trait-objects.rs:7:18 + | +LL | impl Foo for dyn Send + Send {} + | ^^^^ ^^^^ subsequent use of auto trait + | | + | first use of auto trait + | + = note: #[warn(duplicate_auto_traits_in_bounds)] on by default + +warning: duplicate auto trait `Send` found in type parameter bounds + --> $DIR/lint-incoherent-auto-trait-objects.rs:18:18 + | +LL | impl Foo for dyn Send + Sync + Send {} + | ^^^^ ^^^^ subsequent use of auto trait + | | + | first use of auto trait + error: conflicting implementations of trait `Foo` for type `(dyn std::marker::Send + 'static)`: (E0119) --> $DIR/lint-incoherent-auto-trait-objects.rs:7:1 | diff --git a/src/test/ui/traits/trait-object-auto-dedup-in-impl.rs b/src/test/ui/traits/trait-object-auto-dedup-in-impl.rs index 6ba5d28a6c4c2..e474a810b797f 100644 --- a/src/test/ui/traits/trait-object-auto-dedup-in-impl.rs +++ b/src/test/ui/traits/trait-object-auto-dedup-in-impl.rs @@ -1,19 +1,30 @@ +// ignore-tidy-linelength + // Checks to make sure that `dyn Trait + Send` and `dyn Trait + Send + Send` are the same type. // Issue: #47010 struct Struct; + impl Trait for Struct {} + trait Trait {} type Send1 = Trait + Send; type Send2 = Trait + Send + Send; +//~^ WARNING duplicate auto trait `Send` found in type parameter bounds [duplicate_auto_traits_in_bounds] fn main () {} impl Trait + Send { - fn test(&self) { println!("one"); } //~ ERROR duplicate definitions with name `test` + fn test(&self) { + //~^ ERROR duplicate definitions with name `test` + println!("one"); + } } impl Trait + Send + Send { - fn test(&self) { println!("two"); } +//~^ WARNING duplicate auto trait `Send` found in type parameter bounds [duplicate_auto_traits_in_bounds] + fn test(&self) { + println!("two"); + } } diff --git a/src/test/ui/traits/trait-object-auto-dedup-in-impl.stderr b/src/test/ui/traits/trait-object-auto-dedup-in-impl.stderr index 2d2c3843548e9..959b906a36e2c 100644 --- a/src/test/ui/traits/trait-object-auto-dedup-in-impl.stderr +++ b/src/test/ui/traits/trait-object-auto-dedup-in-impl.stderr @@ -1,11 +1,37 @@ +warning: duplicate auto trait `Send` found in type parameter bounds + --> $DIR/trait-object-auto-dedup-in-impl.rs:13:22 + | +LL | type Send2 = Trait + Send + Send; + | ^^^^ ^^^^ subsequent use of auto trait + | | + | first use of auto trait + | + = note: #[warn(duplicate_auto_traits_in_bounds)] on by default + +warning: duplicate auto trait `Send` found in type parameter bounds + --> $DIR/trait-object-auto-dedup-in-impl.rs:25:14 + | +LL | impl Trait + Send + Send { + | ^^^^ ^^^^ subsequent use of auto trait + | | + | first use of auto trait + | + = 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 #56522 + error[E0592]: duplicate definitions with name `test` - --> $DIR/trait-object-auto-dedup-in-impl.rs:14:5 + --> $DIR/trait-object-auto-dedup-in-impl.rs:20:5 | -LL | fn test(&self) { println!("one"); } //~ ERROR duplicate definitions with name `test` - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ duplicate definitions for `test` +LL | / fn test(&self) { +LL | | //~^ ERROR duplicate definitions with name `test` +LL | | println!("one"); +LL | | } + | |_____^ duplicate definitions for `test` ... -LL | fn test(&self) { println!("two"); } - | ----------------------------------- other definition for `test` +LL | / fn test(&self) { +LL | | println!("two"); +LL | | } + | |_____- other definition for `test` error: aborting due to previous error