about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2023-11-20 20:56:42 +0100
committerGitHub <noreply@github.com>2023-11-20 20:56:42 +0100
commit0270afee3148e8a97f9969f037ccbec046d65fd6 (patch)
tree49874d72a13324bdaf59b82fcb40e53755785210
parent7fd7dad07b77ea436d032774274ef43557702a30 (diff)
parent253f5023c3e647631f52a945eb523edd6bc71af8 (diff)
downloadrust-0270afee3148e8a97f9969f037ccbec046d65fd6.tar.gz
rust-0270afee3148e8a97f9969f037ccbec046d65fd6.zip
Rollup merge of #117992 - compiler-errors:sound-but-not-complete, r=lcnr,aliemjay
Don't require intercrate mode for negative coherence

Negative coherence needs to be *sound*, but does not need to be *complete*, since it's looking for the *existence* of a negative goal, not the non-existence of a positive goal.

This removes some trivial and annoying ambiguities when a negative impl has region constraints.

r? lcnr idk if this needs an fcp but if it does, pls kick it off
-rw-r--r--compiler/rustc_infer/src/infer/at.rs11
-rw-r--r--compiler/rustc_trait_selection/src/traits/coherence.rs22
-rw-r--r--tests/ui/coherence/coherence-negative-outlives-lifetimes.rs4
-rw-r--r--tests/ui/coherence/coherence-negative-outlives-lifetimes.stock.stderr2
-rw-r--r--tests/ui/coherence/coherence-negative-outlives-lifetimes.with_negative_coherence.stderr2
-rw-r--r--tests/ui/coherence/coherence-overlap-with-regions.rs8
-rw-r--r--tests/ui/coherence/coherence-overlap-with-regions.stderr11
-rw-r--r--tests/ui/coherence/negative-coherence-considering-regions.any_lt.stderr2
-rw-r--r--tests/ui/coherence/negative-coherence-considering-regions.rs8
-rw-r--r--tests/ui/coherence/negative-coherence-considering-regions.static_lt.stderr12
10 files changed, 32 insertions, 50 deletions
diff --git a/compiler/rustc_infer/src/infer/at.rs b/compiler/rustc_infer/src/infer/at.rs
index 2797d079761..c32c3aa6d29 100644
--- a/compiler/rustc_infer/src/infer/at.rs
+++ b/compiler/rustc_infer/src/infer/at.rs
@@ -65,8 +65,15 @@ impl<'tcx> InferCtxt<'tcx> {
 
     /// Forks the inference context, creating a new inference context with the same inference
     /// variables in the same state. This can be used to "branch off" many tests from the same
-    /// common state. Used in coherence.
+    /// common state.
     pub fn fork(&self) -> Self {
+        self.fork_with_intercrate(self.intercrate)
+    }
+
+    /// Forks the inference context, creating a new inference context with the same inference
+    /// variables in the same state, except possibly changing the intercrate mode. This can be
+    /// used to "branch off" many tests from the same common state. Used in negative coherence.
+    pub fn fork_with_intercrate(&self, intercrate: bool) -> Self {
         Self {
             tcx: self.tcx,
             defining_use_anchor: self.defining_use_anchor,
@@ -81,7 +88,7 @@ impl<'tcx> InferCtxt<'tcx> {
             tainted_by_errors: self.tainted_by_errors.clone(),
             err_count_on_creation: self.err_count_on_creation,
             universe: self.universe.clone(),
-            intercrate: self.intercrate,
+            intercrate,
             next_trait_solver: self.next_trait_solver,
         }
     }
diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs
index 362a734818c..c4c0428dcc1 100644
--- a/compiler/rustc_trait_selection/src/traits/coherence.rs
+++ b/compiler/rustc_trait_selection/src/traits/coherence.rs
@@ -397,6 +397,8 @@ fn impl_intersection_has_negative_obligation(
 ) -> bool {
     debug!("negative_impl(impl1_def_id={:?}, impl2_def_id={:?})", impl1_def_id, impl2_def_id);
 
+    // N.B. We need to unify impl headers *with* intercrate mode, even if proving negative predicates
+    // do not need intercrate mode enabled.
     let ref infcx = tcx.infer_ctxt().intercrate(true).with_next_trait_solver(true).build();
     let root_universe = infcx.universe();
     assert_eq!(root_universe, ty::UniverseIndex::ROOT);
@@ -415,13 +417,6 @@ fn impl_intersection_has_negative_obligation(
         return false;
     };
 
-    plug_infer_with_placeholders(
-        infcx,
-        root_universe,
-        (impl1_header.impl_args, impl2_header.impl_args),
-    );
-    let param_env = infcx.resolve_vars_if_possible(param_env);
-
     // 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.
@@ -429,6 +424,13 @@ fn impl_intersection_has_negative_obligation(
     drop(infcx.take_registered_region_obligations());
     drop(infcx.take_and_reset_region_constraints());
 
+    plug_infer_with_placeholders(
+        infcx,
+        root_universe,
+        (impl1_header.impl_args, impl2_header.impl_args),
+    );
+    let param_env = infcx.resolve_vars_if_possible(param_env);
+
     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))
 }
@@ -554,7 +556,11 @@ fn try_prove_negated_where_clause<'tcx>(
         return false;
     };
 
-    let ref infcx = root_infcx.fork();
+    // N.B. We don't need to use intercrate mode here because we're trying to prove
+    // the *existence* of a negative goal, not the non-existence of a positive goal.
+    // Without this, we over-eagerly register coherence ambiguity candidates when
+    // impl candidates do exist.
+    let ref infcx = root_infcx.fork_with_intercrate(false);
     let ocx = ObligationCtxt::new(infcx);
 
     ocx.register_obligation(Obligation::new(
diff --git a/tests/ui/coherence/coherence-negative-outlives-lifetimes.rs b/tests/ui/coherence/coherence-negative-outlives-lifetimes.rs
index 0e16d12a181..531977d6dac 100644
--- a/tests/ui/coherence/coherence-negative-outlives-lifetimes.rs
+++ b/tests/ui/coherence/coherence-negative-outlives-lifetimes.rs
@@ -1,5 +1,9 @@
 // revisions: stock with_negative_coherence
+
 //[with_negative_coherence] known-bug: unknown
+// Ideally this would work, but we don't use `&'a T` to imply that `T: 'a`
+// which is required for `&'a T: !MyPredicate` to hold. This is similar to the
+// test `negative-coherence-placeholder-region-constraints-on-unification.explicit.stderr`
 
 #![feature(negative_impls)]
 #![cfg_attr(with_negative_coherence, feature(with_negative_coherence))]
diff --git a/tests/ui/coherence/coherence-negative-outlives-lifetimes.stock.stderr b/tests/ui/coherence/coherence-negative-outlives-lifetimes.stock.stderr
index 097cc4e0fe3..6d6e163b206 100644
--- a/tests/ui/coherence/coherence-negative-outlives-lifetimes.stock.stderr
+++ b/tests/ui/coherence/coherence-negative-outlives-lifetimes.stock.stderr
@@ -1,5 +1,5 @@
 error[E0119]: conflicting implementations of trait `MyTrait<'_>` for type `&_`
-  --> $DIR/coherence-negative-outlives-lifetimes.rs:14:1
+  --> $DIR/coherence-negative-outlives-lifetimes.rs:18:1
    |
 LL | impl<'a, T: MyPredicate<'a>> MyTrait<'a> for T {}
    | ---------------------------------------------- first implementation here
diff --git a/tests/ui/coherence/coherence-negative-outlives-lifetimes.with_negative_coherence.stderr b/tests/ui/coherence/coherence-negative-outlives-lifetimes.with_negative_coherence.stderr
index 097cc4e0fe3..6d6e163b206 100644
--- a/tests/ui/coherence/coherence-negative-outlives-lifetimes.with_negative_coherence.stderr
+++ b/tests/ui/coherence/coherence-negative-outlives-lifetimes.with_negative_coherence.stderr
@@ -1,5 +1,5 @@
 error[E0119]: conflicting implementations of trait `MyTrait<'_>` for type `&_`
-  --> $DIR/coherence-negative-outlives-lifetimes.rs:14:1
+  --> $DIR/coherence-negative-outlives-lifetimes.rs:18:1
    |
 LL | impl<'a, T: MyPredicate<'a>> MyTrait<'a> for T {}
    | ---------------------------------------------- first implementation here
diff --git a/tests/ui/coherence/coherence-overlap-with-regions.rs b/tests/ui/coherence/coherence-overlap-with-regions.rs
index 9945c8e6cfd..32f01f41801 100644
--- a/tests/ui/coherence/coherence-overlap-with-regions.rs
+++ b/tests/ui/coherence/coherence-overlap-with-regions.rs
@@ -1,10 +1,4 @@
-// known-bug: unknown
-
-// This fails because we currently perform negative coherence in coherence mode.
-// This means that when looking for a negative predicate, we also assemble a
-// coherence-unknowable predicate. Since confirming the negative impl has region
-// obligations, we don't prefer the impl over the unknowable predicate
-// unconditionally and instead flounder.
+// check-pass
 
 #![feature(negative_impls)]
 #![feature(rustc_attrs)]
diff --git a/tests/ui/coherence/coherence-overlap-with-regions.stderr b/tests/ui/coherence/coherence-overlap-with-regions.stderr
deleted file mode 100644
index fd25f0978ba..00000000000
--- a/tests/ui/coherence/coherence-overlap-with-regions.stderr
+++ /dev/null
@@ -1,11 +0,0 @@
-error[E0119]: conflicting implementations of trait `Bar` for type `&_`
-  --> $DIR/coherence-overlap-with-regions.rs:20:1
-   |
-LL | impl<T: Foo> Bar for T {}
-   | ---------------------- first implementation here
-LL | impl<T> Bar for &T where T: 'static {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `&_`
-
-error: aborting due to previous error
-
-For more information about this error, try `rustc --explain E0119`.
diff --git a/tests/ui/coherence/negative-coherence-considering-regions.any_lt.stderr b/tests/ui/coherence/negative-coherence-considering-regions.any_lt.stderr
index 4cf50b4f208..442934a8949 100644
--- a/tests/ui/coherence/negative-coherence-considering-regions.any_lt.stderr
+++ b/tests/ui/coherence/negative-coherence-considering-regions.any_lt.stderr
@@ -1,5 +1,5 @@
 error[E0119]: conflicting implementations of trait `Bar` for type `&_`
-  --> $DIR/negative-coherence-considering-regions.rs:22:1
+  --> $DIR/negative-coherence-considering-regions.rs:16:1
    |
 LL | impl<T> Bar for T where T: Foo {}
    | ------------------------------ first implementation here
diff --git a/tests/ui/coherence/negative-coherence-considering-regions.rs b/tests/ui/coherence/negative-coherence-considering-regions.rs
index 597a5972628..a43ad19eca7 100644
--- a/tests/ui/coherence/negative-coherence-considering-regions.rs
+++ b/tests/ui/coherence/negative-coherence-considering-regions.rs
@@ -1,11 +1,5 @@
 // revisions: any_lt static_lt
-//[static_lt] known-bug: unknown
-
-// This fails because we currently perform negative coherence in coherence mode.
-// This means that when looking for a negative predicate, we also assemble a
-// coherence-unknowable predicate. Since confirming the negative impl has region
-// obligations, we don't prefer the impl over the unknowable predicate
-// unconditionally and instead flounder.
+//[static_lt] check-pass
 
 #![feature(negative_impls)]
 #![feature(with_negative_coherence)]
diff --git a/tests/ui/coherence/negative-coherence-considering-regions.static_lt.stderr b/tests/ui/coherence/negative-coherence-considering-regions.static_lt.stderr
deleted file mode 100644
index 87e7be2aa44..00000000000
--- a/tests/ui/coherence/negative-coherence-considering-regions.static_lt.stderr
+++ /dev/null
@@ -1,12 +0,0 @@
-error[E0119]: conflicting implementations of trait `Bar` for type `&'static _`
-  --> $DIR/negative-coherence-considering-regions.rs:26:1
-   |
-LL | impl<T> Bar for T where T: Foo {}
-   | ------------------------------ first implementation here
-...
-LL | impl<T> Bar for &'static T {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `&'static _`
-
-error: aborting due to previous error
-
-For more information about this error, try `rustc --explain E0119`.