Auto merge of #98614 - oli-obk:take_unsound_opaque_types, r=wesleywiser

don't succeed `evaluate_obligation` query if new opaque types were registered

fixes #98608
fixes #98604

The root cause of all this is that in type flag computation we entirely ignore nongeneric things like struct fields and the signature of function items. So if a flag had to be set for a struct if it is set for a field, that will only happen if the field is generic, as only the generic parameters are checked.

I now believe we cannot use type flags to handle opaque types. They seem like the wrong tool for this.

Instead, this PR replaces the previous logic by adding a new variant of `EvaluatedToOk`: `EvaluatedToOkModuloOpaqueTypes`, which says that there were some opaque types that got hidden types bound, but that binding may not have been legal (because we don't know if the opaque type was in its defining scope or not).

Author: bors

compiler/rustc_infer/src/infer/mod.rs

@@ -888,6 +888,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             .region_constraints_added_in_snapshot(&snapshot.undo_snapshot)
     }
 
+    pub fn opaque_types_added_in_snapshot(&self, snapshot: &CombinedSnapshot<'a, 'tcx>) -> bool {
+        self.inner.borrow().undo_log.opaque_types_in_snapshot(&snapshot.undo_snapshot)
+    }
+
     pub fn add_given(&self, sub: ty::Region<'tcx>, sup: ty::RegionVid) {
         self.inner.borrow_mut().unwrap_region_constraints().add_given(sub, sup);
     }

compiler/rustc_infer/src/infer/opaque_types.rs

@@ -95,7 +95,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         }
         let (a, b) = if a_is_expected { (a, b) } else { (b, a) };
         let process = |a: Ty<'tcx>, b: Ty<'tcx>| match *a.kind() {
-            ty::Opaque(def_id, substs) => {
+            ty::Opaque(def_id, substs) if def_id.is_local() => {
                 let origin = if self.defining_use_anchor.is_some() {
                     // Check that this is `impl Trait` type is
                     // declared by `parent_def_id` -- i.e., one whose

compiler/rustc_infer/src/infer/undo_log.rs

@@ -185,6 +185,10 @@ impl<'tcx> InferCtxtUndoLogs<'tcx> {
         })
     }
 
+    pub(crate) fn opaque_types_in_snapshot(&self, s: &Snapshot<'tcx>) -> bool {
+        self.logs[s.undo_len..].iter().any(|log| matches!(log, UndoLog::OpaqueTypes(..)))
+    }
+
     pub(crate) fn region_constraints(
         &self,
     ) -> impl Iterator<Item = &'_ region_constraints::UndoLog<'tcx>> + Clone {

compiler/rustc_infer/src/traits/project.rs

@@ -203,7 +203,7 @@ impl<'tcx> ProjectionCache<'_, 'tcx> {
             Some(&ProjectionCacheEntry::NormalizedTy { ref ty, complete: _ }) => {
                 info!("ProjectionCacheEntry::complete({:?}) - completing {:?}", key, ty);
                 let mut ty = ty.clone();
-                if result == EvaluationResult::EvaluatedToOk {
+                if result.must_apply_considering_regions() {
                     ty.obligations = vec![];
                 }
                 map.insert(key, ProjectionCacheEntry::NormalizedTy { ty, complete: Some(result) });

compiler/rustc_middle/src/traits/select.rs

@@ -176,6 +176,10 @@ pub enum EvaluationResult {
     EvaluatedToOk,
     /// Evaluation successful, but there were unevaluated region obligations.
     EvaluatedToOkModuloRegions,
+    /// Evaluation successful, but need to rerun because opaque types got
+    /// hidden types assigned without it being known whether the opaque types
+    /// are within their defining scope
+    EvaluatedToOkModuloOpaqueTypes,
     /// Evaluation is known to be ambiguous -- it *might* hold for some
     /// assignment of inference variables, but it might not.
     ///
@@ -252,9 +256,11 @@ impl EvaluationResult {
 
     pub fn may_apply(self) -> bool {
         match self {
-            EvaluatedToOk | EvaluatedToOkModuloRegions | EvaluatedToAmbig | EvaluatedToUnknown => {
-                true
-            }
+            EvaluatedToOkModuloOpaqueTypes
+            | EvaluatedToOk
+            | EvaluatedToOkModuloRegions
+            | EvaluatedToAmbig
+            | EvaluatedToUnknown => true,
 
             EvaluatedToErr | EvaluatedToRecur => false,
         }
@@ -264,7 +270,11 @@ impl EvaluationResult {
         match self {
             EvaluatedToUnknown | EvaluatedToRecur => true,
 
-            EvaluatedToOk | EvaluatedToOkModuloRegions | EvaluatedToAmbig | EvaluatedToErr => false,
+            EvaluatedToOkModuloOpaqueTypes
+            | EvaluatedToOk
+            | EvaluatedToOkModuloRegions
+            | EvaluatedToAmbig
+            | EvaluatedToErr => false,
         }
     }
 }

compiler/rustc_middle/src/ty/mod.rs

@@ -1089,6 +1089,7 @@ impl<'tcx> InstantiatedPredicates<'tcx> {
 #[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable, Lift)]
 #[derive(TypeFoldable, TypeVisitable)]
 pub struct OpaqueTypeKey<'tcx> {
+    // FIXME(oli-obk): make this a LocalDefId
     pub def_id: DefId,
     pub substs: SubstsRef<'tcx>,
 }

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -777,6 +777,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             Ok(
                 EvaluationResult::EvaluatedToOk
                 | EvaluationResult::EvaluatedToOkModuloRegions
+                | EvaluationResult::EvaluatedToOkModuloOpaqueTypes
                 | EvaluationResult::EvaluatedToAmbig,
             ) => {}
             _ => return false,

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -394,6 +394,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 Err(_) => return Ok(EvaluatedToErr),
             }
 
+            if self.infcx.opaque_types_added_in_snapshot(snapshot) {
+                return Ok(result.max(EvaluatedToOkModuloOpaqueTypes));
+            }
+
             match self.infcx.region_constraints_added_in_snapshot(snapshot) {
                 None => Ok(result),
                 Some(_) => Ok(result.max(EvaluatedToOkModuloRegions)),

compiler/rustc_type_ir/src/lib.rs

@@ -204,14 +204,6 @@ bitflags! {
                                           | TypeFlags::HAS_CT_INFER.bits
                                           | TypeFlags::HAS_TY_PLACEHOLDER.bits
                                           | TypeFlags::HAS_CT_PLACEHOLDER.bits
-                                          // The `evaluate_obligation` query does not return further
-                                          // obligations. If it evaluates an obligation with an opaque
-                                          // type, that opaque type may get compared to another type,
-                                          // constraining it. We would lose this information.
-                                          // FIXME: differentiate between crate-local opaque types
-                                          // and opaque types from other crates, as only opaque types
-                                          // from the local crate can possibly be a local name
-                                          | TypeFlags::HAS_TY_OPAQUE.bits
                                           // We consider 'freshened' types and constants
                                           // to depend on a particular fn.
                                           // The freshening process throws away information,

src/test/ui/impl-trait/auto-trait-leak.rs

@@ -11,7 +11,6 @@ fn main() {
 // return type, which can't depend on the obligation.
 fn cycle1() -> impl Clone {
     //~^ ERROR cycle detected
-    //~| ERROR cycle detected
     send(cycle2().clone());
 
     Rc::new(Cell::new(5))