diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index 7cb1f3aa0e8..425f14a5740 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -647,7 +647,8 @@ rustc_queries! { query trait_impls_of(key: DefId) -> &'tcx ty::trait_def::TraitImpls { desc { |tcx| "trait impls of `{}`", tcx.def_path_str(key) } } - query specialization_graph_of(_: DefId) -> &'tcx specialization_graph::Graph { + query specialization_graph_of(key: DefId) -> &'tcx specialization_graph::Graph { + desc { |tcx| "building specialization graph of trait `{}`", tcx.def_path_str(key) } cache_on_disk_if { true } } query is_object_safe(key: DefId) -> bool { diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 855da0367de..2a667b53550 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -6,7 +6,6 @@ use crate::infer::{CombinedSnapshot, InferOk}; use crate::traits::select::IntercrateAmbiguityCause; -use crate::traits::IntercrateMode; use crate::traits::SkipLeakCheck; use crate::traits::{self, Normalized, Obligation, ObligationCause, SelectionContext}; use crate::ty::fold::TypeFoldable; @@ -27,7 +26,7 @@ enum InCrate { #[derive(Debug, Copy, Clone)] pub enum Conflict { Upstream, - Downstream { used_to_be_broken: bool }, + Downstream, } pub struct OverlapResult<'tcx> { @@ -53,7 +52,6 @@ pub fn overlapping_impls( tcx: TyCtxt<'_>, impl1_def_id: DefId, impl2_def_id: DefId, - intercrate_mode: IntercrateMode, skip_leak_check: SkipLeakCheck, on_overlap: F1, no_overlap: F2, @@ -65,13 +63,12 @@ where debug!( "overlapping_impls(\ impl1_def_id={:?}, \ - impl2_def_id={:?}, - intercrate_mode={:?})", - impl1_def_id, impl2_def_id, intercrate_mode + impl2_def_id={:?})", + impl1_def_id, impl2_def_id, ); let overlaps = tcx.infer_ctxt().enter(|infcx| { - let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode); + let selcx = &mut SelectionContext::intercrate(&infcx); overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).is_some() }); @@ -83,7 +80,7 @@ where // this time tracking intercrate ambuiguity causes for better // diagnostics. (These take time and can lead to false errors.) tcx.infer_ctxt().enter(|infcx| { - let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode); + let selcx = &mut SelectionContext::intercrate(&infcx); selcx.enable_tracking_intercrate_ambiguity_causes(); on_overlap(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).unwrap()) }) @@ -202,15 +199,7 @@ pub fn trait_ref_is_knowable<'tcx>( if orphan_check_trait_ref(tcx, trait_ref, InCrate::Remote).is_ok() { // A downstream or cousin crate is allowed to implement some // substitution of this trait-ref. - - // A trait can be implementable for a trait ref by both the current - // crate and crates downstream of it. Older versions of rustc - // were not aware of this, causing incoherence (issue #43355). - let used_to_be_broken = orphan_check_trait_ref(tcx, trait_ref, InCrate::Local).is_ok(); - if used_to_be_broken { - debug!("trait_ref_is_knowable({:?}) - USED TO BE BROKEN", trait_ref); - } - return Some(Conflict::Downstream { used_to_be_broken }); + return Some(Conflict::Downstream); } if trait_ref_is_local_or_fundamental(tcx, trait_ref) { diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 783807b5c3b..417b52c38a7 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -78,13 +78,6 @@ pub use self::chalk_fulfill::{ pub use self::types::*; -/// Whether to enable bug compatibility with issue #43355. -#[derive(Copy, Clone, PartialEq, Eq, Debug)] -pub enum IntercrateMode { - Issue43355, - Fixed, -} - /// Whether to skip the leak check, as part of a future compatibility warning step. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum SkipLeakCheck { diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index f298d150c6f..cfeb392f87f 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -19,8 +19,8 @@ use super::DerivedObligationCause; use super::Selection; use super::SelectionResult; use super::TraitNotObjectSafe; +use super::TraitQueryMode; use super::{BuiltinDerivedObligation, ImplDerivedObligation, ObligationCauseCode}; -use super::{IntercrateMode, TraitQueryMode}; use super::{ObjectCastObligation, Obligation}; use super::{ObligationCause, PredicateObligation, TraitObligation}; use super::{OutputTypeParameterMismatch, Overflow, SelectionError, Unimplemented}; @@ -80,7 +80,7 @@ pub struct SelectionContext<'cx, 'tcx> { /// 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: Option, + intercrate: bool, intercrate_ambiguity_causes: Option>, @@ -218,22 +218,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { SelectionContext { infcx, freshener: infcx.freshener(), - intercrate: None, + intercrate: false, intercrate_ambiguity_causes: None, allow_negative_impls: false, query_mode: TraitQueryMode::Standard, } } - pub fn intercrate( - infcx: &'cx InferCtxt<'cx, 'tcx>, - mode: IntercrateMode, - ) -> SelectionContext<'cx, 'tcx> { - debug!("intercrate({:?})", mode); + pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'tcx>) -> SelectionContext<'cx, 'tcx> { SelectionContext { infcx, freshener: infcx.freshener(), - intercrate: Some(mode), + intercrate: true, intercrate_ambiguity_causes: None, allow_negative_impls: false, query_mode: TraitQueryMode::Standard, @@ -248,7 +244,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { SelectionContext { infcx, freshener: infcx.freshener(), - intercrate: None, + intercrate: false, intercrate_ambiguity_causes: None, allow_negative_impls, query_mode: TraitQueryMode::Standard, @@ -263,7 +259,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { SelectionContext { infcx, freshener: infcx.freshener(), - intercrate: None, + intercrate: false, intercrate_ambiguity_causes: None, allow_negative_impls: false, query_mode, @@ -276,7 +272,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { /// false overflow results (#47139) and because it costs /// computation time. pub fn enable_tracking_intercrate_ambiguity_causes(&mut self) { - assert!(self.intercrate.is_some()); + assert!(self.intercrate); assert!(self.intercrate_ambiguity_causes.is_none()); self.intercrate_ambiguity_causes = Some(vec![]); debug!("selcx: enable_tracking_intercrate_ambiguity_causes"); @@ -286,7 +282,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { /// was enabled and disables tracking at the same time. If /// tracking is not enabled, just returns an empty vector. pub fn take_intercrate_ambiguity_causes(&mut self) -> Vec { - assert!(self.intercrate.is_some()); + assert!(self.intercrate); self.intercrate_ambiguity_causes.take().unwrap_or(vec![]) } @@ -562,7 +558,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ) -> Result { debug!("evaluate_trait_predicate_recursively({:?})", obligation); - if self.intercrate.is_none() + if !self.intercrate && obligation.is_global() && obligation.param_env.caller_bounds.iter().all(|bound| bound.needs_subst()) { @@ -727,7 +723,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { stack.fresh_trait_ref.skip_binder().input_types().any(|ty| ty.is_fresh()); // This check was an imperfect workaround for a bug in the old // intercrate mode; it should be removed when that goes away. - if unbound_input_types && self.intercrate == Some(IntercrateMode::Issue43355) { + if unbound_input_types && self.intercrate { debug!( "evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous", stack.fresh_trait_ref @@ -1206,7 +1202,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { fn is_knowable<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>) -> Option { debug!("is_knowable(intercrate={:?})", self.intercrate); - if !self.intercrate.is_some() { + if !self.intercrate { return None; } @@ -1218,17 +1214,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // bound regions. let trait_ref = predicate.skip_binder().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 - } + coherence::trait_ref_is_knowable(self.tcx(), trait_ref) } /// Returns `true` if the global caches can be used. @@ -1249,7 +1235,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, '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.is_some() { + if self.intercrate { return false; } @@ -3305,7 +3291,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return Err(()); } - if self.intercrate.is_none() + if !self.intercrate && self.tcx().impl_polarity(impl_def_id) == ty::ImplPolarity::Reservation { debug!("match_impl: reservation impls only apply in intercrate mode"); diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 8b68d6f2603..071b5277dd9 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -332,14 +332,9 @@ pub(super) fn specialization_graph_provider( let impl_span = tcx.sess.source_map().def_span(tcx.span_of_impl(impl_def_id).unwrap()); let mut err = match used_to_be_allowed { - Some(FutureCompatOverlapErrorKind::Issue43355) | None => { - struct_span_err!(tcx.sess, impl_span, E0119, "{}", msg) - } + None => struct_span_err!(tcx.sess, impl_span, E0119, "{}", msg), Some(kind) => { let lint = match kind { - FutureCompatOverlapErrorKind::Issue43355 => { - unreachable!("converted to hard error above") - } FutureCompatOverlapErrorKind::Issue33140 => { ORDER_DEPENDENT_TRAIT_OBJECTS } diff --git a/src/librustc/traits/specialize/specialization_graph.rs b/src/librustc/traits/specialize/specialization_graph.rs index 98908e672f0..ca7740199ec 100644 --- a/src/librustc/traits/specialize/specialization_graph.rs +++ b/src/librustc/traits/specialize/specialization_graph.rs @@ -9,7 +9,6 @@ pub use rustc::traits::types::specialization_graph::*; #[derive(Copy, Clone, Debug)] pub enum FutureCompatOverlapErrorKind { - Issue43355, Issue33140, LeakCheck, } @@ -107,16 +106,16 @@ impl<'tcx> Children { } }; + let allowed_to_overlap = + tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling); + let (le, ge) = traits::overlapping_impls( tcx, possible_sibling, impl_def_id, - traits::IntercrateMode::Issue43355, traits::SkipLeakCheck::default(), |overlap| { - if let Some(overlap_kind) = - tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) - { + if let Some(overlap_kind) = &allowed_to_overlap { match overlap_kind { ty::ImplOverlapKind::Permitted { marker: _ } => {} ty::ImplOverlapKind::Issue33140 => { @@ -154,31 +153,14 @@ impl<'tcx> Children { replace_children.push(possible_sibling); } else { - if let None = tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) { - // do future-compat checks for overlap. Have issue #33140 - // errors overwrite issue #43355 errors when both are present. - - traits::overlapping_impls( - tcx, - possible_sibling, - impl_def_id, - traits::IntercrateMode::Fixed, - traits::SkipLeakCheck::default(), - |overlap| { - last_lint = Some(FutureCompatOverlapError { - error: overlap_error(overlap), - kind: FutureCompatOverlapErrorKind::Issue43355, - }); - }, - || (), - ); + if let None = allowed_to_overlap { + // Do future-compat checks for overlap. if last_lint.is_none() { traits::overlapping_impls( tcx, possible_sibling, impl_def_id, - traits::IntercrateMode::Fixed, traits::SkipLeakCheck::Yes, |overlap| { last_lint = Some(FutureCompatOverlapError { diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index 79a006a898a..1970b1e5c5d 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -18,14 +18,12 @@ use rustc_hir::def_id::DefId; use rustc_hir::ItemKind; pub fn check_trait(tcx: TyCtxt<'_>, trait_def_id: DefId) { + let lang_items = tcx.lang_items(); Checker { tcx, trait_def_id } - .check(tcx.lang_items().drop_trait(), visit_implementation_of_drop) - .check(tcx.lang_items().copy_trait(), visit_implementation_of_copy) - .check(tcx.lang_items().coerce_unsized_trait(), visit_implementation_of_coerce_unsized) - .check( - tcx.lang_items().dispatch_from_dyn_trait(), - visit_implementation_of_dispatch_from_dyn, - ); + .check(lang_items.drop_trait(), visit_implementation_of_drop) + .check(lang_items.copy_trait(), visit_implementation_of_copy) + .check(lang_items.coerce_unsized_trait(), visit_implementation_of_coerce_unsized) + .check(lang_items.dispatch_from_dyn_trait(), visit_implementation_of_dispatch_from_dyn); } struct Checker<'tcx> { diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs index fb9c173f520..ffea849c4f2 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/src/librustc_typeck/coherence/inherent_impls_overlap.rs @@ -1,5 +1,5 @@ use crate::namespace::Namespace; -use rustc::traits::{self, IntercrateMode, SkipLeakCheck}; +use rustc::traits::{self, SkipLeakCheck}; use rustc::ty::{AssocItem, TyCtxt}; use rustc_errors::struct_span_err; use rustc_hir as hir; @@ -93,7 +93,6 @@ impl InherentOverlapChecker<'tcx> { self.tcx, impl1_def_id, impl2_def_id, - IntercrateMode::Issue43355, // We go ahead and just skip the leak check for // inherent impls without warning. SkipLeakCheck::Yes, diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 5583e3418b2..1526182576c 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -10,7 +10,7 @@ use rustc::ty::query::Providers; use rustc::ty::{self, TyCtxt, TypeFoldable}; use rustc_errors::struct_span_err; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; -use rustc_hir::HirId; +use rustc_span::Span; mod builtin; mod inherent_impls; @@ -18,37 +18,35 @@ mod inherent_impls_overlap; mod orphan; mod unsafety; -fn check_impl(tcx: TyCtxt<'_>, hir_id: HirId) { - let impl_def_id = tcx.hir().local_def_id(hir_id); +/// Obtains the span of just the impl header of `impl_def_id`. +fn impl_header_span(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Span { + tcx.sess.source_map().def_span(tcx.span_of_impl(impl_def_id).unwrap()) +} - // If there are no traits, then this implementation must have a - // base type. +fn check_impl(tcx: TyCtxt<'_>, impl_def_id: DefId, trait_ref: ty::TraitRef<'_>) { + debug!( + "(checking implementation) adding impl for trait '{:?}', item '{}'", + trait_ref, + tcx.def_path_str(impl_def_id) + ); - if let Some(trait_ref) = tcx.impl_trait_ref(impl_def_id) { - debug!( - "(checking implementation) adding impl for trait '{:?}', item '{}'", - trait_ref, - tcx.def_path_str(impl_def_id) - ); - - // Skip impls where one of the self type is an error type. - // This occurs with e.g., resolve failures (#30589). - if trait_ref.references_error() { - return; - } - - enforce_trait_manually_implementable(tcx, impl_def_id, trait_ref.def_id); - enforce_empty_impls_for_marker_traits(tcx, impl_def_id, trait_ref.def_id); + // Skip impls where one of the self type is an error type. + // This occurs with e.g., resolve failures (#30589). + if trait_ref.references_error() { + return; } + + enforce_trait_manually_implementable(tcx, impl_def_id, trait_ref.def_id); + enforce_empty_impls_for_marker_traits(tcx, impl_def_id, trait_ref.def_id); } fn enforce_trait_manually_implementable(tcx: TyCtxt<'_>, impl_def_id: DefId, trait_def_id: DefId) { let did = Some(trait_def_id); let li = tcx.lang_items(); - let span = tcx.sess.source_map().def_span(tcx.span_of_impl(impl_def_id).unwrap()); // Disallow *all* explicit impls of `Sized` and `Unsize` for now. if did == li.sized_trait() { + let span = impl_header_span(tcx, impl_def_id); struct_span_err!( tcx.sess, span, @@ -61,6 +59,7 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt<'_>, impl_def_id: DefId, tra } if did == li.unsize_trait() { + let span = impl_header_span(tcx, impl_def_id); struct_span_err!( tcx.sess, span, @@ -86,6 +85,8 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt<'_>, impl_def_id: DefId, tra } else { return; // everything OK }; + + let span = impl_header_span(tcx, impl_def_id); struct_span_err!( tcx.sess, span, @@ -109,7 +110,7 @@ fn enforce_empty_impls_for_marker_traits(tcx: TyCtxt<'_>, impl_def_id: DefId, tr return; } - let span = tcx.sess.source_map().def_span(tcx.span_of_impl(impl_def_id).unwrap()); + let span = impl_header_span(tcx, impl_def_id); struct_span_err!(tcx.sess, span, E0715, "impls for marker traits cannot contain items").emit(); } @@ -129,12 +130,17 @@ pub fn provide(providers: &mut Providers<'_>) { } fn coherent_trait(tcx: TyCtxt<'_>, def_id: DefId) { + // Trigger building the specialization graph for the trait. This will detect and report any + // overlap errors. + tcx.specialization_graph_of(def_id); + let impls = tcx.hir().trait_impls(def_id); - for &impl_id in impls { - check_impl(tcx, impl_id); - } - for &impl_id in impls { - check_impl_overlap(tcx, impl_id); + for &hir_id in impls { + let impl_def_id = tcx.hir().local_def_id(hir_id); + let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); + + check_impl(tcx, impl_def_id, trait_ref); + check_object_overlap(tcx, impl_def_id, trait_ref); } builtin::check_trait(tcx, def_id); } @@ -152,12 +158,12 @@ pub fn check_coherence(tcx: TyCtxt<'_>) { tcx.ensure().crate_inherent_impls_overlap_check(LOCAL_CRATE); } -/// Overlap: no two impls for the same trait are implemented for the -/// same type. Likewise, no two inherent impls for a given type -/// constructor provide a method with the same name. -fn check_impl_overlap<'tcx>(tcx: TyCtxt<'tcx>, hir_id: HirId) { - let impl_def_id = tcx.hir().local_def_id(hir_id); - let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap(); +/// Checks whether an impl overlaps with the automatic `impl Trait for dyn Trait`. +fn check_object_overlap<'tcx>( + tcx: TyCtxt<'tcx>, + impl_def_id: DefId, + trait_ref: ty::TraitRef<'tcx>, +) { let trait_def_id = trait_ref.def_id; if trait_ref.references_error() { @@ -165,11 +171,7 @@ fn check_impl_overlap<'tcx>(tcx: TyCtxt<'tcx>, hir_id: HirId) { return; } - // Trigger building the specialization graph for the trait of this impl. - // This will detect any overlap errors. - tcx.specialization_graph_of(trait_def_id); - - // check for overlap with the automatic `impl Trait for Trait` + // check for overlap with the automatic `impl Trait for dyn Trait` if let ty::Dynamic(ref data, ..) = trait_ref.self_ty().kind { // This is something like impl Trait1 for Trait2. Illegal // if Trait1 is a supertrait of Trait2 or Trait2 is not object safe. @@ -194,17 +196,17 @@ fn check_impl_overlap<'tcx>(tcx: TyCtxt<'tcx>, hir_id: HirId) { } else { let mut supertrait_def_ids = traits::supertrait_def_ids(tcx, component_def_id); if supertrait_def_ids.any(|d| d == trait_def_id) { - let sp = tcx.sess.source_map().def_span(tcx.span_of_impl(impl_def_id).unwrap()); + let span = impl_header_span(tcx, impl_def_id); struct_span_err!( tcx.sess, - sp, + span, E0371, "the object type `{}` automatically implements the trait `{}`", trait_ref.self_ty(), tcx.def_path_str(trait_def_id) ) .span_label( - sp, + span, format!( "`{}` automatically implements trait `{}`", trait_ref.self_ty(), diff --git a/src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr b/src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr index e5cc298a435..71f997c54c6 100644 --- a/src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr +++ b/src/test/ui/coherence/coherence-inherited-assoc-ty-cycle-err.stderr @@ -1,10 +1,10 @@ -error[E0391]: cycle detected when processing `Trait` +error[E0391]: cycle detected when building specialization graph of trait `Trait` --> $DIR/coherence-inherited-assoc-ty-cycle-err.rs:8:1 | LL | trait Trait { type Assoc; } | ^^^^^^^^^^^^^^ | - = note: ...which again requires processing `Trait`, completing the cycle + = note: ...which again requires building specialization graph of trait `Trait`, completing the cycle note: cycle used when coherence checking all impls of trait `Trait` --> $DIR/coherence-inherited-assoc-ty-cycle-err.rs:8:1 |