From d49f27826388ae62467d2111f9e82a828c3e2ea8 Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Sun, 23 Jul 2017 22:30:47 +0900 Subject: [PATCH] Add hints when intercrate ambiguity causes overlap. --- src/librustc/traits/coherence.rs | 15 +++++-- src/librustc/traits/mod.rs | 5 +-- src/librustc/traits/select.rs | 29 ++++++++++++ src/librustc/traits/specialize/mod.rs | 16 +++++++ .../traits/specialize/specialization_graph.rs | 7 +-- .../coherence/inherent_impls_overlap.rs | 45 +++++++++++++------ 6 files changed, 95 insertions(+), 22 deletions(-) diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 431bd8ee88f..b7120ab31f0 100644 --- a/src/librustc/traits/coherence.rs +++ b/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, +} + /// 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> + -> Option> { 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> + -> Option> { 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>, diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 33dcf3c76e6..22c92776c5e 100644 --- a/src/librustc/traits/mod.rs +++ b/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; diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 726e5d83428..6815cb8c55e 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -90,6 +90,14 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { intercrate: bool, inferred_obligations: SnapshotVec>, + + intercrate_ambiguity_causes: Vec, +} + +#[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(&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); } diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 2dd6ca4b5a9..50518d36e61 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/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, + pub intercrate_ambiguity_causes: Vec, } /// 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 { diff --git a/src/librustc/traits/specialize/specialization_graph.rs b/src/librustc/traits/specialize/specialization_graph.rs index 5242accceab..2640542ad10 100644 --- a/src/librustc/traits/specialize/specialization_graph.rs +++ b/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)) diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs index 078ae34bc52..07de5423304 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/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) } }); }