Add hints when intercrate ambiguity causes overlap.

Author: qnighy

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

src/librustc/traits/mod.rs

@@ -28,9 +28,7 @@ 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};
@@ -39,6 +37,7 @@ pub use self::object_safety::ObjectSafetyViolation;
 pub use self::object_safety::MethodViolationCode;
 pub use self::on_unimplemented::{OnUnimplementedDirective, OnUnimplementedNote};
 pub use self::select::{EvaluationCache, SelectionContext, SelectionCache};
+pub use self::select::IntercrateAmbiguityCause;
 pub use self::specialize::{OverlapError, specialization_graph, translate_substs};
 pub use self::specialize::{SpecializesCache, find_associated_item};
 pub use self::util::elaborate_predicates;

src/librustc/traits/select.rs

@@ -90,6 +90,14 @@ 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(DefId),
+    UpstreamCrateUpdate(DefId),
 }
 
 // A stack that walks back up the stack frame.
@@ -380,6 +388,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             freshener: infcx.freshener(),
             intercrate: false,
             inferred_obligations: SnapshotVec::new(),
+            intercrate_ambiguity_causes: Vec::new(),
         }
     }
 
@@ -389,6 +398,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             freshener: infcx.freshener(),
             intercrate: true,
             inferred_obligations: SnapshotVec::new(),
+            intercrate_ambiguity_causes: Vec::new(),
         }
     }
 
@@ -404,6 +414,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
@@ -757,6 +771,14 @@ 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 did = stack.fresh_trait_ref.def_id();
+                    self.intercrate_ambiguity_causes.push(
+                        IntercrateAmbiguityCause::DownstreamCrate(did));
+                }
+            }
             return EvaluatedToAmbig;
         }
         if unbound_input_types &&
@@ -1003,6 +1025,13 @@ 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 did = stack.obligation.predicate.def_id();
+                self.intercrate_ambiguity_causes.push(
+                    IntercrateAmbiguityCause::UpstreamCrateUpdate(did));
+            }
             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
@@ -337,6 +339,20 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx
                     }
                 }
 
+                for cause in &overlap.intercrate_ambiguity_causes {
+                    match cause {
+                        &IntercrateAmbiguityCause::DownstreamCrate(def_id) => {
+                            err.note(&format!("downstream crates may implement {}",
+                                              tcx.item_path_str(def_id)));
+                        }
+                        &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => {
+                            err.note(&format!("upstream crates may add new impl for {} \
+                                              in future versions",
+                                              tcx.item_path_str(def_id)));
+                        }
+                    }
+                }
+
                 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

@@ -12,6 +12,7 @@ use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
 use rustc::hir;
 use rustc::hir::itemlikevisit::ItemLikeVisitor;
 use rustc::traits;
+use rustc::traits::IntercrateAmbiguityCause;
 use rustc::ty::{self, TyCtxt};
 
 pub fn crate_inherent_impls_overlap_check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -26,7 +27,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 +52,32 @@ 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 {
+                        match cause {
+                            &IntercrateAmbiguityCause::DownstreamCrate(def_id) => {
+                                err.note(&format!("downstream crates may implement {}",
+                                                  self.tcx.item_path_str(def_id)));
+                            }
+                            &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => {
+                                err.note(&format!("upstream crates may add new impl for {} \
+                                                  in future versions",
+                                                  self.tcx.item_path_str(def_id)));
+                            }
+                        }
+                    }
+
+                    err.emit();
                 }
             }
         }
@@ -71,8 +89,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)
                     }
                 });
             }