Skip to content

Commit

Permalink
Remove vestigial #43355-compat code
Browse files Browse the repository at this point in the history
This was previously a future-compat warning that has since been turned into
hard error, but without actually removing all the code.

Avoids a call to `traits::overlapping_impls`, which is expensive.
  • Loading branch information
jonas-schievink committed Feb 9, 2020
1 parent 8f468ee commit 2309592
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 85 deletions.
23 changes: 6 additions & 17 deletions src/librustc/traits/coherence.rs
Expand Up @@ -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;
Expand All @@ -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> {
Expand All @@ -53,7 +52,6 @@ pub fn overlapping_impls<F1, F2, R>(
tcx: TyCtxt<'_>,
impl1_def_id: DefId,
impl2_def_id: DefId,
intercrate_mode: IntercrateMode,
skip_leak_check: SkipLeakCheck,
on_overlap: F1,
no_overlap: F2,
Expand All @@ -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()
});

Expand All @@ -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())
})
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 0 additions & 7 deletions src/librustc/traits/mod.rs
Expand Up @@ -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 {
Expand Down
44 changes: 15 additions & 29 deletions src/librustc/traits/select.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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<IntercrateMode>,
intercrate: bool,

intercrate_ambiguity_causes: Option<Vec<IntercrateAmbiguityCause>>,

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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");
Expand All @@ -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<IntercrateAmbiguityCause> {
assert!(self.intercrate.is_some());
assert!(self.intercrate);
self.intercrate_ambiguity_causes.take().unwrap_or(vec![])
}

Expand Down Expand Up @@ -562,7 +558,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
) -> Result<EvaluationResult, OverflowError> {
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())
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1206,7 +1202,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
fn is_knowable<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>) -> Option<Conflict> {
debug!("is_knowable(intercrate={:?})", self.intercrate);

if !self.intercrate.is_some() {
if !self.intercrate {
return None;
}

Expand All @@ -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.
Expand All @@ -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;
}

Expand Down Expand Up @@ -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");
Expand Down
7 changes: 1 addition & 6 deletions src/librustc/traits/specialize/mod.rs
Expand Up @@ -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
}
Expand Down
30 changes: 6 additions & 24 deletions src/librustc/traits/specialize/specialization_graph.rs
Expand Up @@ -9,7 +9,6 @@ pub use rustc::traits::types::specialization_graph::*;

#[derive(Copy, Clone, Debug)]
pub enum FutureCompatOverlapErrorKind {
Issue43355,
Issue33140,
LeakCheck,
}
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions 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;
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 2309592

Please sign in to comment.