diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 431bd8ee88f..f3682f8d35d 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,11 +119,14 @@ 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>, - trait_ref: &ty::TraitRef<'tcx>) -> bool + trait_ref: ty::TraitRef<'tcx>) -> bool { debug!("trait_ref_is_knowable(trait_ref={:?})", trait_ref); @@ -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,12 @@ 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>) + -> bool { + trait_ref.def_id.krate == LOCAL_CRATE || tcx.has_attr(trait_ref.def_id, "fundamental") +} + pub enum OrphanCheckErr<'tcx> { NoLocalInputType, UncoveredTy(Ty<'tcx>), @@ -177,11 +189,11 @@ pub fn orphan_check<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, return Ok(()); } - orphan_check_trait_ref(tcx, &trait_ref, InferIsLocal(false)) + orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(false)) } fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt, - trait_ref: &ty::TraitRef<'tcx>, + trait_ref: ty::TraitRef<'tcx>, infer_is_local: InferIsLocal) -> Result<(), OrphanCheckErr<'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..cd259fc2528 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -90,6 +90,45 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { intercrate: bool, inferred_obligations: SnapshotVec>, + + intercrate_ambiguity_causes: Vec, +} + +#[derive(Clone)] +pub enum IntercrateAmbiguityCause { + DownstreamCrate { + trait_desc: String, + self_desc: Option, + }, + UpstreamCrateUpdate { + trait_desc: String, + self_desc: Option, + }, +} + +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. @@ -380,6 +419,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 +429,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 +445,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 +802,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 && @@ -1003,6 +1064,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); } @@ -1124,7 +1204,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // ok to skip binder because of the nature of the // trait-ref-is-knowable check, which does not care about // bound regions - let trait_ref = &predicate.skip_binder().trait_ref; + let trait_ref = predicate.skip_binder().trait_ref; coherence::trait_ref_is_knowable(self.tcx(), trait_ref) } diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 2dd6ca4b5a9..f9332fc6697 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,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 { 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..76dcfe36e4f 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/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) } }); } diff --git a/src/test/compile-fail/coherence-overlap-downstream-inherent.rs b/src/test/compile-fail/coherence-overlap-downstream-inherent.rs new file mode 100644 index 00000000000..66068b53555 --- /dev/null +++ b/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 or the MIT license +// , 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); +pub trait Sugar {} +pub trait Fruit {} +impl Sweet { fn dummy(&self) { } } +//~^ ERROR E0592 +//~| NOTE duplicate definitions for `dummy` +impl Sweet { fn dummy(&self) { } } +//~^ NOTE other definition for `dummy` + +trait Bar {} +struct A(T, X); +impl A where T: Bar { fn f(&self) {} } +//~^ ERROR E0592 +//~| NOTE duplicate definitions for `f` +//~| NOTE downstream crates may implement trait `Bar<_>` for type `i32` +impl A { fn f(&self) {} } +//~^ NOTE other definition for `f` + +fn main() {} diff --git a/src/test/compile-fail/coherence-overlap-downstream.rs b/src/test/compile-fail/coherence-overlap-downstream.rs new file mode 100644 index 00000000000..1df02737dec --- /dev/null +++ b/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 or the MIT license +// , 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 Sweet for T { } +//~^ NOTE first implementation here +impl Sweet for T { } +//~^ ERROR E0119 +//~| NOTE conflicting implementation + +pub trait Foo {} +pub trait Bar {} +impl Foo for T where T: Bar {} +//~^ NOTE first implementation here +impl Foo for i32 {} +//~^ ERROR E0119 +//~| NOTE conflicting implementation for `i32` +//~| NOTE downstream crates may implement trait `Bar<_>` for type `i32` + +fn main() { } diff --git a/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs b/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs new file mode 100644 index 00000000000..355af60710a --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs @@ -0,0 +1,26 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that we consider `Box: !Sugar` to be ambiguous, even +// though we see no impl of `Sugar` for `Box`. Therefore, an overlap +// error is reported for the following pair of impls (#23516). + +pub trait Sugar {} + +struct Cake(X); + +impl Cake { fn dummy(&self) { } } +//~^ ERROR E0592 +//~| NOTE duplicate definitions for `dummy` +//~| NOTE downstream crates may implement trait `Sugar` for type `std::boxed::Box<_>` +impl Cake> { fn dummy(&self) { } } +//~^ NOTE other definition for `dummy` + +fn main() { } diff --git a/src/test/compile-fail/coherence-overlap-issue-23516.rs b/src/test/compile-fail/coherence-overlap-issue-23516.rs index 51d7c3e8b4c..ffef5bf1087 100644 --- a/src/test/compile-fail/coherence-overlap-issue-23516.rs +++ b/src/test/compile-fail/coherence-overlap-issue-23516.rs @@ -15,5 +15,10 @@ pub trait Sugar { fn dummy(&self) { } } pub trait Sweet { fn dummy(&self) { } } impl Sweet for T { } -impl Sweet for Box { } //~ ERROR E0119 +//~^ NOTE first implementation here +impl Sweet for Box { } +//~^ ERROR E0119 +//~| NOTE conflicting implementation for `std::boxed::Box<_>` +//~| NOTE downstream crates may implement trait `Sugar` for type `std::boxed::Box<_>` + fn main() { } diff --git a/src/test/compile-fail/coherence-overlap-upstream-inherent.rs b/src/test/compile-fail/coherence-overlap-upstream-inherent.rs new file mode 100644 index 00000000000..1d0c63110ce --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-upstream-inherent.rs @@ -0,0 +1,28 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that we consider `i16: Remote` to be ambiguous, even +// though the upstream crate doesn't implement it for now. + +// aux-build:coherence_lib.rs + +extern crate coherence_lib; + +use coherence_lib::Remote; + +struct A(X); +impl A where T: Remote { fn dummy(&self) { } } +//~^ ERROR E0592 +//~| NOTE duplicate definitions for `dummy` +//~| NOTE upstream crates may add new impl of trait `coherence_lib::Remote` for type `i16` +impl A { fn dummy(&self) { } } +//~^ NOTE other definition for `dummy` + +fn main() {} diff --git a/src/test/compile-fail/coherence-overlap-upstream.rs b/src/test/compile-fail/coherence-overlap-upstream.rs new file mode 100644 index 00000000000..e978143a067 --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-upstream.rs @@ -0,0 +1,28 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that we consider `i16: Remote` to be ambiguous, even +// though the upstream crate doesn't implement it for now. + +// aux-build:coherence_lib.rs + +extern crate coherence_lib; + +use coherence_lib::Remote; + +trait Foo {} +impl Foo for T where T: Remote {} +//~^ NOTE first implementation here +impl Foo for i16 {} +//~^ ERROR E0119 +//~| NOTE conflicting implementation for `i16` +//~| NOTE upstream crates may add new impl of trait `coherence_lib::Remote` for type `i16` + +fn main() {} diff --git a/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr b/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr index de8a24cf33f..eaf42cde22f 100644 --- a/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr +++ b/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr @@ -24,6 +24,8 @@ error[E0592]: duplicate definitions with name `baz` ... 43 | fn baz(&self) {} | ---------------- other definition for `baz` + | + = note: upstream crates may add new impl of trait `std::marker::Copy` for type `std::vec::Vec<_>` in future versions error: aborting due to 3 previous errors