From 126ff3477d47c950fdd7f67440664460ca7ba90c Mon Sep 17 00:00:00 2001
From: Michael Goulet <michael@errs.io>
Date: Wed, 13 Dec 2023 14:39:35 +0000
Subject: [PATCH] Add some comments, add can_define_opaque_ty check to
 try_normalize_ty_recur

---
 .../src/solve/alias_relate.rs                 | 22 ++++++++-----
 .../rustc_trait_selection/src/solve/mod.rs    | 33 +++++++++++--------
 2 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/compiler/rustc_trait_selection/src/solve/alias_relate.rs b/compiler/rustc_trait_selection/src/solve/alias_relate.rs
index 2e99854ddc676..bb183b3bc8d53 100644
--- a/compiler/rustc_trait_selection/src/solve/alias_relate.rs
+++ b/compiler/rustc_trait_selection/src/solve/alias_relate.rs
@@ -2,15 +2,10 @@
 //! Doing this via a separate goal is called "deferred alias relation" and part
 //! of our more general approach to "lazy normalization".
 //!
-//! This goal, e.g. `A alias-relate B`, may be satisfied by one of three branches:
-//! * normalizes-to: If `A` is a projection, we can prove the equivalent
-//!   projection predicate with B as the right-hand side of the projection.
-//!   This goal is computed in both directions, if both are aliases.
-//! * subst-relate: Equate `A` and `B` by their substs, if they're both
-//!   aliases with the same def-id.
-//! * bidirectional-normalizes-to: If `A` and `B` are both projections, and both
-//!   may apply, then we can compute the "intersection" of both normalizes-to by
-//!   performing them together. This is used specifically to resolve ambiguities.
+//! This is done by first normalizing both sides of the goal, ending up in
+//! either a concrete type, rigid projection, opaque, or an infer variable.
+//! These are related further, described best with inline comments below.
+
 use super::EvalCtxt;
 use rustc_infer::infer::DefineOpaqueTypes;
 use rustc_infer::traits::query::NoSolution;
@@ -45,11 +40,18 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
                 self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
             }
 
+            // 1. When we have an alias being related to an infer var, then assign
+            // the type (or const) of the alias to the infer var.
+            // 2. When we have an opaque being related to a rigid type (which, due to 1,
+            // is not an infer var), then assign the hidden type of the opaque to be
+            // the rigid type.
+            // 3. Otherwise, a rigid projection does not equal a concrete type ever.
             (Some(alias), None) => {
                 if rhs.is_infer() {
                     self.relate(param_env, lhs, variance, rhs)?;
                     self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
                 } else if alias.is_opaque(tcx) {
+                    // FIXME: This doesn't account for variance.
                     self.define_opaque(param_env, alias, rhs)
                 } else {
                     Err(NoSolution)
@@ -60,6 +62,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
                     self.relate(param_env, lhs, variance, rhs)?;
                     self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
                 } else if alias.is_opaque(tcx) {
+                    // FIXME: This doesn't account for variance.
                     self.define_opaque(param_env, alias, lhs)
                 } else {
                     Err(NoSolution)
@@ -72,6 +75,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         }
     }
 
+    // FIXME: This needs a name that reflects that it's okay to bottom-out with an inference var.
     /// Normalize the `term` to equate it later. This does not define opaque types.
     #[instrument(level = "debug", skip(self, param_env), ret)]
     fn try_normalize_term(
diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs
index 1e58106e3533c..09b17db290281 100644
--- a/compiler/rustc_trait_selection/src/solve/mod.rs
+++ b/compiler/rustc_trait_selection/src/solve/mod.rs
@@ -22,6 +22,7 @@ use rustc_middle::traits::solve::{
     CanonicalResponse, Certainty, ExternalConstraintsData, Goal, IsNormalizesToHack, QueryResult,
     Response,
 };
+use rustc_middle::traits::Reveal;
 use rustc_middle::ty::{self, OpaqueTypeKey, Ty, TyCtxt, UniverseIndex};
 use rustc_middle::ty::{
     CoercePredicate, RegionOutlivesPredicate, SubtypePredicate, TypeOutlivesPredicate,
@@ -312,19 +313,25 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
             return Some(ty);
         };
 
-        // We do no always define opaque types eagerly to allow non-defining uses in the defining scope.
-        if let (DefineOpaqueTypes::No, ty::AliasKind::Opaque) = (define_opaque_types, kind) {
-            if let Some(def_id) = alias.def_id.as_local() {
-                if self
-                    .unify_existing_opaque_tys(
-                        param_env,
-                        OpaqueTypeKey { def_id, args: alias.args },
-                        self.next_ty_infer(),
-                    )
-                    .is_empty()
-                {
-                    return Some(ty);
-                }
+        // We do no always define opaque types eagerly to allow non-defining uses
+        // in the defining scope. However, if we can unify this opaque to an existing
+        // opaque, then we should attempt to eagerly reveal the opaque, and we fall
+        // through.
+        if let DefineOpaqueTypes::No = define_opaque_types
+            && let Reveal::UserFacing = param_env.reveal()
+            && let ty::Opaque = kind
+            && let Some(def_id) = alias.def_id.as_local()
+            && self.can_define_opaque_ty(def_id)
+        {
+            if self
+                .unify_existing_opaque_tys(
+                    param_env,
+                    OpaqueTypeKey { def_id, args: alias.args },
+                    self.next_ty_infer(),
+                )
+                .is_empty()
+            {
+                return Some(ty);
             }
         }