Rollup merge of rust-lang#117994 - compiler-errors:throw-away-regions-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.

Author: compiler-errors

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -408,14 +408,21 @@ fn impl_intersection_has_negative_obligation(
 
     // Equate the headers to find their intersection (the general type, with infer vars,
     // that may apply both impls).
-    let Some(_equate_obligations) =
+    let Some(equate_obligations) =
         equate_impl_headers(infcx, param_env, &impl1_header, &impl2_header)
     else {
         return false;
     };
 
     plug_infer_with_placeholders(infcx, universe, (impl1_header.impl_args, impl2_header.impl_args));
 
+    // FIXME(with_negative_coherence): the infcx has constraints from equating
+    // the impl headers. We should use these constraints as assumptions, not as
+    // requirements, when proving the negated where clauses below.
+    drop(equate_obligations);
+    drop(infcx.take_registered_region_obligations());
+    drop(infcx.take_and_reset_region_constraints());
+
     util::elaborate(tcx, tcx.predicates_of(impl2_def_id).instantiate(tcx, impl2_header.impl_args))
         .any(|(clause, _)| try_prove_negated_where_clause(infcx, clause, param_env))
 }
@@ -541,14 +548,6 @@ fn try_prove_negated_where_clause<'tcx>(
         return false;
     };
 
-    // 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.
-    //
-    // 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.
     let ref infcx = root_infcx.fork();
     let ocx = ObligationCtxt::new(infcx);
 

tests/ui/coherence/negative-coherence-placeholder-region-constraints-on-unification.explicit.stderr

@@ -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
+

tests/ui/coherence/negative-coherence-placeholder-region-constraints-on-unification.rs

@@ -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() {}