From bc33dd7ac4d4fdc2d14c6da4cce62e82a4f94f86 Mon Sep 17 00:00:00 2001 From: Aaron Turon Date: Tue, 1 Dec 2015 11:26:47 -0800 Subject: [PATCH] Provide overlapping types for coherence errors Currently, a coherence error based on overlapping impls simply mentions the trait, and points to the two conflicting impls: ``` error: conflicting implementations for trait `Foo` ``` With this commit, the error will include all input types to the trait (including the `Self` type) after unification between the overlapping impls. In other words, the error message will provide feedback with full type details, like: ``` error: conflicting implementations of trait `Foo` for type `u8`: ``` When the `Self` type for the two impls unify to an inference variable, it is elided in the output, since "for type `_`" is just noise in that case. Closes #23980 --- src/librustc/middle/traits/coherence.rs | 32 +++++------ src/librustc_typeck/coherence/overlap.rs | 53 ++++++++++--------- src/librustc_typeck/diagnostics.rs | 4 +- ...herence-conflicting-negative-trait-impl.rs | 8 +-- .../coherence-default-trait-impl.rs | 2 +- .../coherence-overlap-messages.rs | 42 +++++++++++++++ src/test/compile-fail/issue-28568.rs | 4 +- 7 files changed, 95 insertions(+), 50 deletions(-) create mode 100644 src/test/compile-fail/coherence-overlap-messages.rs diff --git a/src/librustc/middle/traits/coherence.rs b/src/librustc/middle/traits/coherence.rs index 76428ca6d90..cb166fbff05 100644 --- a/src/librustc/middle/traits/coherence.rs +++ b/src/librustc/middle/traits/coherence.rs @@ -27,11 +27,12 @@ use syntax::codemap::{DUMMY_SP, Span}; #[derive(Copy, Clone)] struct InferIsLocal(bool); -/// True if there exist types that satisfy both of the two given impls. -pub fn overlapping_impls(infcx: &InferCtxt, - impl1_def_id: DefId, - impl2_def_id: DefId) - -> bool +/// If there are types that satisfy both impls, returns a `TraitRef` +/// with those types substituted (by updating the given `infcx`) +pub fn overlapping_impls<'cx, 'tcx>(infcx: &InferCtxt<'cx, 'tcx>, + impl1_def_id: DefId, + impl2_def_id: DefId) + -> Option> { debug!("impl_can_satisfy(\ impl1_def_id={:?}, \ @@ -40,16 +41,15 @@ pub fn overlapping_impls(infcx: &InferCtxt, impl2_def_id); let selcx = &mut SelectionContext::intercrate(infcx); - infcx.probe(|_| { - overlap(selcx, impl1_def_id, impl2_def_id) - }) + overlap(selcx, impl1_def_id, impl2_def_id) } -/// Can both impl `a` and impl `b` be satisfied by a common type (including `where` clauses)? -fn overlap(selcx: &mut SelectionContext, - a_def_id: DefId, - b_def_id: DefId) - -> bool +/// Can both impl `a` and impl `b` be satisfied by a common type (including +/// `where` clauses)? If so, returns a `TraitRef` that unifies the two impls. +fn overlap<'cx, 'tcx>(selcx: &mut SelectionContext<'cx, 'tcx>, + a_def_id: DefId, + b_def_id: DefId) + -> Option> { debug!("overlap(a_def_id={:?}, b_def_id={:?})", a_def_id, @@ -73,7 +73,7 @@ fn overlap(selcx: &mut SelectionContext, TypeOrigin::Misc(DUMMY_SP), a_trait_ref, b_trait_ref) { - return false; + return None; } debug!("overlap: unification check succeeded"); @@ -88,10 +88,10 @@ fn overlap(selcx: &mut SelectionContext, if let Some(failing_obligation) = opt_failing_obligation { debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); - return false + return None } - true + Some(selcx.infcx().resolve_type_vars_if_possible(&a_trait_ref)) } pub fn trait_ref_is_knowable<'tcx>(tcx: &ty::ctxt<'tcx>, trait_ref: &ty::TraitRef<'tcx>) -> bool diff --git a/src/librustc_typeck/coherence/overlap.rs b/src/librustc_typeck/coherence/overlap.rs index 693c8716ab5..90a3b99b71c 100644 --- a/src/librustc_typeck/coherence/overlap.rs +++ b/src/librustc_typeck/coherence/overlap.rs @@ -65,22 +65,19 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> { // borrows are safe. let blanket_impls = trait_def.blanket_impls.borrow(); let nonblanket_impls = trait_def.nonblanket_impls.borrow(); - let trait_def_id = trait_def.trait_ref.def_id; // Conflicts can only occur between a blanket impl and another impl, // or between 2 non-blanket impls of the same kind. for (i, &impl1_def_id) in blanket_impls.iter().enumerate() { for &impl2_def_id in &blanket_impls[(i+1)..] { - self.check_if_impls_overlap(trait_def_id, - impl1_def_id, + self.check_if_impls_overlap(impl1_def_id, impl2_def_id); } for v in nonblanket_impls.values() { for &impl2_def_id in v { - self.check_if_impls_overlap(trait_def_id, - impl1_def_id, + self.check_if_impls_overlap(impl1_def_id, impl2_def_id); } } @@ -89,8 +86,7 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> { for impl_group in nonblanket_impls.values() { for (i, &impl1_def_id) in impl_group.iter().enumerate() { for &impl2_def_id in &impl_group[(i+1)..] { - self.check_if_impls_overlap(trait_def_id, - impl1_def_id, + self.check_if_impls_overlap(impl1_def_id, impl2_def_id); } } @@ -121,40 +117,47 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> { fn check_if_impls_overlap(&self, - trait_def_id: DefId, impl1_def_id: DefId, impl2_def_id: DefId) { if let Some((impl1_def_id, impl2_def_id)) = self.order_impls( impl1_def_id, impl2_def_id) { - debug!("check_if_impls_overlap({:?}, {:?}, {:?})", - trait_def_id, + debug!("check_if_impls_overlap({:?}, {:?})", impl1_def_id, impl2_def_id); let infcx = infer::new_infer_ctxt(self.tcx, &self.tcx.tables, None, false); - if traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id) { - self.report_overlap_error(trait_def_id, impl1_def_id, impl2_def_id); + if let Some(trait_ref) = traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id) { + self.report_overlap_error(impl1_def_id, impl2_def_id, trait_ref); } } } - fn report_overlap_error(&self, trait_def_id: DefId, - impl1: DefId, impl2: DefId) { + fn report_overlap_error(&self, + impl1: DefId, + impl2: DefId, + trait_ref: ty::TraitRef) + { + // only print the Self type if it's concrete; otherwise, it's not adding much information. + let self_type = { + trait_ref.substs.self_ty().and_then(|ty| { + if let ty::TyInfer(_) = ty.sty { + None + } else { + Some(format!(" for type `{}`", ty)) + } + }).unwrap_or(String::new()) + }; span_err!(self.tcx.sess, self.span_of_impl(impl1), E0119, - "conflicting implementations for trait `{}`", - self.tcx.item_path_str(trait_def_id)); - - self.report_overlap_note(impl2); - } - - fn report_overlap_note(&self, impl2: DefId) { + "conflicting implementations of trait `{}`{}:", + trait_ref, + self_type); if impl2.is_local() { span_note!(self.tcx.sess, self.span_of_impl(impl2), - "note conflicting implementation here"); + "conflicting implementation is here:"); } else { let cname = self.tcx.sess.cstore.crate_name(impl2.krate); self.tcx.sess.note(&format!("conflicting implementation in crate `{}`", cname)); @@ -180,9 +183,9 @@ impl<'cx, 'tcx,'v> intravisit::Visitor<'v> for OverlapChecker<'cx, 'tcx> { let prev_default_impl = self.default_impls.insert(trait_ref.def_id, item.id); match prev_default_impl { Some(prev_id) => { - self.report_overlap_error(trait_ref.def_id, - impl_def_id, - self.tcx.map.local_def_id(prev_id)); + self.report_overlap_error(impl_def_id, + self.tcx.map.local_def_id(prev_id), + trait_ref); } None => { } } diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 2d7178bd55c..8c15892b7e1 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -1569,8 +1569,8 @@ struct Foo { value: usize } -impl MyTrait for Foo { // error: conflicting implementations for trait - // `MyTrait` +impl MyTrait for Foo { // error: conflicting implementations of trait + // `MyTrait` for type `Foo` fn get(&self) -> usize { self.value } } ``` diff --git a/src/test/compile-fail/coherence-conflicting-negative-trait-impl.rs b/src/test/compile-fail/coherence-conflicting-negative-trait-impl.rs index 55c9ba2a0e8..344ec89d25d 100644 --- a/src/test/compile-fail/coherence-conflicting-negative-trait-impl.rs +++ b/src/test/compile-fail/coherence-conflicting-negative-trait-impl.rs @@ -15,14 +15,14 @@ trait MyTrait {} struct TestType(::std::marker::PhantomData); unsafe impl Send for TestType {} -//~^ ERROR conflicting implementations for trait `core::marker::Send` -//~^^ ERROR conflicting implementations for trait `core::marker::Send` +//~^ ERROR conflicting implementations of trait `core::marker::Send` +//~^^ ERROR conflicting implementations of trait `core::marker::Send` impl !Send for TestType {} -//~^ ERROR conflicting implementations for trait `core::marker::Send` +//~^ ERROR conflicting implementations of trait `core::marker::Send` unsafe impl Send for TestType {} -//~^ ERROR error: conflicting implementations for trait `core::marker::Send` +//~^ ERROR error: conflicting implementations of trait `core::marker::Send` impl !Send for TestType {} diff --git a/src/test/compile-fail/coherence-default-trait-impl.rs b/src/test/compile-fail/coherence-default-trait-impl.rs index cccc8b05b30..0705702b031 100644 --- a/src/test/compile-fail/coherence-default-trait-impl.rs +++ b/src/test/compile-fail/coherence-default-trait-impl.rs @@ -15,7 +15,7 @@ trait MyTrait {} impl MyTrait for .. {} impl MyTrait for .. {} -//~^ ERROR conflicting implementations for trait `MyTrait` +//~^ ERROR conflicting implementations of trait `MyTrait` trait MySafeTrait {} diff --git a/src/test/compile-fail/coherence-overlap-messages.rs b/src/test/compile-fail/coherence-overlap-messages.rs new file mode 100644 index 00000000000..4f1092f960e --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-messages.rs @@ -0,0 +1,42 @@ +// 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. + +trait Foo {} + +impl Foo for T {} //~ ERROR conflicting implementations of trait `Foo`: +impl Foo for U {} + +trait Bar {} + +impl Bar for T {} //~ ERROR conflicting implementations of trait `Bar` for type `u8`: +impl Bar for u8 {} + +trait Baz {} + +impl Baz for T {} //~ ERROR conflicting implementations of trait `Baz<_>` for type `u8`: +impl Baz for u8 {} + +trait Quux {} + +impl Quux for T {} //~ ERROR conflicting implementations of trait `Quux<_>`: +impl Quux for T {} + +trait Qaar {} + +impl Qaar for T {} //~ ERROR conflicting implementations of trait `Qaar`: +impl Qaar for T {} + +trait Qaax {} + +impl Qaax for T {} +//~^ ERROR conflicting implementations of trait `Qaax` for type `u32`: +impl Qaax for u32 {} + +fn main() {} diff --git a/src/test/compile-fail/issue-28568.rs b/src/test/compile-fail/issue-28568.rs index 36b4a57eb11..1dfff144cef 100644 --- a/src/test/compile-fail/issue-28568.rs +++ b/src/test/compile-fail/issue-28568.rs @@ -11,12 +11,12 @@ struct MyStruct; impl Drop for MyStruct { -//~^ ERROR conflicting implementations for trait +//~^ ERROR conflicting implementations of trait fn drop(&mut self) { } } impl Drop for MyStruct { -//~^ NOTE conflicting implementation here +//~^ NOTE conflicting implementation is here fn drop(&mut self) { } }