about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMichael Goulet <michael@errs.io>2023-07-06 20:11:39 -0700
committerGitHub <noreply@github.com>2023-07-06 20:11:39 -0700
commitde49a9f2f51757dced984a1b7f45e38d59985b5e (patch)
treeef8308cbeb4b5a9a6fef87d0b317ed8789ef5a34
parent75febc6ed65c50f949264d7fa3be7be89fee0407 (diff)
parent388c230cf77c633eeb9eee2e5388ce957c69797b (diff)
Rollup merge of #112825 - compiler-errors:tait-defining-cycle, r=lcnr
Don't call `type_of` on TAIT in defining scope in new solver

It's *never* productive to call `consider_auto_trait_candidate` on a TAIT in the defining scope, since it will always lead to a query cycle since we call `type_of` on the TAIT. So let's just don't.

I've reserved this behavior just to `SolverMode::Normal` just to avoid any future problems, since this is *technically* incomplete since we're discarding a candidate that could *theoretically* apply. But given such candidate assembly *always* leads to a query cycle, I think it's relatively low risk, and I could be convinced otherwise and make this apply to both solver mode. I assume it's far less likely to be encountered in coherence, though.

This is much more likely to encounter in the new solver, though it can also be encountered in the old solver too, so I'm happy to discuss whether this new behavior we even want in the first place...

I encountered this in a couple of failing UI tests:
* `tests/ui/type-alias-impl-trait/issue-62000-associate-impl-trait-lifetimes.rs`
* `tests/ui/type-alias-impl-trait/issue-93411.rs`

r? `@lcnr`
-rw-r--r--compiler/rustc_trait_selection/src/solve/eval_ctxt.rs2
-rw-r--r--compiler/rustc_trait_selection/src/solve/trait_goals.rs27
-rw-r--r--tests/ui/traits/new-solver/dont-remap-tait-substs.rs2
-rw-r--r--tests/ui/traits/new-solver/dont-remap-tait-substs.stderr99
-rw-r--r--tests/ui/traits/new-solver/dont-type_of-tait-in-defining-scope.not_send.stderr16
-rw-r--r--tests/ui/traits/new-solver/dont-type_of-tait-in-defining-scope.rs22
6 files changed, 67 insertions, 101 deletions
diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
index 6fb788e296f..74dfbdddbab 100644
--- a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
+++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
@@ -837,7 +837,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         }
     }
 
-    pub(super) fn can_define_opaque_ty(&mut self, def_id: LocalDefId) -> bool {
+    pub(super) fn can_define_opaque_ty(&self, def_id: LocalDefId) -> bool {
         self.infcx.opaque_type_origin(def_id).is_some()
     }
 
diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs
index 7ff47295e7c..cd68626bed1 100644
--- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs
+++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs
@@ -7,6 +7,7 @@ use rustc_hir::{LangItem, Movability};
 use rustc_infer::traits::query::NoSolution;
 use rustc_infer::traits::util::supertraits;
 use rustc_middle::traits::solve::{CanonicalResponse, Certainty, Goal, QueryResult};
+use rustc_middle::traits::Reveal;
 use rustc_middle::ty::fast_reject::{DeepRejectCtxt, TreatParams, TreatProjections};
 use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt};
 use rustc_middle::ty::{TraitPredicate, TypeVisitableExt};
@@ -118,6 +119,32 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
             return result;
         }
 
+        // Don't call `type_of` on a local TAIT that's in the defining scope,
+        // since that may require calling `typeck` on the same item we're
+        // currently type checking, which will result in a fatal cycle that
+        // ideally we want to avoid, since we can make progress on this goal
+        // via an alias bound or a locally-inferred hidden type instead.
+        //
+        // Also, don't call `type_of` on a TAIT in `Reveal::All` mode, since
+        // we already normalize the self type in
+        // `assemble_candidates_after_normalizing_self_ty`, and we'd
+        // just be registering an identical candidate here.
+        //
+        // Returning `Err(NoSolution)` here is ok in `SolverMode::Coherence`
+        // since we'll always be registering an ambiguous candidate in
+        // `assemble_candidates_after_normalizing_self_ty` due to normalizing
+        // the TAIT.
+        if let ty::Alias(ty::Opaque, opaque_ty) = goal.predicate.self_ty().kind() {
+            if matches!(goal.param_env.reveal(), Reveal::All)
+                || opaque_ty
+                    .def_id
+                    .as_local()
+                    .is_some_and(|def_id| ecx.can_define_opaque_ty(def_id))
+            {
+                return Err(NoSolution);
+            }
+        }
+
         ecx.probe_and_evaluate_goal_for_constituent_tys(
             goal,
             structural_traits::instantiate_constituent_tys_for_auto_trait,
diff --git a/tests/ui/traits/new-solver/dont-remap-tait-substs.rs b/tests/ui/traits/new-solver/dont-remap-tait-substs.rs
index 08c08e83076..028222f4e6d 100644
--- a/tests/ui/traits/new-solver/dont-remap-tait-substs.rs
+++ b/tests/ui/traits/new-solver/dont-remap-tait-substs.rs
@@ -1,5 +1,5 @@
 // compile-flags: -Ztrait-solver=next
-// known-bug: #112825
+// check-pass
 
 // Makes sure we don't prepopulate the MIR typeck of `define`
 // with `Foo<T, U> = T`, but instead, `Foo<B, A> = B`, so that
diff --git a/tests/ui/traits/new-solver/dont-remap-tait-substs.stderr b/tests/ui/traits/new-solver/dont-remap-tait-substs.stderr
deleted file mode 100644
index 769eea7dfd6..00000000000
--- a/tests/ui/traits/new-solver/dont-remap-tait-substs.stderr
+++ /dev/null
@@ -1,99 +0,0 @@
-error[E0391]: cycle detected when computing type of `Foo::{opaque#0}`
-  --> $DIR/dont-remap-tait-substs.rs:10:24
-   |
-LL | type Foo<T: Send, U> = impl NeedsSend<T>;
-   |                        ^^^^^^^^^^^^^^^^^
-   |
-note: ...which requires borrow-checking `define`...
-  --> $DIR/dont-remap-tait-substs.rs:15:1
-   |
-LL | fn define<A, B: Send>(a: A, b: B) {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which again requires computing type of `Foo::{opaque#0}`, completing the cycle
-note: cycle used when checking item types in top-level module
-  --> $DIR/dont-remap-tait-substs.rs:8:1
-   |
-LL | / #![feature(type_alias_impl_trait)]
-LL | |
-LL | | type Foo<T: Send, U> = impl NeedsSend<T>;
-LL | |
-...  |
-LL | |
-LL | | fn main() {}
-   | |____________^
-
-error[E0391]: cycle detected when computing type of `Foo::{opaque#0}`
-  --> $DIR/dont-remap-tait-substs.rs:10:24
-   |
-LL | type Foo<T: Send, U> = impl NeedsSend<T>;
-   |                        ^^^^^^^^^^^^^^^^^
-   |
-note: ...which requires borrow-checking `define`...
-  --> $DIR/dont-remap-tait-substs.rs:15:1
-   |
-LL | fn define<A, B: Send>(a: A, b: B) {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which again requires computing type of `Foo::{opaque#0}`, completing the cycle
-note: cycle used when checking item types in top-level module
-  --> $DIR/dont-remap-tait-substs.rs:8:1
-   |
-LL | / #![feature(type_alias_impl_trait)]
-LL | |
-LL | | type Foo<T: Send, U> = impl NeedsSend<T>;
-LL | |
-...  |
-LL | |
-LL | | fn main() {}
-   | |____________^
-
-error[E0391]: cycle detected when computing type of `Foo::{opaque#0}`
-  --> $DIR/dont-remap-tait-substs.rs:10:24
-   |
-LL | type Foo<T: Send, U> = impl NeedsSend<T>;
-   |                        ^^^^^^^^^^^^^^^^^
-   |
-note: ...which requires borrow-checking `define`...
-  --> $DIR/dont-remap-tait-substs.rs:15:1
-   |
-LL | fn define<A, B: Send>(a: A, b: B) {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which again requires computing type of `Foo::{opaque#0}`, completing the cycle
-note: cycle used when checking item types in top-level module
-  --> $DIR/dont-remap-tait-substs.rs:8:1
-   |
-LL | / #![feature(type_alias_impl_trait)]
-LL | |
-LL | | type Foo<T: Send, U> = impl NeedsSend<T>;
-LL | |
-...  |
-LL | |
-LL | | fn main() {}
-   | |____________^
-
-error[E0391]: cycle detected when computing type of `Foo::{opaque#0}`
-  --> $DIR/dont-remap-tait-substs.rs:10:24
-   |
-LL | type Foo<T: Send, U> = impl NeedsSend<T>;
-   |                        ^^^^^^^^^^^^^^^^^
-   |
-note: ...which requires borrow-checking `define`...
-  --> $DIR/dont-remap-tait-substs.rs:15:1
-   |
-LL | fn define<A, B: Send>(a: A, b: B) {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which again requires computing type of `Foo::{opaque#0}`, completing the cycle
-note: cycle used when checking item types in top-level module
-  --> $DIR/dont-remap-tait-substs.rs:8:1
-   |
-LL | / #![feature(type_alias_impl_trait)]
-LL | |
-LL | | type Foo<T: Send, U> = impl NeedsSend<T>;
-LL | |
-...  |
-LL | |
-LL | | fn main() {}
-   | |____________^
-
-error: aborting due to 4 previous errors
-
-For more information about this error, try `rustc --explain E0391`.
diff --git a/tests/ui/traits/new-solver/dont-type_of-tait-in-defining-scope.not_send.stderr b/tests/ui/traits/new-solver/dont-type_of-tait-in-defining-scope.not_send.stderr
new file mode 100644
index 00000000000..ec1c3231abc
--- /dev/null
+++ b/tests/ui/traits/new-solver/dont-type_of-tait-in-defining-scope.not_send.stderr
@@ -0,0 +1,16 @@
+error[E0283]: type annotations needed: cannot satisfy `Foo: Send`
+  --> $DIR/dont-type_of-tait-in-defining-scope.rs:16:5
+   |
+LL |     needs_send::<Foo>();
+   |     ^^^^^^^^^^^^^^^^^
+   |
+   = note: cannot satisfy `Foo: Send`
+note: required by a bound in `needs_send`
+  --> $DIR/dont-type_of-tait-in-defining-scope.rs:13:18
+   |
+LL | fn needs_send<T: Send>() {}
+   |                  ^^^^ required by this bound in `needs_send`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0283`.
diff --git a/tests/ui/traits/new-solver/dont-type_of-tait-in-defining-scope.rs b/tests/ui/traits/new-solver/dont-type_of-tait-in-defining-scope.rs
new file mode 100644
index 00000000000..5a0dcd0e8cc
--- /dev/null
+++ b/tests/ui/traits/new-solver/dont-type_of-tait-in-defining-scope.rs
@@ -0,0 +1,22 @@
+// revisions: is_send not_send
+// compile-flags: -Ztrait-solver=next
+//[is_send] check-pass
+
+#![feature(type_alias_impl_trait)]
+
+#[cfg(is_send)]
+type Foo = impl Send;
+
+#[cfg(not_send)]
+type Foo = impl Sized;
+
+fn needs_send<T: Send>() {}
+
+fn test() {
+    needs_send::<Foo>();
+    //[not_send]~^ ERROR type annotations needed: cannot satisfy `Foo: Send`
+}
+
+fn main() {
+    let _: Foo = ();
+}