Skip to content

Commit f83d20e

Browse files
committed
Auto merge of #43426 - qnighy:intercrate-ambiguity-hints, r=nikomatsakis
Add hints when intercrate ambiguity causes overlap. I'm going to tackle #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 ```
2 parents 2f1ef9e + 37c9a60 commit f83d20e

13 files changed

+290
-31
lines changed

src/librustc/traits/coherence.rs

+22-10
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use hir::def_id::{DefId, LOCAL_CRATE};
1414
use syntax_pos::DUMMY_SP;
1515
use traits::{self, Normalized, SelectionContext, Obligation, ObligationCause, Reveal};
16+
use traits::select::IntercrateAmbiguityCause;
1617
use ty::{self, Ty, TyCtxt};
1718
use ty::subst::Subst;
1819

@@ -21,12 +22,17 @@ use infer::{InferCtxt, InferOk};
2122
#[derive(Copy, Clone)]
2223
struct InferIsLocal(bool);
2324

25+
pub struct OverlapResult<'tcx> {
26+
pub impl_header: ty::ImplHeader<'tcx>,
27+
pub intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
28+
}
29+
2430
/// If there are types that satisfy both impls, returns a suitably-freshened
2531
/// `ImplHeader` with those types substituted
2632
pub fn overlapping_impls<'cx, 'gcx, 'tcx>(infcx: &InferCtxt<'cx, 'gcx, 'tcx>,
2733
impl1_def_id: DefId,
2834
impl2_def_id: DefId)
29-
-> Option<ty::ImplHeader<'tcx>>
35+
-> Option<OverlapResult<'tcx>>
3036
{
3137
debug!("impl_can_satisfy(\
3238
impl1_def_id={:?}, \
@@ -65,7 +71,7 @@ fn with_fresh_ty_vars<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, '
6571
fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
6672
a_def_id: DefId,
6773
b_def_id: DefId)
68-
-> Option<ty::ImplHeader<'tcx>>
74+
-> Option<OverlapResult<'tcx>>
6975
{
7076
debug!("overlap(a_def_id={:?}, b_def_id={:?})",
7177
a_def_id,
@@ -113,11 +119,14 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
113119
return None
114120
}
115121

116-
Some(selcx.infcx().resolve_type_vars_if_possible(&a_impl_header))
122+
Some(OverlapResult {
123+
impl_header: selcx.infcx().resolve_type_vars_if_possible(&a_impl_header),
124+
intercrate_ambiguity_causes: selcx.intercrate_ambiguity_causes().to_vec(),
125+
})
117126
}
118127

119128
pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
120-
trait_ref: &ty::TraitRef<'tcx>) -> bool
129+
trait_ref: ty::TraitRef<'tcx>) -> bool
121130
{
122131
debug!("trait_ref_is_knowable(trait_ref={:?})", trait_ref);
123132

@@ -131,10 +140,7 @@ pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
131140
// if the trait is not marked fundamental, then it's always possible that
132141
// an ancestor crate will impl this in the future, if they haven't
133142
// already
134-
if
135-
trait_ref.def_id.krate != LOCAL_CRATE &&
136-
!tcx.has_attr(trait_ref.def_id, "fundamental")
137-
{
143+
if !trait_ref_is_local_or_fundamental(tcx, trait_ref) {
138144
debug!("trait_ref_is_knowable: trait is neither local nor fundamental");
139145
return false;
140146
}
@@ -148,6 +154,12 @@ pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
148154
orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(true)).is_err()
149155
}
150156

157+
pub fn trait_ref_is_local_or_fundamental<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
158+
trait_ref: ty::TraitRef<'tcx>)
159+
-> bool {
160+
trait_ref.def_id.krate == LOCAL_CRATE || tcx.has_attr(trait_ref.def_id, "fundamental")
161+
}
162+
151163
pub enum OrphanCheckErr<'tcx> {
152164
NoLocalInputType,
153165
UncoveredTy(Ty<'tcx>),
@@ -177,11 +189,11 @@ pub fn orphan_check<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
177189
return Ok(());
178190
}
179191

180-
orphan_check_trait_ref(tcx, &trait_ref, InferIsLocal(false))
192+
orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(false))
181193
}
182194

183195
fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt,
184-
trait_ref: &ty::TraitRef<'tcx>,
196+
trait_ref: ty::TraitRef<'tcx>,
185197
infer_is_local: InferIsLocal)
186198
-> Result<(), OrphanCheckErr<'tcx>>
187199
{

src/librustc/traits/mod.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ use std::rc::Rc;
2828
use syntax::ast;
2929
use syntax_pos::{Span, DUMMY_SP};
3030

31-
pub use self::coherence::orphan_check;
32-
pub use self::coherence::overlapping_impls;
33-
pub use self::coherence::OrphanCheckErr;
31+
pub use self::coherence::{orphan_check, overlapping_impls, OrphanCheckErr, OverlapResult};
3432
pub use self::fulfill::{FulfillmentContext, RegionObligation};
3533
pub use self::project::MismatchedProjectionTypes;
3634
pub use self::project::{normalize, normalize_projection_type, Normalized};
@@ -39,6 +37,7 @@ pub use self::object_safety::ObjectSafetyViolation;
3937
pub use self::object_safety::MethodViolationCode;
4038
pub use self::on_unimplemented::{OnUnimplementedDirective, OnUnimplementedNote};
4139
pub use self::select::{EvaluationCache, SelectionContext, SelectionCache};
40+
pub use self::select::IntercrateAmbiguityCause;
4241
pub use self::specialize::{OverlapError, specialization_graph, translate_substs};
4342
pub use self::specialize::{SpecializesCache, find_associated_item};
4443
pub use self::util::elaborate_predicates;

src/librustc/traits/select.rs

+81-1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,45 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {
9090
intercrate: bool,
9191

9292
inferred_obligations: SnapshotVec<InferredObligationsSnapshotVecDelegate<'tcx>>,
93+
94+
intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
95+
}
96+
97+
#[derive(Clone)]
98+
pub enum IntercrateAmbiguityCause {
99+
DownstreamCrate {
100+
trait_desc: String,
101+
self_desc: Option<String>,
102+
},
103+
UpstreamCrateUpdate {
104+
trait_desc: String,
105+
self_desc: Option<String>,
106+
},
107+
}
108+
109+
impl IntercrateAmbiguityCause {
110+
/// Emits notes when the overlap is caused by complex intercrate ambiguities.
111+
/// See #23980 for details.
112+
pub fn add_intercrate_ambiguity_hint<'a, 'tcx>(&self,
113+
err: &mut ::errors::DiagnosticBuilder) {
114+
match self {
115+
&IntercrateAmbiguityCause::DownstreamCrate { ref trait_desc, ref self_desc } => {
116+
let self_desc = if let &Some(ref ty) = self_desc {
117+
format!(" for type `{}`", ty)
118+
} else { "".to_string() };
119+
err.note(&format!("downstream crates may implement trait `{}`{}",
120+
trait_desc, self_desc));
121+
}
122+
&IntercrateAmbiguityCause::UpstreamCrateUpdate { ref trait_desc, ref self_desc } => {
123+
let self_desc = if let &Some(ref ty) = self_desc {
124+
format!(" for type `{}`", ty)
125+
} else { "".to_string() };
126+
err.note(&format!("upstream crates may add new impl of trait `{}`{} \
127+
in future versions",
128+
trait_desc, self_desc));
129+
}
130+
}
131+
}
93132
}
94133

95134
// A stack that walks back up the stack frame.
@@ -380,6 +419,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
380419
freshener: infcx.freshener(),
381420
intercrate: false,
382421
inferred_obligations: SnapshotVec::new(),
422+
intercrate_ambiguity_causes: Vec::new(),
383423
}
384424
}
385425

@@ -389,6 +429,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
389429
freshener: infcx.freshener(),
390430
intercrate: true,
391431
inferred_obligations: SnapshotVec::new(),
432+
intercrate_ambiguity_causes: Vec::new(),
392433
}
393434
}
394435

@@ -404,6 +445,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
404445
self.infcx
405446
}
406447

448+
pub fn intercrate_ambiguity_causes(&self) -> &[IntercrateAmbiguityCause] {
449+
&self.intercrate_ambiguity_causes
450+
}
451+
407452
/// Wraps the inference context's in_snapshot s.t. snapshot handling is only from the selection
408453
/// context's self.
409454
fn in_snapshot<R, F>(&mut self, f: F) -> R
@@ -757,6 +802,22 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
757802
if unbound_input_types && self.intercrate {
758803
debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous",
759804
stack.fresh_trait_ref);
805+
// Heuristics: show the diagnostics when there are no candidates in crate.
806+
if let Ok(candidate_set) = self.assemble_candidates(stack) {
807+
if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
808+
let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
809+
let self_ty = trait_ref.self_ty();
810+
let cause = IntercrateAmbiguityCause::DownstreamCrate {
811+
trait_desc: trait_ref.to_string(),
812+
self_desc: if self_ty.has_concrete_skeleton() {
813+
Some(self_ty.to_string())
814+
} else {
815+
None
816+
},
817+
};
818+
self.intercrate_ambiguity_causes.push(cause);
819+
}
820+
}
760821
return EvaluatedToAmbig;
761822
}
762823
if unbound_input_types &&
@@ -1003,6 +1064,25 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
10031064

10041065
if !self.is_knowable(stack) {
10051066
debug!("coherence stage: not knowable");
1067+
// Heuristics: show the diagnostics when there are no candidates in crate.
1068+
let candidate_set = self.assemble_candidates(stack)?;
1069+
if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
1070+
let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
1071+
let self_ty = trait_ref.self_ty();
1072+
let trait_desc = trait_ref.to_string();
1073+
let self_desc = if self_ty.has_concrete_skeleton() {
1074+
Some(self_ty.to_string())
1075+
} else {
1076+
None
1077+
};
1078+
let cause = if !coherence::trait_ref_is_local_or_fundamental(self.tcx(),
1079+
trait_ref) {
1080+
IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc }
1081+
} else {
1082+
IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc }
1083+
};
1084+
self.intercrate_ambiguity_causes.push(cause);
1085+
}
10061086
return Ok(None);
10071087
}
10081088

@@ -1124,7 +1204,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
11241204
// ok to skip binder because of the nature of the
11251205
// trait-ref-is-knowable check, which does not care about
11261206
// bound regions
1127-
let trait_ref = &predicate.skip_binder().trait_ref;
1207+
let trait_ref = predicate.skip_binder().trait_ref;
11281208

11291209
coherence::trait_ref_is_knowable(self.tcx(), trait_ref)
11301210
}

src/librustc/traits/specialize/mod.rs

+6
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use hir::def_id::DefId;
2525
use infer::{InferCtxt, InferOk};
2626
use ty::subst::{Subst, Substs};
2727
use traits::{self, Reveal, ObligationCause};
28+
use traits::select::IntercrateAmbiguityCause;
2829
use ty::{self, TyCtxt, TypeFoldable};
2930
use syntax_pos::DUMMY_SP;
3031
use std::rc::Rc;
@@ -36,6 +37,7 @@ pub struct OverlapError {
3637
pub with_impl: DefId,
3738
pub trait_desc: String,
3839
pub self_desc: Option<String>,
40+
pub intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
3941
}
4042

4143
/// 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
337339
}
338340
}
339341

342+
for cause in &overlap.intercrate_ambiguity_causes {
343+
cause.add_intercrate_ambiguity_hint(&mut err);
344+
}
345+
340346
err.emit();
341347
}
342348
} else {

src/librustc/traits/specialize/specialization_graph.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ impl<'a, 'gcx, 'tcx> Children {
113113
let overlap = traits::overlapping_impls(&infcx,
114114
possible_sibling,
115115
impl_def_id);
116-
if let Some(impl_header) = overlap {
116+
if let Some(overlap) = overlap {
117117
if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
118118
return Ok((false, false));
119119
}
@@ -123,7 +123,7 @@ impl<'a, 'gcx, 'tcx> Children {
123123

124124
if le == ge {
125125
// overlap, but no specialization; error out
126-
let trait_ref = impl_header.trait_ref.unwrap();
126+
let trait_ref = overlap.impl_header.trait_ref.unwrap();
127127
let self_ty = trait_ref.self_ty();
128128
Err(OverlapError {
129129
with_impl: possible_sibling,
@@ -135,7 +135,8 @@ impl<'a, 'gcx, 'tcx> Children {
135135
Some(self_ty.to_string())
136136
} else {
137137
None
138-
}
138+
},
139+
intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
139140
})
140141
} else {
141142
Ok((le, ge))

src/librustc_typeck/coherence/inherent_impls_overlap.rs

+21-13
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ struct InherentOverlapChecker<'a, 'tcx: 'a> {
2626
}
2727

2828
impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
29-
fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId) {
29+
fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId,
30+
overlap: traits::OverlapResult) {
3031
#[derive(Copy, Clone, PartialEq)]
3132
enum Namespace {
3233
Type,
@@ -50,16 +51,22 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
5051

5152
for &item2 in &impl_items2[..] {
5253
if (name, namespace) == name_and_namespace(item2) {
53-
struct_span_err!(self.tcx.sess,
54-
self.tcx.span_of_impl(item1).unwrap(),
55-
E0592,
56-
"duplicate definitions with name `{}`",
57-
name)
58-
.span_label(self.tcx.span_of_impl(item1).unwrap(),
59-
format!("duplicate definitions for `{}`", name))
60-
.span_label(self.tcx.span_of_impl(item2).unwrap(),
61-
format!("other definition for `{}`", name))
62-
.emit();
54+
let mut err = struct_span_err!(self.tcx.sess,
55+
self.tcx.span_of_impl(item1).unwrap(),
56+
E0592,
57+
"duplicate definitions with name `{}`",
58+
name);
59+
60+
err.span_label(self.tcx.span_of_impl(item1).unwrap(),
61+
format!("duplicate definitions for `{}`", name));
62+
err.span_label(self.tcx.span_of_impl(item2).unwrap(),
63+
format!("other definition for `{}`", name));
64+
65+
for cause in &overlap.intercrate_ambiguity_causes {
66+
cause.add_intercrate_ambiguity_hint(&mut err);
67+
}
68+
69+
err.emit();
6370
}
6471
}
6572
}
@@ -71,8 +78,9 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
7178
for (i, &impl1_def_id) in impls.iter().enumerate() {
7279
for &impl2_def_id in &impls[(i + 1)..] {
7380
self.tcx.infer_ctxt().enter(|infcx| {
74-
if traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id).is_some() {
75-
self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id)
81+
if let Some(overlap) =
82+
traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id) {
83+
self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap)
7684
}
7785
});
7886
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Tests that we consider `T: Sugar + Fruit` to be ambiguous, even
12+
// though no impls are found.
13+
14+
struct Sweet<X>(X);
15+
pub trait Sugar {}
16+
pub trait Fruit {}
17+
impl<T:Sugar> Sweet<T> { fn dummy(&self) { } }
18+
//~^ ERROR E0592
19+
//~| NOTE duplicate definitions for `dummy`
20+
impl<T:Fruit> Sweet<T> { fn dummy(&self) { } }
21+
//~^ NOTE other definition for `dummy`
22+
23+
trait Bar<X> {}
24+
struct A<T, X>(T, X);
25+
impl<X, T> A<T, X> where T: Bar<X> { fn f(&self) {} }
26+
//~^ ERROR E0592
27+
//~| NOTE duplicate definitions for `f`
28+
//~| NOTE downstream crates may implement trait `Bar<_>` for type `i32`
29+
impl<X> A<i32, X> { fn f(&self) {} }
30+
//~^ NOTE other definition for `f`
31+
32+
fn main() {}

0 commit comments

Comments
 (0)