Skip to content

Commit

Permalink
remove the coherence_leak_check lint
Browse files Browse the repository at this point in the history
  • Loading branch information
lcnr committed Jan 10, 2024
1 parent fb574b2 commit e49e69b
Show file tree
Hide file tree
Showing 31 changed files with 87 additions and 386 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_index::IndexVec;
use rustc_middle::traits::specialization_graph::OverlapMode;
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::Symbol;
use rustc_trait_selection::traits::{self, SkipLeakCheck};
use rustc_trait_selection::traits;
use smallvec::SmallVec;
use std::collections::hash_map::Entry;

Expand Down Expand Up @@ -139,15 +139,8 @@ impl<'tcx> InherentOverlapChecker<'tcx> {
impl1_def_id: DefId,
impl2_def_id: DefId,
) {
let maybe_overlap = traits::overlapping_impls(
self.tcx,
impl1_def_id,
impl2_def_id,
// We go ahead and just skip the leak check for
// inherent impls without warning.
SkipLeakCheck::Yes,
overlap_mode,
);
let maybe_overlap =
traits::overlapping_impls(self.tcx, impl1_def_id, impl2_def_id, overlap_mode);

if let Some(overlap) = maybe_overlap {
self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap);
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_infer/src/infer/at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ impl<'tcx> InferCtxt<'tcx> {
tcx: self.tcx,
defining_use_anchor: self.defining_use_anchor,
considering_regions: self.considering_regions,
skip_leak_check: self.skip_leak_check,
inner: self.inner.clone(),
lexical_region_resolutions: self.lexical_region_resolutions.clone(),
selection_cache: self.selection_cache.clone(),
Expand Down
15 changes: 0 additions & 15 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,6 @@ pub struct InferCtxt<'tcx> {
/// solving is left to borrowck instead.
pub considering_regions: bool,

/// If set, this flag causes us to skip the 'leak check' during
/// higher-ranked subtyping operations. This flag is a temporary one used
/// to manage the removal of the leak-check: for the time being, we still run the
/// leak-check, but we issue warnings.
skip_leak_check: bool,

pub inner: RefCell<InferCtxtInner<'tcx>>,

/// Once region inference is done, the values for each variable.
Expand Down Expand Up @@ -606,7 +600,6 @@ pub struct InferCtxtBuilder<'tcx> {
tcx: TyCtxt<'tcx>,
defining_use_anchor: DefiningAnchor,
considering_regions: bool,
skip_leak_check: bool,
/// Whether we are in coherence mode.
intercrate: bool,
/// Whether we should use the new trait solver in the local inference context,
Expand All @@ -624,7 +617,6 @@ impl<'tcx> TyCtxtInferExt<'tcx> for TyCtxt<'tcx> {
tcx: self,
defining_use_anchor: DefiningAnchor::Error,
considering_regions: true,
skip_leak_check: false,
intercrate: false,
next_trait_solver: self.next_trait_solver_globally(),
}
Expand Down Expand Up @@ -658,11 +650,6 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
self
}

pub fn skip_leak_check(mut self, skip_leak_check: bool) -> Self {
self.skip_leak_check = skip_leak_check;
self
}

/// Given a canonical value `C` as a starting point, create an
/// inference context that contains each of the bound values
/// within instantiated as a fresh variable. The `f` closure is
Expand All @@ -688,15 +675,13 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
tcx,
defining_use_anchor,
considering_regions,
skip_leak_check,
intercrate,
next_trait_solver,
} = *self;
InferCtxt {
tcx,
defining_use_anchor,
considering_regions,
skip_leak_check,
inner: RefCell::new(InferCtxtInner::new()),
lexical_region_resolutions: RefCell::new(None),
selection_cache: Default::default(),
Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_infer/src/infer/relate/higher_ranked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,11 @@ impl<'tcx> InferCtxt<'tcx> {
outer_universe: ty::UniverseIndex,
only_consider_snapshot: Option<&CombinedSnapshot<'tcx>>,
) -> RelateResult<'tcx, ()> {
// If the user gave `-Zno-leak-check`, or we have been
// configured to skip the leak check, then skip the leak check
// completely. The leak check is deprecated. Any legitimate
// subtyping errors that it would have caught will now be
// caught later on, during region checking. However, we
// continue to use it for a transition period.
if self.tcx.sess.opts.unstable_opts.no_leak_check || self.skip_leak_check {
// If the user gave `-Zno-leak-check`, then skip the leak check
// completely. While we considered to remove the leak check at
// some point, we are now confident that it will remain in some
// form or another.
if self.tcx.sess.opts.unstable_opts.no_leak_check {
return Ok(());
}

Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,11 @@ fn register_builtins(store: &mut LintStore) {
"converted into hard error, see issue #48950 \
<https:/rust-lang/rust/issues/48950> for more information",
);
store.register_removed(
"coherence_leak_check",
"no longer a warning, see issue #119820 \
<https:/rust-lang/rust/issues/119820> for more information",
);
store.register_removed(
"resolve_trait_on_defaulted_unit",
"converted into hard error, see issue #48950 \
Expand Down
41 changes: 0 additions & 41 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ declare_lint_pass! {
BREAK_WITH_LABEL_AND_LOOP,
BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE,
CENUM_IMPL_DROP_CAST,
COHERENCE_LEAK_CHECK,
CONFLICTING_REPR_HINTS,
CONST_EVALUATABLE_UNCHECKED,
CONST_ITEM_MUTATION,
Expand Down Expand Up @@ -1467,46 +1466,6 @@ declare_lint! {
};
}

declare_lint! {
/// The `coherence_leak_check` lint detects conflicting implementations of
/// a trait that are only distinguished by the old leak-check code.
///
/// ### Example
///
/// ```rust
/// trait SomeTrait { }
/// impl SomeTrait for for<'a> fn(&'a u8) { }
/// impl<'a> SomeTrait for fn(&'a u8) { }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// In the past, the compiler would accept trait implementations for
/// identical functions that differed only in where the lifetime binder
/// appeared. Due to a change in the borrow checker implementation to fix
/// several bugs, this is no longer allowed. However, since this affects
/// existing code, this is a [future-incompatible] lint to transition this
/// to a hard error in the future.
///
/// Code relying on this pattern should introduce "[newtypes]",
/// like `struct Foo(for<'a> fn(&'a u8))`.
///
/// See [issue #56105] for more details.
///
/// [issue #56105]: https:/rust-lang/rust/issues/56105
/// [newtypes]: https://doc.rust-lang.org/book/ch19-04-advanced-types.html#using-the-newtype-pattern-for-type-safety-and-abstraction
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub COHERENCE_LEAK_CHECK,
Warn,
"distinct impls distinguished only by the leak-check code",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
reference: "issue #56105 <https:/rust-lang/rust/issues/56105>",
};
}

declare_lint! {
/// The `deprecated` lint detects use of deprecated items.
///
Expand Down
38 changes: 14 additions & 24 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use crate::traits::query::evaluate_obligation::InferCtxtExt;
use crate::traits::select::IntercrateAmbiguityCause;
use crate::traits::structural_normalize::StructurallyNormalizeExt;
use crate::traits::NormalizeExt;
use crate::traits::SkipLeakCheck;
use crate::traits::{
Obligation, ObligationCause, ObligationCtxt, PredicateObligation, PredicateObligations,
SelectionContext,
Expand Down Expand Up @@ -84,12 +83,11 @@ impl TrackAmbiguityCauses {
/// If there are types that satisfy both impls, returns `Some`
/// with a suitably-freshened `ImplHeader` with those types
/// substituted. Otherwise, returns `None`.
#[instrument(skip(tcx, skip_leak_check), level = "debug")]
#[instrument(skip(tcx), level = "debug")]
pub fn overlapping_impls(
tcx: TyCtxt<'_>,
impl1_def_id: DefId,
impl2_def_id: DefId,
skip_leak_check: SkipLeakCheck,
overlap_mode: OverlapMode,
) -> Option<OverlapResult<'_>> {
// Before doing expensive operations like entering an inference context, do
Expand All @@ -114,27 +112,14 @@ pub fn overlapping_impls(
return None;
}

let _overlap_with_bad_diagnostics = overlap(
tcx,
TrackAmbiguityCauses::No,
skip_leak_check,
impl1_def_id,
impl2_def_id,
overlap_mode,
)?;
let _overlap_with_bad_diagnostics =
overlap(tcx, TrackAmbiguityCauses::No, impl1_def_id, impl2_def_id, overlap_mode)?;

// In the case where we detect an error, run the check again, but
// this time tracking intercrate ambiguity causes for better
// diagnostics. (These take time and can lead to false errors.)
let overlap = overlap(
tcx,
TrackAmbiguityCauses::Yes,
skip_leak_check,
impl1_def_id,
impl2_def_id,
overlap_mode,
)
.unwrap();
let overlap =
overlap(tcx, TrackAmbiguityCauses::Yes, impl1_def_id, impl2_def_id, overlap_mode).unwrap();
Some(overlap)
}

Expand Down Expand Up @@ -176,7 +161,6 @@ fn fresh_impl_header_normalized<'tcx>(
fn overlap<'tcx>(
tcx: TyCtxt<'tcx>,
track_ambiguity_causes: TrackAmbiguityCauses,
skip_leak_check: SkipLeakCheck,
impl1_def_id: DefId,
impl2_def_id: DefId,
overlap_mode: OverlapMode,
Expand All @@ -192,7 +176,6 @@ fn overlap<'tcx>(
let infcx = tcx
.infer_ctxt()
.with_opaque_type_inference(DefiningAnchor::Bubble)
.skip_leak_check(skip_leak_check.is_yes())
.intercrate(true)
.with_next_trait_solver(tcx.next_trait_solver_in_coherence())
.build();
Expand Down Expand Up @@ -230,8 +213,15 @@ fn overlap<'tcx>(
}
}

// We toggle the `leak_check` by using `skip_leak_check` when constructing the
// inference context, so this may be a noop.
// Detect any region errors caused by equating these two impls.
//
// Only higher ranked region errors are possible here, given that we
// replaced all parameter regions with existentials.
//
// Unlike a full region check, which sometimes incompletely handles
// `TypeOutlives` constraints, the leak check is a complete. While the
// leak check does not detect all region errors, it never
// fails in cases which would later pass full region checking.
if infcx.leak_check(ty::UniverseIndex::ROOT, None).is_err() {
debug!("overlap: leak check failed");
return None;
Expand Down
18 changes: 0 additions & 18 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,6 @@ pub use self::util::{get_vtable_index_of_object_method, impl_item_is_final, upca

pub use rustc_infer::traits::*;

/// Whether to skip the leak check, as part of a future compatibility warning step.
///
/// The "default" for skip-leak-check corresponds to the current
/// behavior (do not skip the leak check) -- not the behavior we are
/// transitioning into.
#[derive(Copy, Clone, PartialEq, Eq, Debug, Default)]
pub enum SkipLeakCheck {
Yes,
#[default]
No,
}

impl SkipLeakCheck {
fn is_yes(self) -> bool {
self == SkipLeakCheck::Yes
}
}

/// The mode that trait queries run in.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum TraitQueryMode {
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_trait_selection/src/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use rustc_errors::{error_code, DelayDm, Diagnostic};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{GenericArgs, GenericArgsRef};
use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK;
use rustc_session::lint::builtin::ORDER_DEPENDENT_TRAIT_OBJECTS;
use rustc_span::{Span, DUMMY_SP};

Expand Down Expand Up @@ -441,7 +440,6 @@ fn report_conflicting_impls<'tcx>(
Some(kind) => {
let lint = match kind {
FutureCompatOverlapErrorKind::Issue33140 => ORDER_DEPENDENT_TRAIT_OBJECTS,
FutureCompatOverlapErrorKind::LeakCheck => COHERENCE_LEAK_CHECK,
};
tcx.struct_span_lint_hir(
lint,
Expand Down
Loading

0 comments on commit e49e69b

Please sign in to comment.