Rollup merge of rust-lang#43426 - qnighy:intercrate-ambiguity-hints, r=nikomatsakis

Add hints when intercrate ambiguity causes overlap.

I'm going to tackle rust-lang#23980.

# Examples

## Trait impl overlap caused by possible downstream impl

```rust
trait Foo<X> {}
trait Bar<X> {}
impl<X, T> Foo<X> for T where T: Bar<X> {}
impl<X> Foo<X> for i32 {}

fn main() {}
```

```
error[E0119]: conflicting implementations of trait `Foo<_>` for type `i32`:
 --> test1.rs:4:1
  |
3 | impl<X, T> Foo<X> for T where T: Bar<X> {}
  | ------------------------------------------ first implementation here
4 | impl<X> Foo<X> for i32 {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `i32`
  |
  = note: downstream crates may implement Bar

error: aborting due to previous error
```

## Trait impl overlap caused by possible upstream update

```rust
trait Foo {}
impl<T> Foo for T where T: ::std::fmt::Octal {}
impl Foo for () {}

fn main() {}
```

```
error[E0119]: conflicting implementations of trait `Foo` for type `()`:
 --> test2.rs:3:1
  |
2 | impl<T> Foo for T where T: ::std::fmt::Octal {}
  | ----------------------------------------------- first implementation here
3 | impl Foo for () {}
  | ^^^^^^^^^^^^^^^^^^ conflicting implementation for `()`
  |
  = note: upstream crates may add new impl for std::fmt::Octal in future versions

error: aborting due to previous error
```

## Inherent impl overlap caused by possible downstream impl

```rust
trait Bar<X> {}

struct A<T, X>(T, X);
impl<X, T> A<T, X> where T: Bar<X> { fn f(&self) {} }
impl<X> A<i32, X> { fn f(&self) {} }

fn main() {}
```

```
error[E0592]: duplicate definitions with name `f`
 --> test3.rs:4:38
  |
4 | impl<X, T> A<T, X> where T: Bar<X> { fn f(&self) {} }
  |                                      ^^^^^^^^^^^^^^ duplicate definitions for `f`
5 | impl<X> A<i32, X> { fn f(&self) {} }
  |                     -------------- other definition for `f`
  |
  = note: downstream crates may implement Bar

error: aborting due to previous error
```

## Inherent impl overlap caused by possible upstream update

```rust
struct A<T>(T);

impl<T> A<T> where T: ::std::fmt::Octal { fn f(&self) {} }
impl A<()> { fn f(&self) {} }

fn main() {}
```

```
error[E0592]: duplicate definitions with name `f`
 --> test4.rs:3:43
  |
3 | impl<T> A<T> where T: ::std::fmt::Octal { fn f(&self) {} }
  |                                           ^^^^^^^^^^^^^^ duplicate definitions for `f`
4 | impl A<()> { fn f(&self) {} }
  |              -------------- other definition for `f`
  |
  = note: upstream crates may add new impl for std::fmt::Octal in future versions

error: aborting due to previous error
```

Author: frewsxcv

src/librustc/traits/coherence.rs

@@ -13,6 +13,7 @@
 use hir::def_id::{DefId, LOCAL_CRATE};
 use syntax_pos::DUMMY_SP;
 use traits::{self, Normalized, SelectionContext, Obligation, ObligationCause, Reveal};
+use traits::select::IntercrateAmbiguityCause;
 use ty::{self, Ty, TyCtxt};
 use ty::subst::Subst;
 
@@ -21,12 +22,17 @@ use infer::{InferCtxt, InferOk};
 #[derive(Copy, Clone)]
 struct InferIsLocal(bool);
 
+pub struct OverlapResult<'tcx> {
+    pub impl_header: ty::ImplHeader<'tcx>,
+    pub intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
+}
+
 /// If there are types that satisfy both impls, returns a suitably-freshened
 /// `ImplHeader` with those types substituted
 pub fn overlapping_impls<'cx, 'gcx, 'tcx>(infcx: &InferCtxt<'cx, 'gcx, 'tcx>,
                                           impl1_def_id: DefId,
                                           impl2_def_id: DefId)
-                                          -> Option<ty::ImplHeader<'tcx>>
+                                          -> Option<OverlapResult<'tcx>>
 {
     debug!("impl_can_satisfy(\
            impl1_def_id={:?}, \
@@ -65,7 +71,7 @@ fn with_fresh_ty_vars<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, '
 fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
                             a_def_id: DefId,
                             b_def_id: DefId)
-                            -> Option<ty::ImplHeader<'tcx>>
+                            -> Option<OverlapResult<'tcx>>
 {
     debug!("overlap(a_def_id={:?}, b_def_id={:?})",
            a_def_id,
@@ -113,7 +119,10 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
         return None
     }
 
-    Some(selcx.infcx().resolve_type_vars_if_possible(&a_impl_header))
+    Some(OverlapResult {
+        impl_header: selcx.infcx().resolve_type_vars_if_possible(&a_impl_header),
+        intercrate_ambiguity_causes: selcx.intercrate_ambiguity_causes().to_vec(),
+    })
 }
 
 pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
@@ -131,10 +140,7 @@ pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
     // if the trait is not marked fundamental, then it's always possible that
     // an ancestor crate will impl this in the future, if they haven't
     // already
-    if
-        trait_ref.def_id.krate != LOCAL_CRATE &&
-        !tcx.has_attr(trait_ref.def_id, "fundamental")
-    {
+    if !trait_ref_is_local_or_fundamental(tcx, trait_ref) {
         debug!("trait_ref_is_knowable: trait is neither local nor fundamental");
         return false;
     }
@@ -148,6 +154,11 @@ pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
     orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(true)).is_err()
 }
 
+pub fn trait_ref_is_local_or_fundamental<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
+                                                         trait_ref: &ty::TraitRef<'tcx>) {
+    trait_ref.def_id.krate == LOCAL_CRATE || tcx.has_attr(trait_ref.def_id, "fundamental")
+}
+
 pub enum OrphanCheckErr<'tcx> {
     NoLocalInputType,
     UncoveredTy(Ty<'tcx>),

src/librustc/traits/mod.rs

@@ -28,16 +28,15 @@ use std::rc::Rc;
 use syntax::ast;
 use syntax_pos::{Span, DUMMY_SP};
 
-pub use self::coherence::orphan_check;
-pub use self::coherence::overlapping_impls;
-pub use self::coherence::OrphanCheckErr;
+pub use self::coherence::{orphan_check, overlapping_impls, OrphanCheckErr, OverlapResult};
 pub use self::fulfill::{FulfillmentContext, RegionObligation};
 pub use self::project::MismatchedProjectionTypes;
 pub use self::project::{normalize, normalize_projection_type, Normalized};
 pub use self::project::{ProjectionCache, ProjectionCacheSnapshot, Reveal};
 pub use self::object_safety::ObjectSafetyViolation;
 pub use self::object_safety::MethodViolationCode;
 pub use self::select::{EvaluationCache, SelectionContext, SelectionCache};
+pub use self::select::IntercrateAmbiguityCause;
 pub use self::specialize::{OverlapError, specialization_graph, specializes, translate_substs};
 pub use self::specialize::{SpecializesCache, find_associated_item};
 pub use self::util::elaborate_predicates;

src/librustc/traits/select.rs

@@ -31,7 +31,7 @@ use super::{VtableImplData, VtableObjectData, VtableBuiltinData, VtableGenerator
 use super::util;
 
 use dep_graph::{DepNodeIndex, DepKind};
-use hir::def_id::DefId;
+use hir::def_id::{DefId, LOCAL_CRATE};
 use infer;
 use infer::{InferCtxt, InferOk, TypeFreshener};
 use ty::subst::{Kind, Subst, Substs};
@@ -91,6 +91,45 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {
     intercrate: bool,
 
     inferred_obligations: SnapshotVec<InferredObligationsSnapshotVecDelegate<'tcx>>,
+
+    intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
+}
+
+#[derive(Clone)]
+pub enum IntercrateAmbiguityCause {
+    DownstreamCrate {
+        trait_desc: String,
+        self_desc: Option<String>,
+    },
+    UpstreamCrateUpdate {
+        trait_desc: String,
+        self_desc: Option<String>,
+    },
+}
+
+impl IntercrateAmbiguityCause {
+    /// Emits notes when the overlap is caused by complex intercrate ambiguities.
+    /// See #23980 for details.
+    pub fn add_intercrate_ambiguity_hint<'a, 'tcx>(&self,
+                                                   err: &mut ::errors::DiagnosticBuilder) {
+        match self {
+            &IntercrateAmbiguityCause::DownstreamCrate { ref trait_desc, ref self_desc } => {
+                let self_desc = if let &Some(ref ty) = self_desc {
+                    format!(" for type `{}`", ty)
+                } else { "".to_string() };
+                err.note(&format!("downstream crates may implement trait `{}`{}",
+                                  trait_desc, self_desc));
+            }
+            &IntercrateAmbiguityCause::UpstreamCrateUpdate { ref trait_desc, ref self_desc } => {
+                let self_desc = if let &Some(ref ty) = self_desc {
+                    format!(" for type `{}`", ty)
+                } else { "".to_string() };
+                err.note(&format!("upstream crates may add new impl of trait `{}`{} \
+                                  in future versions",
+                                  trait_desc, self_desc));
+            }
+        }
+    }
 }
 
 // A stack that walks back up the stack frame.
@@ -390,6 +429,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             freshener: infcx.freshener(),
             intercrate: false,
             inferred_obligations: SnapshotVec::new(),
+            intercrate_ambiguity_causes: Vec::new(),
         }
     }
 
@@ -399,6 +439,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             freshener: infcx.freshener(),
             intercrate: true,
             inferred_obligations: SnapshotVec::new(),
+            intercrate_ambiguity_causes: Vec::new(),
         }
     }
 
@@ -414,6 +455,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         self.infcx
     }
 
+    pub fn intercrate_ambiguity_causes(&self) -> &[IntercrateAmbiguityCause] {
+        &self.intercrate_ambiguity_causes
+    }
+
     /// Wraps the inference context's in_snapshot s.t. snapshot handling is only from the selection
     /// context's self.
     fn in_snapshot<R, F>(&mut self, f: F) -> R
@@ -767,6 +812,22 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         if unbound_input_types && self.intercrate {
             debug!("evaluate_stack({:?}) --> unbound argument, intercrate -->  ambiguous",
                    stack.fresh_trait_ref);
+            // Heuristics: show the diagnostics when there are no candidates in crate.
+            if let Ok(candidate_set) = self.assemble_candidates(stack) {
+                if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
+                    let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
+                    let self_ty = trait_ref.self_ty();
+                    let cause = IntercrateAmbiguityCause::DownstreamCrate {
+                        trait_desc: trait_ref.to_string(),
+                        self_desc: if self_ty.has_concrete_skeleton() {
+                            Some(self_ty.to_string())
+                        } else {
+                            None
+                        },
+                    };
+                    self.intercrate_ambiguity_causes.push(cause);
+                }
+            }
             return EvaluatedToAmbig;
         }
         if unbound_input_types &&
@@ -1021,6 +1082,25 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
 
         if !self.is_knowable(stack) {
             debug!("coherence stage: not knowable");
+            // Heuristics: show the diagnostics when there are no candidates in crate.
+            let candidate_set = self.assemble_candidates(stack)?;
+            if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
+                let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
+                let self_ty = trait_ref.self_ty();
+                let trait_desc = trait_ref.to_string();
+                let self_desc = if self_ty.has_concrete_skeleton() {
+                    Some(self_ty.to_string())
+                } else {
+                    None
+                };
+                let cause = if !coherence::trait_ref_is_local_or_fundamental(self.tcx(),
+                                                                             trait_ref) {
+                    IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc }
+                } else {
+                    IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc }
+                };
+                self.intercrate_ambiguity_causes.push(cause);
+            }
             return Ok(None);
         }
 

src/librustc/traits/specialize/mod.rs

@@ -25,6 +25,7 @@ use hir::def_id::DefId;
 use infer::{InferCtxt, InferOk};
 use ty::subst::{Subst, Substs};
 use traits::{self, Reveal, ObligationCause};
+use traits::select::IntercrateAmbiguityCause;
 use ty::{self, TyCtxt, TypeFoldable};
 use syntax_pos::DUMMY_SP;
 use std::rc::Rc;
@@ -36,6 +37,7 @@ pub struct OverlapError {
     pub with_impl: DefId,
     pub trait_desc: String,
     pub self_desc: Option<String>,
+    pub intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
 }
 
 /// Given a subst for the requested impl, translate it to a subst
@@ -343,6 +345,10 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx
                     }
                 }
 
+                for cause in &overlap.intercrate_ambiguity_causes {
+                    cause.add_intercrate_ambiguity_hint(&mut err);
+                }
+
                 err.emit();
             }
         } else {

src/librustc/traits/specialize/specialization_graph.rs

@@ -113,7 +113,7 @@ impl<'a, 'gcx, 'tcx> Children {
                 let overlap = traits::overlapping_impls(&infcx,
                                                         possible_sibling,
                                                         impl_def_id);
-                if let Some(impl_header) = overlap {
+                if let Some(overlap) = overlap {
                     if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
                         return Ok((false, false));
                     }
@@ -123,7 +123,7 @@ impl<'a, 'gcx, 'tcx> Children {
 
                     if le == ge {
                         // overlap, but no specialization; error out
-                        let trait_ref = impl_header.trait_ref.unwrap();
+                        let trait_ref = overlap.impl_header.trait_ref.unwrap();
                         let self_ty = trait_ref.self_ty();
                         Err(OverlapError {
                             with_impl: possible_sibling,
@@ -135,7 +135,8 @@ impl<'a, 'gcx, 'tcx> Children {
                                 Some(self_ty.to_string())
                             } else {
                                 None
-                            }
+                            },
+                            intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
                         })
                     } else {
                         Ok((le, ge))

src/librustc_typeck/coherence/inherent_impls_overlap.rs

@@ -26,7 +26,8 @@ struct InherentOverlapChecker<'a, 'tcx: 'a> {
 }
 
 impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
-    fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId) {
+    fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId,
+                                       overlap: traits::OverlapResult) {
         #[derive(Copy, Clone, PartialEq)]
         enum Namespace {
             Type,
@@ -50,16 +51,22 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
 
             for &item2 in &impl_items2[..] {
                 if (name, namespace) == name_and_namespace(item2) {
-                    struct_span_err!(self.tcx.sess,
-                                     self.tcx.span_of_impl(item1).unwrap(),
-                                     E0592,
-                                     "duplicate definitions with name `{}`",
-                                     name)
-                        .span_label(self.tcx.span_of_impl(item1).unwrap(),
-                                    format!("duplicate definitions for `{}`", name))
-                        .span_label(self.tcx.span_of_impl(item2).unwrap(),
-                                    format!("other definition for `{}`", name))
-                        .emit();
+                    let mut err = struct_span_err!(self.tcx.sess,
+                                                   self.tcx.span_of_impl(item1).unwrap(),
+                                                   E0592,
+                                                   "duplicate definitions with name `{}`",
+                                                   name);
+
+                    err.span_label(self.tcx.span_of_impl(item1).unwrap(),
+                                   format!("duplicate definitions for `{}`", name));
+                    err.span_label(self.tcx.span_of_impl(item2).unwrap(),
+                                   format!("other definition for `{}`", name));
+
+                    for cause in &overlap.intercrate_ambiguity_causes {
+                        cause.add_intercrate_ambiguity_hint(&mut err);
+                    }
+
+                    err.emit();
                 }
             }
         }
@@ -71,8 +78,9 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
         for (i, &impl1_def_id) in impls.iter().enumerate() {
             for &impl2_def_id in &impls[(i + 1)..] {
                 self.tcx.infer_ctxt().enter(|infcx| {
-                    if traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id).is_some() {
-                        self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id)
+                    if let Some(overlap) =
+                            traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id) {
+                        self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap)
                     }
                 });
             }

src/test/compile-fail/coherence-overlap-downstream-inherent.rs

@@ -0,0 +1,32 @@
+// Copyright 2017 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Tests that we consider `T: Sugar + Fruit` to be ambiguous, even
+// though no impls are found.
+
+struct Sweet<X>(X);
+pub trait Sugar {}
+pub trait Fruit {}
+impl<T:Sugar> Sweet<T> { fn dummy(&self) { } }
+//~^ ERROR E0592
+//~| NOTE duplicate definitions for `dummy`
+impl<T:Fruit> Sweet<T> { fn dummy(&self) { } }
+//~^ NOTE other definition for `dummy`
+
+trait Bar<X> {}
+struct A<T, X>(T, X);
+impl<X, T> A<T, X> where T: Bar<X> { fn f(&self) {} }
+//~^ ERROR E0592
+//~| NOTE duplicate definitions for `f`
+//~| NOTE downstream crates may implement trait `Bar<_>` for type `i32`
+impl<X> A<i32, X> { fn f(&self) {} }
+//~^ NOTE other definition for `f`
+
+fn main() {}

src/test/compile-fail/coherence-overlap-downstream.rs

@@ -0,0 +1,32 @@
+// Copyright 2017 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Tests that we consider `T: Sugar + Fruit` to be ambiguous, even
+// though no impls are found.
+
+pub trait Sugar {}
+pub trait Fruit {}
+pub trait Sweet {}
+impl<T:Sugar> Sweet for T { }
+//~^ NOTE first implementation here
+impl<T:Fruit> Sweet for T { }
+//~^ ERROR E0119
+//~| NOTE conflicting implementation
+
+pub trait Foo<X> {}
+pub trait Bar<X> {}
+impl<X, T> Foo<X> for T where T: Bar<X> {}
+//~^ NOTE first implementation here
+impl<X> Foo<X> for i32 {}
+//~^ ERROR E0119
+//~| NOTE conflicting implementation for `i32`
+//~| NOTE downstream crates may implement trait `Bar<_>` for type `i32`
+
+fn main() { }