diff options
| author | Ariel Ben-Yehuda <ariel.byd@gmail.com> | 2017-11-23 19:05:23 +0200 |
|---|---|---|
| committer | Ariel Ben-Yehuda <ariel.byd@gmail.com> | 2017-12-05 15:42:33 +0200 |
| commit | 2614cc51dde5e57983dd9809372845073ac30aac (patch) | |
| tree | cfb570cb827ef0f3901498d46afbe56964e86f68 | |
| parent | 1271ea4f958d335ee67452faff925ab9b8715648 (diff) | |
| download | rust-2614cc51dde5e57983dd9809372845073ac30aac.tar.gz rust-2614cc51dde5e57983dd9809372845073ac30aac.zip | |
convert the new conflicts to a soft error
| -rw-r--r-- | src/librustc/lint/builtin.rs | 7 | ||||
| -rw-r--r-- | src/librustc/traits/coherence.rs | 12 | ||||
| -rw-r--r-- | src/librustc/traits/mod.rs | 3 | ||||
| -rw-r--r-- | src/librustc/traits/select.rs | 47 | ||||
| -rw-r--r-- | src/librustc/traits/specialize/mod.rs | 39 | ||||
| -rw-r--r-- | src/librustc/traits/specialize/specialization_graph.rs | 68 | ||||
| -rw-r--r-- | src/librustc_lint/lib.rs | 5 | ||||
| -rw-r--r-- | src/librustc_typeck/coherence/inherent_impls_overlap.rs | 52 | ||||
| -rw-r--r-- | src/test/compile-fail/issue-43355.rs | 1 | ||||
| -rw-r--r-- | src/test/run-pass/issue-43355.rs | 38 |
10 files changed, 207 insertions, 65 deletions
diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 1008da1e937..e31fc48b907 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -205,6 +205,12 @@ declare_lint! { } declare_lint! { + pub INCOHERENT_FUNDAMENTAL_IMPLS, + Deny, + "potentially-conflicting impls were erroneously allowed" +} + +declare_lint! { pub DEPRECATED, Warn, "detects use of deprecated items" @@ -267,6 +273,7 @@ impl LintPass for HardwiredLints { MISSING_FRAGMENT_SPECIFIER, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES, LATE_BOUND_LIFETIME_ARGUMENTS, + INCOHERENT_FUNDAMENTAL_IMPLS, DEPRECATED, UNUSED_UNSAFE, UNUSED_MUT, diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index e7c750f4bb4..756921d7a87 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::IntercrateMode; use traits::select::IntercrateAmbiguityCause; use ty::{self, Ty, TyCtxt}; use ty::subst::Subst; @@ -42,16 +43,19 @@ pub struct OverlapResult<'tcx> { /// `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) + impl2_def_id: DefId, + intercrate_mode: IntercrateMode) -> Option<OverlapResult<'tcx>> { debug!("impl_can_satisfy(\ impl1_def_id={:?}, \ - impl2_def_id={:?})", + impl2_def_id={:?}, + intercrate_mode={:?})", impl1_def_id, - impl2_def_id); + impl2_def_id, + intercrate_mode); - let selcx = &mut SelectionContext::intercrate(infcx); + let selcx = &mut SelectionContext::intercrate(infcx, intercrate_mode); overlap(selcx, impl1_def_id, impl2_def_id) } diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index d65a5f1c3aa..94605d895a5 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -60,7 +60,8 @@ mod structural_impls; pub mod trans; mod util; -#[derive(Copy, Clone, Debug)] +// Whether to enable bug compatibility with issue #43355 +#[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum IntercrateMode { Issue43355, Fixed diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 9625894b872..6d845accc64 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -15,6 +15,7 @@ use self::EvaluationResult::*; use super::coherence::{self, Conflict}; use super::DerivedObligationCause; +use super::IntercrateMode; use super::project; use super::project::{normalize_with_depth, Normalized, ProjectionCacheKey}; use super::{PredicateObligation, TraitObligation, ObligationCause}; @@ -87,7 +88,7 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { /// other words, we consider `$0 : Bar` to be unimplemented if /// there is no type that the user could *actually name* that /// would satisfy it. This avoids crippling inference, basically. - intercrate: bool, + intercrate: Option<IntercrateMode>, inferred_obligations: SnapshotVec<InferredObligationsSnapshotVecDelegate<'tcx>>, @@ -111,21 +112,24 @@ impl IntercrateAmbiguityCause { /// See #23980 for details. pub fn add_intercrate_ambiguity_hint<'a, 'tcx>(&self, err: &mut ::errors::DiagnosticBuilder) { + err.note(&self.intercrate_ambiguity_hint()); + } + + pub fn intercrate_ambiguity_hint(&self) -> String { 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)); + 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)); + format!("upstream crates may add new impl of trait `{}`{} \ + in future versions", + trait_desc, self_desc) } } } @@ -417,17 +421,19 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { SelectionContext { infcx, freshener: infcx.freshener(), - intercrate: false, + intercrate: None, inferred_obligations: SnapshotVec::new(), intercrate_ambiguity_causes: Vec::new(), } } - pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>) -> SelectionContext<'cx, 'gcx, 'tcx> { + pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>, + mode: IntercrateMode) -> SelectionContext<'cx, 'gcx, 'tcx> { + debug!("intercrate({:?})", mode); SelectionContext { infcx, freshener: infcx.freshener(), - intercrate: true, + intercrate: Some(mode), inferred_obligations: SnapshotVec::new(), intercrate_ambiguity_causes: Vec::new(), } @@ -758,7 +764,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { debug!("evaluate_trait_predicate_recursively({:?})", obligation); - if !self.intercrate && obligation.is_global() { + if !self.intercrate.is_some() && obligation.is_global() { // If a param env is consistent, global obligations do not depend on its particular // value in order to work, so we can clear out the param env and get better // caching. (If the current param env is inconsistent, we don't care what happens). @@ -814,7 +820,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // terms of `Fn` etc, but we could probably make this more // precise still. let unbound_input_types = stack.fresh_trait_ref.input_types().any(|ty| ty.is_fresh()); - if unbound_input_types && self.intercrate && false { + // this check was an imperfect workaround for a bug n the old + // intercrate mode, it should be removed when that goes away. + if unbound_input_types && + self.intercrate == Some(IntercrateMode::Issue43355) + { debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous", stack.fresh_trait_ref); // Heuristics: show the diagnostics when there are no candidates in crate. @@ -1212,9 +1222,9 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { stack: &TraitObligationStack<'o, 'tcx>) -> Option<Conflict> { - debug!("is_knowable(intercrate={})", self.intercrate); + debug!("is_knowable(intercrate={:?})", self.intercrate); - if !self.intercrate { + if !self.intercrate.is_some() { return None; } @@ -1226,7 +1236,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // bound regions let trait_ref = predicate.skip_binder().trait_ref; - coherence::trait_ref_is_knowable(self.tcx(), trait_ref) + let result = coherence::trait_ref_is_knowable(self.tcx(), trait_ref); + if let (Some(Conflict::Downstream { used_to_be_broken: true }), + Some(IntercrateMode::Issue43355)) = (result, self.intercrate) { + debug!("is_knowable: IGNORING conflict to be bug-compatible with #43355"); + None + } else { + result + } } /// Returns true if the global caches can be used. @@ -1251,7 +1268,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // the master cache. Since coherence executes pretty quickly, // it's not worth going to more trouble to increase the // hit-rate I don't think. - if self.intercrate { + if self.intercrate.is_some() { return false; } diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 1b5b0d35ba3..6a96d01d5f9 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -30,6 +30,8 @@ use ty::{self, TyCtxt, TypeFoldable}; use syntax_pos::DUMMY_SP; use std::rc::Rc; +use lint; + pub mod specialization_graph; /// Information pertinent to an overlapping impl error. @@ -325,16 +327,33 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx // This is where impl overlap checking happens: let insert_result = sg.insert(tcx, impl_def_id); // Report error if there was one. - if let Err(overlap) = insert_result { - let mut err = struct_span_err!(tcx.sess, - tcx.span_of_impl(impl_def_id).unwrap(), - E0119, - "conflicting implementations of trait `{}`{}:", - overlap.trait_desc, - overlap.self_desc.clone().map_or(String::new(), - |ty| { - format!(" for type `{}`", ty) - })); + let (overlap, used_to_be_allowed) = match insert_result { + Err(overlap) => (Some(overlap), false), + Ok(opt_overlap) => (opt_overlap, true) + }; + + if let Some(overlap) = overlap { + let msg = format!("conflicting implementations of trait `{}`{}:{}", + overlap.trait_desc, + overlap.self_desc.clone().map_or( + String::new(), |ty| { + format!(" for type `{}`", ty) + }), + if used_to_be_allowed { " (E0119)" } else { "" } + ); + let mut err = if used_to_be_allowed { + tcx.struct_span_lint_node( + lint::builtin::INCOHERENT_FUNDAMENTAL_IMPLS, + tcx.hir.as_local_node_id(impl_def_id).unwrap(), + tcx.span_of_impl(impl_def_id).unwrap(), + &msg) + } else { + struct_span_err!(tcx.sess, + tcx.span_of_impl(impl_def_id).unwrap(), + E0119, + "{}", + msg) + }; match tcx.span_of_impl(overlap.with_impl) { Ok(span) => { diff --git a/src/librustc/traits/specialize/specialization_graph.rs b/src/librustc/traits/specialize/specialization_graph.rs index da9dbc0e2c9..834389e5d00 100644 --- a/src/librustc/traits/specialize/specialization_graph.rs +++ b/src/librustc/traits/specialize/specialization_graph.rs @@ -68,7 +68,7 @@ struct Children { /// The result of attempting to insert an impl into a group of children. enum Inserted { /// The impl was inserted as a new child in this group of children. - BecameNewSibling, + BecameNewSibling(Option<OverlapError>), /// The impl replaced an existing impl that specializes it. Replaced(DefId), @@ -105,17 +105,39 @@ impl<'a, 'gcx, 'tcx> Children { simplified_self: Option<SimplifiedType>) -> Result<Inserted, OverlapError> { + let mut last_lint = None; + for slot in match simplified_self { Some(sty) => self.filtered_mut(sty), None => self.iter_mut(), } { let possible_sibling = *slot; + let overlap_error = |overlap: traits::coherence::OverlapResult| { + // overlap, but no specialization; error out + let trait_ref = overlap.impl_header.trait_ref.unwrap(); + let self_ty = trait_ref.self_ty(); + OverlapError { + with_impl: possible_sibling, + trait_desc: trait_ref.to_string(), + // only report the Self type if it has at least + // some outer concrete shell; otherwise, it's + // not adding much information. + self_desc: if self_ty.has_concrete_skeleton() { + Some(self_ty.to_string()) + } else { + None + }, + intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes, + } + }; + let tcx = tcx.global_tcx(); let (le, ge) = tcx.infer_ctxt().enter(|infcx| { let overlap = traits::overlapping_impls(&infcx, possible_sibling, - impl_def_id); + impl_def_id, + traits::IntercrateMode::Issue43355); if let Some(overlap) = overlap { if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) { return Ok((false, false)); @@ -125,22 +147,7 @@ impl<'a, 'gcx, 'tcx> Children { let ge = tcx.specializes((possible_sibling, impl_def_id)); if le == ge { - // overlap, but no specialization; error out - let trait_ref = overlap.impl_header.trait_ref.unwrap(); - let self_ty = trait_ref.self_ty(); - Err(OverlapError { - with_impl: possible_sibling, - trait_desc: trait_ref.to_string(), - // only report the Self type if it has at least - // some outer concrete shell; otherwise, it's - // not adding much information. - self_desc: if self_ty.has_concrete_skeleton() { - Some(self_ty.to_string()) - } else { - None - }, - intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes, - }) + Err(overlap_error(overlap)) } else { Ok((le, ge)) } @@ -163,6 +170,19 @@ impl<'a, 'gcx, 'tcx> Children { *slot = impl_def_id; return Ok(Inserted::Replaced(possible_sibling)); } else { + if !tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) { + tcx.infer_ctxt().enter(|infcx| { + if let Some(overlap) = traits::overlapping_impls( + &infcx, + possible_sibling, + impl_def_id, + traits::IntercrateMode::Fixed) + { + last_lint = Some(overlap_error(overlap)); + } + }); + } + // no overlap (error bailed already via ?) } } @@ -170,7 +190,7 @@ impl<'a, 'gcx, 'tcx> Children { // no overlap with any potential siblings, so add as a new sibling debug!("placing as new sibling"); self.insert_blindly(tcx, impl_def_id); - Ok(Inserted::BecameNewSibling) + Ok(Inserted::BecameNewSibling(last_lint)) } fn iter_mut(&'a mut self) -> Box<Iterator<Item = &'a mut DefId> + 'a> { @@ -199,7 +219,7 @@ impl<'a, 'gcx, 'tcx> Graph { pub fn insert(&mut self, tcx: TyCtxt<'a, 'gcx, 'tcx>, impl_def_id: DefId) - -> Result<(), OverlapError> { + -> Result<Option<OverlapError>, OverlapError> { assert!(impl_def_id.is_local()); let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); @@ -220,10 +240,11 @@ impl<'a, 'gcx, 'tcx> Graph { self.parent.insert(impl_def_id, trait_def_id); self.children.entry(trait_def_id).or_insert(Children::new()) .insert_blindly(tcx, impl_def_id); - return Ok(()); + return Ok(None); } let mut parent = trait_def_id; + let mut last_lint = None; let simplified = fast_reject::simplify_type(tcx, trait_ref.self_ty(), false); // Descend the specialization tree, where `parent` is the current parent node @@ -234,7 +255,8 @@ impl<'a, 'gcx, 'tcx> Graph { .insert(tcx, impl_def_id, simplified)?; match insert_result { - BecameNewSibling => { + BecameNewSibling(opt_lint) => { + last_lint = opt_lint; break; } Replaced(new_child) => { @@ -251,7 +273,7 @@ impl<'a, 'gcx, 'tcx> Graph { } self.parent.insert(impl_def_id, parent); - Ok(()) + Ok(last_lint) } /// Insert cached metadata mapping from a child impl back to its parent. diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index fc05f8f0dc2..8b41dd62742 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -248,10 +248,13 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { reference: "issue #46043 <https://github.com/rust-lang/rust/issues/46043>", }, FutureIncompatibleInfo { + id: LintId::of(INCOHERENT_FUNDAMENTAL_IMPLS), + reference: "issue #46205 <https://github.com/rust-lang/rust/issues/46205>", + }, + FutureIncompatibleInfo { id: LintId::of(COERCE_NEVER), reference: "issue #46325 <https://github.com/rust-lang/rust/issues/46325>", }, - ]); // Register renamed and removed lints diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs index 1355f711a4b..07d5f813cbb 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/src/librustc_typeck/coherence/inherent_impls_overlap.rs @@ -12,9 +12,11 @@ use namespace::Namespace; use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; -use rustc::traits; +use rustc::traits::{self, IntercrateMode}; use rustc::ty::TyCtxt; +use lint; + pub fn crate_inherent_impls_overlap_check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum) { assert_eq!(crate_num, LOCAL_CRATE); @@ -28,7 +30,8 @@ struct InherentOverlapChecker<'a, 'tcx: 'a> { impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId, - overlap: traits::OverlapResult) { + overlap: traits::OverlapResult, + used_to_be_allowed: bool) { let name_and_namespace = |def_id| { let item = self.tcx.associated_item(def_id); @@ -43,11 +46,21 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { for &item2 in &impl_items2[..] { if (name, namespace) == name_and_namespace(item2) { - let mut err = struct_span_err!(self.tcx.sess, - self.tcx.span_of_impl(item1).unwrap(), - E0592, - "duplicate definitions with name `{}`", - name); + let node_id = self.tcx.hir.as_local_node_id(impl1); + let mut err = if used_to_be_allowed && node_id.is_some() { + self.tcx.struct_span_lint_node( + lint::builtin::INCOHERENT_FUNDAMENTAL_IMPLS, + node_id.unwrap(), + self.tcx.span_of_impl(item1).unwrap(), + &format!("duplicate definitions with name `{}` (E0592)", name) + ) + } else { + 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)); @@ -69,12 +82,30 @@ 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| { + let used_to_be_allowed = self.tcx.infer_ctxt().enter(|infcx| { 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) + traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id, + IntercrateMode::Issue43355) + { + self.check_for_common_items_in_impls( + impl1_def_id, impl2_def_id, overlap, false); + false + } else { + true } }); + + if used_to_be_allowed { + self.tcx.infer_ctxt().enter(|infcx| { + if let Some(overlap) = + traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id, + IntercrateMode::Fixed) + { + self.check_for_common_items_in_impls( + impl1_def_id, impl2_def_id, overlap, true); + } + }); + } } } } @@ -100,4 +131,3 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentOverlapChecker<'a, 'tcx> { fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) { } } - diff --git a/src/test/compile-fail/issue-43355.rs b/src/test/compile-fail/issue-43355.rs index b379507c1db..d793a78799a 100644 --- a/src/test/compile-fail/issue-43355.rs +++ b/src/test/compile-fail/issue-43355.rs @@ -22,6 +22,7 @@ impl<X, T> Trait1<X> for T where T: Trait2<X> { impl<X> Trait1<Box<X>> for A { //~^ ERROR conflicting implementations of trait +//~| hard error //~| downstream crates may implement trait `Trait2<std::boxed::Box<_>>` for type `A` type Output = i32; } diff --git a/src/test/run-pass/issue-43355.rs b/src/test/run-pass/issue-43355.rs new file mode 100644 index 00000000000..58d68bb955f --- /dev/null +++ b/src/test/run-pass/issue-43355.rs @@ -0,0 +1,38 @@ +// 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 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Check that the code for issue #43355 can run without an ICE, please remove +// this test when it becomes an hard error. + +#![allow(incoherent_fundamental_impls)] + +pub trait Trait1<X> { + type Output; +} +pub trait Trait2<X> {} + +impl<X, T> Trait1<X> for T where T: Trait2<X> { + type Output = (); +} +impl<X> Trait1<Box<X>> for A { + type Output = i32; +} + +pub struct A; + +fn f<X, T: Trait1<Box<X>>>() { + println!("k: {}", ::std::mem::size_of::<<T as Trait1<Box<X>>>::Output>()); +} + +pub fn g<X, T: Trait2<Box<X>>>() { + f::<X, T>(); +} + +fn main() {} |
