detect additional uses of opaques after writeback

Cargo.lock

@@ -4516,7 +4516,6 @@ dependencies = [
  "rustc_session",
  "rustc_span",
  "rustc_transmute",
- "rustc_type_ir",
  "smallvec",
  "thin-vec",
  "tracing",

compiler/rustc_hir_typeck/src/_match.rs

@@ -603,7 +603,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // FIXME(-Znext-solver): Remove this branch once `replace_opaque_types_with_infer` is gone.
             ty::Infer(ty::TyVar(_)) => self
                 .inner
-                .borrow()
+                .borrow_mut()
+                .opaque_types()
                 .iter_opaque_types()
                 .find(|(_, v)| v.ty == expected_ty)
                 .map(|(k, _)| (k.def_id, k.args))?,

compiler/rustc_hir_typeck/src/lib.rs

@@ -32,6 +32,7 @@ mod gather_locals;
 mod intrinsicck;
 mod method;
 mod op;
+mod opaque_types;
 mod pat;
 mod place_op;
 mod rvalue_scopes;
@@ -245,9 +246,7 @@ fn typeck_with_inspect<'tcx>(
 
     let typeck_results = fcx.resolve_type_vars_in_body(body);
 
-    // We clone the defined opaque types during writeback in the new solver
-    // because we have to use them during normalization.
-    let _ = fcx.infcx.take_opaque_types();
+    fcx.detect_opaque_types_added_during_writeback();
 
     // Consistency check our TypeckResults instance can hold all ItemLocalIds
     // it will need to hold.

compiler/rustc_hir_typeck/src/opaque_types.rs

@@ -0,0 +1,26 @@
+use super::FnCtxt;
+impl<'tcx> FnCtxt<'_, 'tcx> {
+    /// We may in theory add further uses of an opaque after cloning the opaque
+    /// types storage during writeback when computing the defining uses.
+    ///
+    /// Silently ignoring them is dangerous and could result in ICE or even in
+    /// unsoundness, so we make sure we catch such cases here. There's currently
+    /// no known code where this actually happens, even with the new solver which
+    /// does normalize types in writeback after cloning the opaque type storage.
+    ///
+    /// FIXME(@lcnr): I believe this should be possible in theory and would like
+    /// an actual test here. After playing around with this for an hour, I wasn't
+    /// able to do anything which didn't already try to normalize the opaque before
+    /// then, either allowing compilation to succeed or causing an ambiguity error.
+    pub(super) fn detect_opaque_types_added_during_writeback(&self) {
+        let num_entries = self.checked_opaque_types_storage_entries.take().unwrap();
+        if let Some((key, hidden_type)) =
+            self.inner.borrow_mut().opaque_types().opaque_types_added_since(num_entries).next()
+        {
+            let opaque_type_string = self.tcx.def_path_str(key.def_id);
+            let msg = format!("unexpected cyclic definition of `{opaque_type_string}`");
+            self.dcx().span_bug(hidden_type.span, msg);
+        }
+        let _ = self.take_opaque_types();
+    }
+}

compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs

@@ -1,10 +1,10 @@
-use std::cell::RefCell;
+use std::cell::{Cell, RefCell};
 use std::ops::Deref;
 
 use rustc_data_structures::unord::{UnordMap, UnordSet};
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::{self as hir, HirId, HirIdMap, LangItem};
-use rustc_infer::infer::{InferCtxt, InferOk, TyCtxtInferExt};
+use rustc_infer::infer::{InferCtxt, InferOk, OpaqueTypeStorageEntries, TyCtxtInferExt};
 use rustc_middle::span_bug;
 use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt, TypingMode};
 use rustc_span::Span;
@@ -37,6 +37,11 @@ pub(crate) struct TypeckRootCtxt<'tcx> {
 
     pub(super) fulfillment_cx: RefCell<Box<dyn TraitEngine<'tcx, FulfillmentError<'tcx>>>>,
 
+    // Used to detect opaque types uses added after we've already checked them.
+    //
+    // See [FnCtxt::detect_opaque_types_added_during_writeback] for more details.
+    pub(super) checked_opaque_types_storage_entries: Cell<Option<OpaqueTypeStorageEntries>>,
+
     /// Some additional `Sized` obligations badly affect type inference.
     /// These obligations are added in a later stage of typeck.
     /// Removing these may also cause additional complications, see #101066.
@@ -85,12 +90,14 @@ impl<'tcx> TypeckRootCtxt<'tcx> {
         let infcx =
             tcx.infer_ctxt().ignoring_regions().build(TypingMode::typeck_for_body(tcx, def_id));
         let typeck_results = RefCell::new(ty::TypeckResults::new(hir_owner));
+        let fulfillment_cx = RefCell::new(<dyn TraitEngine<'_, _>>::new(&infcx));
 
         TypeckRootCtxt {
-            typeck_results,
-            fulfillment_cx: RefCell::new(<dyn TraitEngine<'_, _>>::new(&infcx)),
             infcx,
+            typeck_results,
             locals: RefCell::new(Default::default()),
+            fulfillment_cx,
+            checked_opaque_types_storage_entries: Cell::new(None),
             deferred_sized_obligations: RefCell::new(Vec::new()),
             deferred_call_resolutions: RefCell::new(Default::default()),
             deferred_cast_checks: RefCell::new(Vec::new()),

compiler/rustc_hir_typeck/src/writeback.rs

@@ -535,13 +535,10 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
         let tcx = self.tcx();
         // We clone the opaques instead of stealing them here as they are still used for
         // normalization in the next generation trait solver.
-        //
-        // FIXME(-Znext-solver): Opaque types defined after this would simply get dropped
-        // at the end of typeck. While this seems unlikely to happen in practice this
-        // should still get fixed. Either by preventing writeback from defining new opaque
-        // types or by using this function at the end of writeback and running it as a
-        // fixpoint.
         let opaque_types = self.fcx.infcx.clone_opaque_types();
+        let num_entries = self.fcx.inner.borrow_mut().opaque_types().num_entries();
+        let prev = self.fcx.checked_opaque_types_storage_entries.replace(Some(num_entries));
+        debug_assert_eq!(prev, None);
         for (opaque_type_key, hidden_type) in opaque_types {
             let hidden_type = self.resolve(hidden_type, &hidden_type.span);
             let opaque_type_key = self.resolve(opaque_type_key, &hidden_type.span);

compiler/rustc_infer/src/infer/canonical/query_response.rs

@@ -132,7 +132,13 @@ impl<'tcx> InferCtxt<'tcx> {
 
         let certainty = if errors.is_empty() { Certainty::Proven } else { Certainty::Ambiguous };
 
-        let opaque_types = self.take_opaque_types_for_query_response();
+        let opaque_types = self
+            .inner
+            .borrow_mut()
+            .opaque_type_storage
+            .take_opaque_types()
+            .map(|(k, v)| (k, v.ty))
+            .collect();
 
         Ok(QueryResponse {
             var_values: inference_vars,
@@ -143,24 +149,6 @@ impl<'tcx> InferCtxt<'tcx> {
         })
     }
 
-    /// Used by the new solver as that one takes the opaque types at the end of a probe
-    /// to deal with multiple candidates without having to recompute them.
-    pub fn clone_opaque_types_for_query_response(
-        &self,
-    ) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> {
-        self.inner
-            .borrow()
-            .opaque_type_storage
-            .opaque_types
-            .iter()
-            .map(|(k, v)| (*k, v.ty))
-            .collect()
-    }
-
-    fn take_opaque_types_for_query_response(&self) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> {
-        self.take_opaque_types().into_iter().map(|(k, v)| (k, v.ty)).collect()
-    }
-
     /// Given the (canonicalized) result to a canonical query,
     /// instantiates the result so it can be used, plugging in the
     /// values from the canonical query. (Note that the result may

compiler/rustc_infer/src/infer/context.rs

@@ -6,7 +6,10 @@ use rustc_middle::ty::relate::combine::PredicateEmittingRelation;
 use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
 use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span};
 
-use super::{BoundRegionConversionTime, InferCtxt, RegionVariableOrigin, SubregionOrigin};
+use super::{
+    BoundRegionConversionTime, InferCtxt, OpaqueTypeStorageEntries, RegionVariableOrigin,
+    SubregionOrigin,
+};
 
 impl<'tcx> rustc_type_ir::InferCtxtLike for InferCtxt<'tcx> {
     type Interner = TyCtxt<'tcx>;
@@ -213,4 +216,58 @@ impl<'tcx> rustc_type_ir::InferCtxtLike for InferCtxt<'tcx> {
     fn register_ty_outlives(&self, ty: Ty<'tcx>, r: ty::Region<'tcx>, span: Span) {
         self.register_region_obligation_with_cause(ty, r, &ObligationCause::dummy_with_span(span));
     }
+
+    type OpaqueTypeStorageEntries = OpaqueTypeStorageEntries;
+    fn opaque_types_storage_num_entries(&self) -> OpaqueTypeStorageEntries {
+        self.inner.borrow_mut().opaque_types().num_entries()
+    }
+    fn clone_opaque_types_lookup_table(&self) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> {
+        self.inner.borrow_mut().opaque_types().iter_lookup_table().map(|(k, h)| (k, h.ty)).collect()
+    }
+    fn clone_duplicate_opaque_types(&self) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> {
+        self.inner
+            .borrow_mut()
+            .opaque_types()
+            .iter_duplicate_entries()
+            .map(|(k, h)| (k, h.ty))
+            .collect()
+    }
+    fn clone_opaque_types_added_since(
+        &self,
+        prev_entries: OpaqueTypeStorageEntries,
+    ) -> Vec<(ty::OpaqueTypeKey<'tcx>, Ty<'tcx>)> {
+        self.inner
+            .borrow_mut()
+            .opaque_types()
+            .opaque_types_added_since(prev_entries)
+            .map(|(k, h)| (k, h.ty))
+            .collect()
+    }
+
+    fn register_hidden_type_in_storage(
+        &self,
+        opaque_type_key: ty::OpaqueTypeKey<'tcx>,
+        hidden_ty: Ty<'tcx>,
+        span: Span,
+    ) -> Option<Ty<'tcx>> {
+        self.register_hidden_type_in_storage(
+            opaque_type_key,
+            ty::OpaqueHiddenType { span, ty: hidden_ty },
+        )
+    }
+    fn add_duplicate_opaque_type(
+        &self,
+        opaque_type_key: ty::OpaqueTypeKey<'tcx>,
+        hidden_ty: Ty<'tcx>,
+        span: Span,
+    ) {
+        self.inner
+            .borrow_mut()
+            .opaque_types()
+            .add_duplicate(opaque_type_key, ty::OpaqueHiddenType { span, ty: hidden_ty })
+    }
+
+    fn reset_opaque_types(&self) {
+        let _ = self.take_opaque_types();
+    }
 }

compiler/rustc_infer/src/infer/mod.rs

@@ -9,7 +9,7 @@ use free_regions::RegionRelations;
 pub use freshen::TypeFreshener;
 use lexical_region_resolve::LexicalRegionResolutions;
 pub use lexical_region_resolve::RegionResolutionError;
-use opaque_types::OpaqueTypeStorage;
+pub use opaque_types::{OpaqueTypeStorage, OpaqueTypeStorageEntries, OpaqueTypeTable};
 use region_constraints::{
     GenericKind, RegionConstraintCollector, RegionConstraintStorage, VarInfos, VerifyBound,
 };
@@ -31,9 +31,9 @@ use rustc_middle::traits::solve::Goal;
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::{
     self, BoundVarReplacerDelegate, ConstVid, FloatVid, GenericArg, GenericArgKind, GenericArgs,
-    GenericArgsRef, GenericParamDefKind, InferConst, IntVid, PseudoCanonicalInput, Term, TermKind,
-    Ty, TyCtxt, TyVid, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable,
-    TypeVisitableExt, TypingEnv, TypingMode, fold_regions,
+    GenericArgsRef, GenericParamDefKind, InferConst, IntVid, OpaqueHiddenType, OpaqueTypeKey,
+    PseudoCanonicalInput, Term, TermKind, Ty, TyCtxt, TyVid, TypeFoldable, TypeFolder,
+    TypeSuperFoldable, TypeVisitable, TypeVisitableExt, TypingEnv, TypingMode, fold_regions,
 };
 use rustc_span::{Span, Symbol};
 use snapshot::undo_log::InferCtxtUndoLogs;
@@ -198,7 +198,7 @@ impl<'tcx> InferCtxtInner<'tcx> {
     }
 
     #[inline]
-    fn opaque_types(&mut self) -> opaque_types::OpaqueTypeTable<'_, 'tcx> {
+    pub fn opaque_types(&mut self) -> opaque_types::OpaqueTypeTable<'_, 'tcx> {
         self.opaque_type_storage.with_log(&mut self.undo_log)
     }
 
@@ -224,15 +224,6 @@ impl<'tcx> InferCtxtInner<'tcx> {
             .expect("region constraints already solved")
             .with_log(&mut self.undo_log)
     }
-
-    // Iterates through the opaque type definitions without taking them; this holds the
-    // `InferCtxtInner` lock, so make sure to not do anything with `InferCtxt` side-effects
-    // while looping through this.
-    pub fn iter_opaque_types(
-        &self,
-    ) -> impl Iterator<Item = (ty::OpaqueTypeKey<'tcx>, ty::OpaqueHiddenType<'tcx>)> {
-        self.opaque_type_storage.opaque_types.iter().map(|(&k, &v)| (k, v))
-    }
 }
 
 pub struct InferCtxt<'tcx> {
@@ -954,13 +945,13 @@ impl<'tcx> InferCtxt<'tcx> {
     }
 
     #[instrument(level = "debug", skip(self), ret)]
-    pub fn take_opaque_types(&self) -> opaque_types::OpaqueTypeMap<'tcx> {
-        std::mem::take(&mut self.inner.borrow_mut().opaque_type_storage.opaque_types)
+    pub fn take_opaque_types(&self) -> Vec<(OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>)> {
+        self.inner.borrow_mut().opaque_type_storage.take_opaque_types().collect()
     }
 
     #[instrument(level = "debug", skip(self), ret)]
-    pub fn clone_opaque_types(&self) -> opaque_types::OpaqueTypeMap<'tcx> {
-        self.inner.borrow().opaque_type_storage.opaque_types.clone()
+    pub fn clone_opaque_types(&self) -> Vec<(OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>)> {
+        self.inner.borrow_mut().opaque_type_storage.iter_opaque_types().collect()
     }
 
     #[inline(always)]

compiler/rustc_infer/src/infer/opaque_types/mod.rs

@@ -1,5 +1,4 @@
 use hir::def_id::{DefId, LocalDefId};
-use rustc_data_structures::fx::FxIndexMap;
 use rustc_hir as hir;
 use rustc_middle::bug;
 use rustc_middle::traits::ObligationCause;
@@ -19,8 +18,7 @@ use crate::traits::{self, Obligation, PredicateObligations};
 
 mod table;
 
-pub(crate) type OpaqueTypeMap<'tcx> = FxIndexMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>>;
-pub(crate) use table::{OpaqueTypeStorage, OpaqueTypeTable};
+pub use table::{OpaqueTypeStorage, OpaqueTypeStorageEntries, OpaqueTypeTable};
 
 impl<'tcx> InferCtxt<'tcx> {
     /// This is a backwards compatibility hack to prevent breaking changes from