Lint duplicate trait and lifetime bounds.

Author: Alexander Regueiro

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;

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,
         )
     }
 }

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;

src/librustc/ty/mod.rs

@@ -1212,18 +1212,32 @@ 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<Ty<'tcx>> {
+        self.map_bound(|p| p.self_ty())
+    }
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, RustcEncodable, RustcDecodable)]
-pub struct OutlivesPredicate<A,B>(pub A, pub B); // `A: B`
-pub type PolyOutlivesPredicate<A,B> = ty::Binder<OutlivesPredicate<A,B>>;
+pub struct OutlivesPredicate<A, B>(pub A, pub B); // `A: B`
+pub type PolyOutlivesPredicate<A, B> = ty::Binder<OutlivesPredicate<A, B>>;
 pub type RegionOutlivesPredicate<'tcx> = OutlivesPredicate<ty::Region<'tcx>,
                                                            ty::Region<'tcx>>;
 pub type TypeOutlivesPredicate<'tcx> = OutlivesPredicate<Ty<'tcx>,
                                                          ty::Region<'tcx>>;
 pub type PolyRegionOutlivesPredicate<'tcx> = ty::Binder<RegionOutlivesPredicate<'tcx>>;
 pub type PolyTypeOutlivesPredicate<'tcx> = ty::Binder<TypeOutlivesPredicate<'tcx>>;
 
+impl<A: Copy, B: Copy> PolyOutlivesPredicate<A, B> {
+    pub fn var(&self) -> Binder<A> {
+        self.map_bound(|pred| pred.0)
+    }
+
+    pub fn value(&self) -> Binder<B> {
+        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<Ty<'tcx>> {
-        self.map_bound(|predicate| predicate.ty)
+        self.map_bound(|pred| pred.ty)
     }
 
     /// The `DefId` of the `TraitItem` for the associated type.

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! {

src/librustc_lint/lib.rs

@@ -345,7 +345,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
             reference: "issue #57644 <https://github.com/rust-lang/rust/issues/57644>",
             edition: None,
         },
-        ]);
+    ]);
 
     // Register renamed and removed lints.
     store.register_renamed("single_use_lifetime", "single_use_lifetimes");

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);

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<Ty<'tcx>>),
+    Region(Binder<ty::Region<'tcx>>),
+}
+
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
+pub enum LintedBoundValue<'tcx> {
+    DefId(DefId),
+    Ty(Binder<Ty<'tcx>>),
+    Region(Binder<ty::Region<'tcx>>),
+}
+
+pub fn lint_duplicate_bounds<'a, 'gcx, 'tcx>(
+    tcx: TyCtxt<'a, 'gcx, 'tcx>,
+    node_id: ast::NodeId,
+    bounds: impl IntoIterator<Item = (LintedBoundVar<'tcx>, LintedBoundValue<'tcx>, Span)>,
+) {
+    let mut bounds: Vec<_> = bounds.into_iter().collect();
+    bounds.sort_unstable();
+
+    let emit_lint = |range: Range<usize>| {
+        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::<Vec<_>>(),
+            &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<I: IntoIterator<Item = (ty::Predicate<'tcx>, Span)>>(&mut self, iter: I) {
+        fn extend<I: IntoIterator<Item = (Predicate<'tcx>, 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![],
     }

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() {}

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
    |