Skip to content

Commit

Permalink
Rollup merge of rust-lang#117994 - compiler-errors:throw-away-regions…
Browse files Browse the repository at this point in the history
…-in-coherence, r=lcnr

Ignore but do not assume region obligations from unifying headers in negative coherence

Partly addresses a FIXME that was added in rust-lang#112875. Just as we can throw away the nested trait/projection obligations from unifying two impl headers, we can also just throw away the region obligations too.

I removed part of the FIXME that was incorrect, namely:
> Given that the only region constraints we get are involving inference regions in the root, it shouldn't matter, but still sus.

This is not true when unifying `fn(A)` and `for<'b> fn(&'b B)` which ends up with placeholder region outlives from non-root universes. I'm pretty sure this is okay, though it would be nice if we were to use them as assumptions. See the `explicit` revision of the test I committed, which still fails.

Fixes rust-lang#117986

r? lcnr, feel free to reassign tho.
  • Loading branch information
matthiaskrgr authored Nov 17, 2023
2 parents 09b964a + d688b86 commit 7eb676f
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 6 deletions.
12 changes: 6 additions & 6 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,14 +546,14 @@ fn try_prove_negated_where_clause<'tcx>(
};

// FIXME(with_negative_coherence): the infcx has region contraints from equating
// the impl headers as requirements. Given that the only region constraints we
// get are involving inference regions in the root, it shouldn't matter, but
// still sus.
// the impl headers as requirements.
//
// We probably should just throw away the region obligations registered up until
// now, or ideally use them as assumptions when proving the region obligations
// that we get from proving the negative predicate below.
// We ideally should use these region constraints as assumptions when proving
// the region obligations that we get from proving the negative predicate below.
let ref infcx = root_infcx.fork();
let _ = infcx.take_registered_region_obligations();
let _ = infcx.take_and_reset_region_constraints();

let ocx = ObligationCtxt::new(infcx);

ocx.register_obligation(Obligation::new(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error: conflicting implementations of trait `FnMarker` for type `fn(&_)`
--> $DIR/negative-coherence-placeholder-region-constraints-on-unification.rs:21:1
|
LL | impl<T: ?Sized + Marker> FnMarker for fn(T) {}
| ------------------------------------------- first implementation here
LL | impl<T: ?Sized> FnMarker for fn(&T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `fn(&_)`
|
= 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
note: the lint level is defined here
--> $DIR/negative-coherence-placeholder-region-constraints-on-unification.rs:4:11
|
LL | #![forbid(coherence_leak_check)]
| ^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// revisions: explicit implicit
//[implicit] check-pass

#![forbid(coherence_leak_check)]
#![feature(negative_impls, with_negative_coherence)]

pub trait Marker {}

#[cfg(implicit)]
impl<T: ?Sized> !Marker for &T {}

#[cfg(explicit)]
impl<'a, T: ?Sized + 'a> !Marker for &'a T {}

trait FnMarker {}

// Unifying these two impls below results in a `T: '!0` obligation
// that we shouldn't need to care about. Ideally, we'd treat that
// as an assumption when proving `&'!0 T: Marker`...
impl<T: ?Sized + Marker> FnMarker for fn(T) {}
impl<T: ?Sized> FnMarker for fn(&T) {}
//[explicit]~^ ERROR conflicting implementations of trait `FnMarker` for type `fn(&_)`
//[explicit]~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!

fn main() {}

0 comments on commit 7eb676f

Please sign in to comment.