Skip to content

Commit

Permalink
lint impls that will become incoherent when leak-check is removed
Browse files Browse the repository at this point in the history
  • Loading branch information
nikomatsakis committed Feb 6, 2020
1 parent e9c7894 commit 363faba
Show file tree
Hide file tree
Showing 13 changed files with 166 additions and 17 deletions.
7 changes: 4 additions & 3 deletions src/librustc/infer/mod.rs
Expand Up @@ -836,14 +836,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
r
}

/// Execute `f` then unroll any bindings it creates.
pub fn skip_leak_check<R, F>(&self, f: F) -> R
/// If `should_skip` is true, then execute `f` then unroll any bindings it creates.
pub fn probe_maybe_skip_leak_check<R, F>(&self, should_skip: bool, f: F) -> R
where
F: FnOnce(&CombinedSnapshot<'a, 'tcx>) -> R,
{
debug!("probe()");
let snapshot = self.start_snapshot();
self.skip_leak_check.set(true);
let skip_leak_check = should_skip || self.skip_leak_check.get();
self.skip_leak_check.set(skip_leak_check);
let r = f(&snapshot);
self.rollback_to("probe", snapshot);
r
Expand Down
15 changes: 11 additions & 4 deletions src/librustc/traits/coherence.rs
Expand Up @@ -7,6 +7,7 @@
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;
use crate::ty::subst::Subst;
Expand Down Expand Up @@ -53,6 +54,7 @@ pub fn overlapping_impls<F1, F2, R>(
impl1_def_id: DefId,
impl2_def_id: DefId,
intercrate_mode: IntercrateMode,
skip_leak_check: SkipLeakCheck,
on_overlap: F1,
no_overlap: F2,
) -> R
Expand All @@ -70,7 +72,7 @@ where

let overlaps = tcx.infer_ctxt().enter(|infcx| {
let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode);
overlap(selcx, impl1_def_id, impl2_def_id).is_some()
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).is_some()
});

if !overlaps {
Expand All @@ -83,7 +85,7 @@ where
tcx.infer_ctxt().enter(|infcx| {
let selcx = &mut SelectionContext::intercrate(&infcx, intercrate_mode);
selcx.enable_tracking_intercrate_ambiguity_causes();
on_overlap(overlap(selcx, impl1_def_id, impl2_def_id).unwrap())
on_overlap(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).unwrap())
})
}

Expand Down Expand Up @@ -113,12 +115,15 @@ fn with_fresh_ty_vars<'cx, 'tcx>(
/// where-clauses)? If so, returns an `ImplHeader` that unifies the two impls.
fn overlap<'cx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'tcx>,
skip_leak_check: SkipLeakCheck,
a_def_id: DefId,
b_def_id: DefId,
) -> Option<OverlapResult<'tcx>> {
debug!("overlap(a_def_id={:?}, b_def_id={:?})", a_def_id, b_def_id);

selcx.infcx().probe(|snapshot| overlap_within_probe(selcx, a_def_id, b_def_id, snapshot))
selcx.infcx().probe_maybe_skip_leak_check(skip_leak_check.is_yes(), |snapshot| {
overlap_within_probe(selcx, a_def_id, b_def_id, snapshot)
})
}

fn overlap_within_probe(
Expand Down Expand Up @@ -146,7 +151,9 @@ fn overlap_within_probe(
.eq_impl_headers(&a_impl_header, &b_impl_header)
{
Ok(InferOk { obligations, value: () }) => obligations,
Err(_) => return None,
Err(_) => {
return None;
}
};

debug!("overlap: unification check succeeded");
Expand Down
22 changes: 22 additions & 0 deletions src/librustc/traits/mod.rs
Expand Up @@ -83,6 +83,28 @@ pub enum IntercrateMode {
Fixed,
}

/// Whether to skip the leak check, as part of a future compatibility warning step.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum SkipLeakCheck {
Yes,
No,
}

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

/// The "default" for skip-leak-check corresponds to the current
/// behavior (do not skip the leak check) -- not the behavior we are
/// transitioning into.
impl Default for SkipLeakCheck {
fn default() -> Self {
SkipLeakCheck::No
}
}

/// The mode that trait queries run in.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum TraitQueryMode {
Expand Down
6 changes: 4 additions & 2 deletions src/librustc/traits/specialize/mod.rs
Expand Up @@ -19,6 +19,7 @@ use crate::ty::{self, TyCtxt, TypeFoldable};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::struct_span_err;
use rustc_hir::def_id::DefId;
use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK;
use rustc_session::lint::builtin::ORDER_DEPENDENT_TRAIT_OBJECTS;
use rustc_span::DUMMY_SP;

Expand Down Expand Up @@ -97,7 +98,7 @@ pub fn translate_substs<'a, 'tcx>(
|_| {
bug!(
"When translating substitutions for specialization, the expected \
specialization failed to hold"
specialization failed to hold"
)
},
)
Expand Down Expand Up @@ -268,7 +269,7 @@ fn fulfill_implication<'a, 'tcx>(
// no dice!
debug!(
"fulfill_implication: for impls on {:?} and {:?}, \
could not fulfill: {:?} given {:?}",
could not fulfill: {:?} given {:?}",
source_trait_ref, target_trait_ref, errors, param_env.caller_bounds
);
Err(())
Expand Down Expand Up @@ -342,6 +343,7 @@ pub(super) fn specialization_graph_provider(
FutureCompatOverlapErrorKind::Issue33140 => {
ORDER_DEPENDENT_TRAIT_OBJECTS
}
FutureCompatOverlapErrorKind::LeakCheck => COHERENCE_LEAK_CHECK,
};
tcx.struct_span_lint_hir(
lint,
Expand Down
24 changes: 22 additions & 2 deletions src/librustc/traits/specialize/specialization_graph.rs
Expand Up @@ -11,6 +11,7 @@ pub use rustc::traits::types::specialization_graph::*;
pub enum FutureCompatOverlapErrorKind {
Issue43355,
Issue33140,
LeakCheck,
}

#[derive(Debug)]
Expand Down Expand Up @@ -111,6 +112,7 @@ impl<'tcx> Children {
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)
Expand Down Expand Up @@ -161,6 +163,7 @@ impl<'tcx> Children {
possible_sibling,
impl_def_id,
traits::IntercrateMode::Fixed,
traits::SkipLeakCheck::default(),
|overlap| {
last_lint = Some(FutureCompatOverlapError {
error: overlap_error(overlap),
Expand All @@ -169,6 +172,23 @@ impl<'tcx> Children {
},
|| (),
);

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 {
error: overlap_error(overlap),
kind: FutureCompatOverlapErrorKind::LeakCheck,
});
},
|| (),
);
}
}

// no overlap (error bailed already via ?)
Expand Down Expand Up @@ -247,7 +267,7 @@ impl<'tcx> Graph {
if trait_ref.references_error() {
debug!(
"insert: inserting dummy node for erroneous TraitRef {:?}, \
impl_def_id={:?}, trait_def_id={:?}",
impl_def_id={:?}, trait_def_id={:?}",
trait_ref, impl_def_id, trait_def_id
);

Expand Down Expand Up @@ -326,7 +346,7 @@ impl<'tcx> Graph {
if self.parent.insert(child, parent).is_some() {
bug!(
"When recording an impl from the crate store, information about its parent \
was already present."
was already present."
);
}

Expand Down
11 changes: 11 additions & 0 deletions src/librustc_session/lint/builtin.rs
Expand Up @@ -260,6 +260,16 @@ declare_lint! {
};
}

declare_lint! {
pub COHERENCE_LEAK_CHECK,
Deny,
"distinct impls distinguished only by the leak-check code",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #56105 <https://github.com/rust-lang/rust/issues/56105>",
edition: None,
};
}

declare_lint! {
pub DEPRECATED,
Warn,
Expand Down Expand Up @@ -509,6 +519,7 @@ declare_lint_pass! {
MISSING_FRAGMENT_SPECIFIER,
LATE_BOUND_LIFETIME_ARGUMENTS,
ORDER_DEPENDENT_TRAIT_OBJECTS,
COHERENCE_LEAK_CHECK,
DEPRECATED,
UNUSED_UNSAFE,
UNUSED_MUT,
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_typeck/coherence/inherent_impls_overlap.rs
@@ -1,5 +1,5 @@
use crate::namespace::Namespace;
use rustc::traits::{self, IntercrateMode};
use rustc::traits::{self, IntercrateMode, SkipLeakCheck};
use rustc::ty::TyCtxt;
use rustc_errors::struct_span_err;
use rustc_hir as hir;
Expand Down Expand Up @@ -76,6 +76,9 @@ impl InherentOverlapChecker<'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,
|overlap| {
self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap);
false
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/coherence/coherence-inherited-subtyping.old.stderr
@@ -0,0 +1,14 @@
error[E0592]: duplicate definitions with name `method1`
--> $DIR/coherence-inherited-subtyping.rs:14:5
|
LL | fn method1(&self) {}
| ^^^^^^^^^^^^^^^^^^^^ duplicate definitions for `method1`
...
LL | fn method1(&self) {}
| -------------------- other definition for `method1`
|
= note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details

error: aborting due to previous error

For more information about this error, try `rustc --explain E0592`.
14 changes: 14 additions & 0 deletions src/test/ui/coherence/coherence-inherited-subtyping.re.stderr
@@ -0,0 +1,14 @@
error[E0592]: duplicate definitions with name `method1`
--> $DIR/coherence-inherited-subtyping.rs:14:5
|
LL | fn method1(&self) {}
| ^^^^^^^^^^^^^^^^^^^^ duplicate definitions for `method1`
...
LL | fn method1(&self) {}
| -------------------- other definition for `method1`
|
= note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details

error: aborting due to previous error

For more information about this error, try `rustc --explain E0592`.
21 changes: 21 additions & 0 deletions src/test/ui/coherence/coherence-inherited-subtyping.rs
@@ -0,0 +1,21 @@
// Test that two distinct impls which match subtypes of one another
// yield coherence errors (or not) depending on the variance.
//
// Note: This scenario is currently accepted, but as part of the
// universe transition (#56105) may eventually become an error.

// revisions: old re

struct Foo<T> {
t: T,
}

impl Foo<for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8> {
fn method1(&self) {} //~ ERROR duplicate definitions with name `method1`
}

impl Foo<for<'a> fn(&'a u8, &'a u8) -> &'a u8> {
fn method1(&self) {}
}

fn main() {}
16 changes: 16 additions & 0 deletions src/test/ui/coherence/coherence-subtyping.old.stderr
@@ -0,0 +1,16 @@
error: conflicting implementations of trait `TheTrait` for type `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`:
--> $DIR/coherence-subtyping.rs:15:1
|
LL | impl TheTrait for for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8 {}
| ---------------------------------------------------------- first implementation here
LL |
LL | impl TheTrait for for<'a> fn(&'a u8, &'a u8) -> &'a u8 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`
|
= note: `#[deny(coherence_leak_check)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #56105 <https://github.com/rust-lang/rust/issues/56105>
= note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details

error: aborting due to previous error

16 changes: 16 additions & 0 deletions src/test/ui/coherence/coherence-subtyping.re.stderr
@@ -0,0 +1,16 @@
error: conflicting implementations of trait `TheTrait` for type `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`:
--> $DIR/coherence-subtyping.rs:15:1
|
LL | impl TheTrait for for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8 {}
| ---------------------------------------------------------- first implementation here
LL |
LL | impl TheTrait for for<'a> fn(&'a u8, &'a u8) -> &'a u8 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`
|
= note: `#[deny(coherence_leak_check)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #56105 <https://github.com/rust-lang/rust/issues/56105>
= note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details

error: aborting due to previous error

12 changes: 7 additions & 5 deletions src/test/ui/coherence/coherence-subtyping.rs
Expand Up @@ -5,16 +5,18 @@
// universe transition (#56105) may eventually become an error.

// revisions: old re
// build-pass (FIXME(62277): could be check-pass?)

trait TheTrait {
fn foo(&self) { }
fn foo(&self) {}
}

impl TheTrait for for<'a,'b> fn(&'a u8, &'b u8) -> &'a u8 {
}
impl TheTrait for for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8 {}

impl TheTrait for for<'a> fn(&'a u8, &'a u8) -> &'a u8 {
//[re]~^ ERROR conflicting implementation
//[re]~^^ WARNING this was previously accepted by the compiler but is being phased out
//[old]~^^^ ERROR conflicting implementation
//[old]~^^^^ WARNING this was previously accepted by the compiler but is being phased out
}

fn main() { }
fn main() {}

0 comments on commit 363faba

Please sign in to comment.