remove sub_relations from infcx, recompute in diagnostics

we don't track them when canonicalizing or when freshening,
resulting in instable caching in the old solver, and issues when
instantiating query responses in the new one.

Author: lcnr

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -1522,10 +1522,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         if self.next_trait_solver()
             && let ty::Alias(..) = ty.kind()
         {
-            match self
+            // We need to use a separate variable here as otherwise the temporary for
+            // `self.fulfillment_cx.borrow_mut()` is alive in the `Err` branch, resulting
+            // in a reentrant borrow, causing an ICE.
+            let result = self
                 .at(&self.misc(sp), self.param_env)
-                .structurally_normalize(ty, &mut **self.fulfillment_cx.borrow_mut())
-            {
+                .structurally_normalize(ty, &mut **self.fulfillment_cx.borrow_mut());
+            match result {
                 Ok(normalized_ty) => normalized_ty,
                 Err(errors) => {
                     let guar = self.err_ctxt().report_fulfillment_errors(errors);

compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs

@@ -11,6 +11,7 @@ use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir_analysis::astconv::AstConv;
 use rustc_infer::infer;
+use rustc_infer::infer::error_reporting::sub_relations::SubRelations;
 use rustc_infer::infer::error_reporting::TypeErrCtxt;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind};
@@ -155,8 +156,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     ///
     /// [`InferCtxt::err_ctxt`]: infer::InferCtxt::err_ctxt
     pub fn err_ctxt(&'a self) -> TypeErrCtxt<'a, 'tcx> {
+        let mut sub_relations = SubRelations::default();
+        sub_relations.add_constraints(
+            self,
+            self.fulfillment_cx.borrow_mut().pending_obligations().iter().map(|o| o.predicate),
+        );
         TypeErrCtxt {
             infcx: &self.infcx,
+            sub_relations: RefCell::new(sub_relations),
             typeck_results: Some(self.typeck_results.borrow()),
             fallback_has_occurred: self.fallback_has_occurred.get(),
             normalize_fn_sig: Box::new(|fn_sig| {

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

@@ -88,6 +88,7 @@ mod note_and_explain;
 mod suggest;
 
 pub(crate) mod need_type_info;
+pub mod sub_relations;
 pub use need_type_info::TypeAnnotationNeeded;
 
 pub mod nice_region_error;
@@ -123,6 +124,8 @@ fn escape_literal(s: &str) -> String {
 /// methods which should not be used during the happy path.
 pub struct TypeErrCtxt<'a, 'tcx> {
     pub infcx: &'a InferCtxt<'tcx>,
+    pub sub_relations: std::cell::RefCell<sub_relations::SubRelations>,
+
     pub typeck_results: Option<std::cell::Ref<'a, ty::TypeckResults<'tcx>>>,
     pub fallback_has_occurred: bool,
 

compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs

@@ -502,7 +502,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     parent_name,
                 });
 
-                let args = if self.infcx.tcx.get_diagnostic_item(sym::iterator_collect_fn)
+                let args = if self.tcx.get_diagnostic_item(sym::iterator_collect_fn)
                     == Some(generics_def_id)
                 {
                     "Vec<_>".to_string()
@@ -710,7 +710,7 @@ struct InsertableGenericArgs<'tcx> {
 /// While doing so, the currently best spot is stored in `infer_source`.
 /// For details on how we rank spots, see [Self::source_cost]
 struct FindInferSourceVisitor<'a, 'tcx> {
-    infcx: &'a InferCtxt<'tcx>,
+    tecx: &'a TypeErrCtxt<'a, 'tcx>,
     typeck_results: &'a TypeckResults<'tcx>,
 
     target: GenericArg<'tcx>,
@@ -722,12 +722,12 @@ struct FindInferSourceVisitor<'a, 'tcx> {
 
 impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
     fn new(
-        infcx: &'a InferCtxt<'tcx>,
+        tecx: &'a TypeErrCtxt<'a, 'tcx>,
         typeck_results: &'a TypeckResults<'tcx>,
         target: GenericArg<'tcx>,
     ) -> Self {
         FindInferSourceVisitor {
-            infcx,
+            tecx,
             typeck_results,
 
             target,
@@ -778,7 +778,7 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
         }
 
         // The sources are listed in order of preference here.
-        let tcx = self.infcx.tcx;
+        let tcx = self.tecx.tcx;
         let ctx = CostCtxt { tcx };
         match source.kind {
             InferSourceKind::LetBinding { ty, .. } => ctx.ty_cost(ty),
@@ -829,12 +829,12 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
 
     fn node_args_opt(&self, hir_id: HirId) -> Option<GenericArgsRef<'tcx>> {
         let args = self.typeck_results.node_args_opt(hir_id);
-        self.infcx.resolve_vars_if_possible(args)
+        self.tecx.resolve_vars_if_possible(args)
     }
 
     fn opt_node_type(&self, hir_id: HirId) -> Option<Ty<'tcx>> {
         let ty = self.typeck_results.node_type_opt(hir_id);
-        self.infcx.resolve_vars_if_possible(ty)
+        self.tecx.resolve_vars_if_possible(ty)
     }
 
     // Check whether this generic argument is the inference variable we
@@ -849,20 +849,17 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
                 use ty::{Infer, TyVar};
                 match (inner_ty.kind(), target_ty.kind()) {
                     (&Infer(TyVar(a_vid)), &Infer(TyVar(b_vid))) => {
-                        self.infcx.inner.borrow_mut().type_variables().sub_unified(a_vid, b_vid)
+                        self.tecx.sub_relations.borrow_mut().unified(self.tecx, a_vid, b_vid)
                     }
                     _ => false,
                 }
             }
             (GenericArgKind::Const(inner_ct), GenericArgKind::Const(target_ct)) => {
                 use ty::InferConst::*;
                 match (inner_ct.kind(), target_ct.kind()) {
-                    (ty::ConstKind::Infer(Var(a_vid)), ty::ConstKind::Infer(Var(b_vid))) => self
-                        .infcx
-                        .inner
-                        .borrow_mut()
-                        .const_unification_table()
-                        .unioned(a_vid, b_vid),
+                    (ty::ConstKind::Infer(Var(a_vid)), ty::ConstKind::Infer(Var(b_vid))) => {
+                        self.tecx.inner.borrow_mut().const_unification_table().unioned(a_vid, b_vid)
+                    }
                     _ => false,
                 }
             }
@@ -917,7 +914,7 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
         &self,
         expr: &'tcx hir::Expr<'tcx>,
     ) -> Box<dyn Iterator<Item = InsertableGenericArgs<'tcx>> + 'a> {
-        let tcx = self.infcx.tcx;
+        let tcx = self.tecx.tcx;
         match expr.kind {
             hir::ExprKind::Path(ref path) => {
                 if let Some(args) = self.node_args_opt(expr.hir_id) {
@@ -980,7 +977,7 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
         path: &'tcx hir::Path<'tcx>,
         args: GenericArgsRef<'tcx>,
     ) -> impl Iterator<Item = InsertableGenericArgs<'tcx>> + 'a {
-        let tcx = self.infcx.tcx;
+        let tcx = self.tecx.tcx;
         let have_turbofish = path.segments.iter().any(|segment| {
             segment.args.is_some_and(|args| args.args.iter().any(|arg| arg.is_ty_or_const()))
         });
@@ -1034,7 +1031,7 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
         args: GenericArgsRef<'tcx>,
         qpath: &'tcx hir::QPath<'tcx>,
     ) -> Box<dyn Iterator<Item = InsertableGenericArgs<'tcx>> + 'a> {
-        let tcx = self.infcx.tcx;
+        let tcx = self.tecx.tcx;
         match qpath {
             hir::QPath::Resolved(_self_ty, path) => {
                 Box::new(self.resolved_path_inferred_arg_iter(path, args))
@@ -1107,7 +1104,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FindInferSourceVisitor<'a, 'tcx> {
     type NestedFilter = nested_filter::OnlyBodies;
 
     fn nested_visit_map(&mut self) -> Self::Map {
-        self.infcx.tcx.hir()
+        self.tecx.tcx.hir()
     }
 
     fn visit_local(&mut self, local: &'tcx Local<'tcx>) {
@@ -1163,7 +1160,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FindInferSourceVisitor<'a, 'tcx> {
 
     #[instrument(level = "debug", skip(self))]
     fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
-        let tcx = self.infcx.tcx;
+        let tcx = self.tecx.tcx;
         match expr.kind {
             // When encountering `func(arg)` first look into `arg` and then `func`,
             // as `arg` is "more specific".
@@ -1194,7 +1191,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FindInferSourceVisitor<'a, 'tcx> {
                 if generics.parent.is_none() && generics.has_self {
                     argument_index += 1;
                 }
-                let args = self.infcx.resolve_vars_if_possible(args);
+                let args = self.tecx.resolve_vars_if_possible(args);
                 let generic_args =
                     &generics.own_args_no_defaults(tcx, args)[generics.own_counts().lifetimes..];
                 let span = match expr.kind {
@@ -1224,7 +1221,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FindInferSourceVisitor<'a, 'tcx> {
             {
                 let output = args.as_closure().sig().output().skip_binder();
                 if self.generic_arg_contains_target(output.into()) {
-                    let body = self.infcx.tcx.hir().body(body);
+                    let body = self.tecx.tcx.hir().body(body);
                     let should_wrap_expr = if matches!(body.value.kind, ExprKind::Block(..)) {
                         None
                     } else {
@@ -1252,12 +1249,12 @@ impl<'a, 'tcx> Visitor<'tcx> for FindInferSourceVisitor<'a, 'tcx> {
             && let Some(args) = self.node_args_opt(expr.hir_id)
             && args.iter().any(|arg| self.generic_arg_contains_target(arg))
             && let Some(def_id) = self.typeck_results.type_dependent_def_id(expr.hir_id)
-            && self.infcx.tcx.trait_of_item(def_id).is_some()
+            && self.tecx.tcx.trait_of_item(def_id).is_some()
             && !has_impl_trait(def_id)
         {
             let successor =
                 method_args.get(0).map_or_else(|| (")", span.hi()), |arg| (", ", arg.span.lo()));
-            let args = self.infcx.resolve_vars_if_possible(args);
+            let args = self.tecx.resolve_vars_if_possible(args);
             self.update_infer_source(InferSource {
                 span: path.ident.span,
                 kind: InferSourceKind::FullyQualifiedMethodCall {

compiler/rustc_infer/src/infer/error_reporting/sub_relations.rs

@@ -0,0 +1,81 @@
+use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::undo_log::NoUndo;
+use rustc_data_structures::unify as ut;
+use rustc_middle::ty;
+
+use crate::infer::InferCtxt;
+
+#[derive(Debug, Copy, Clone, PartialEq)]
+struct SubId(u32);
+impl ut::UnifyKey for SubId {
+    type Value = ();
+    #[inline]
+    fn index(&self) -> u32 {
+        self.0
+    }
+    #[inline]
+    fn from_index(i: u32) -> SubId {
+        SubId(i)
+    }
+    fn tag() -> &'static str {
+        "SubId"
+    }
+}
+
+/// When reporting ambiguity errors, we sometimes want to
+/// treat all inference vars which are subtypes of each
+/// others as if they are equal. For this case we compute
+/// the transitive closure of our subtype obligations here.
+///
+/// E.g. when encountering ambiguity errors, we want to suggest
+/// specifying some method argument or to add a type annotation
+/// to a local variable. Because subtyping cannot change the
+/// shape of a type, it's fine if the cause of the ambiguity error
+/// is only related to the suggested variable via subtyping.
+///
+/// Even for something like `let x = returns_arg(); x.method();` the
+/// type of `x` is only a supertype of the argument of `returns_arg`. We
+/// still want to suggest specifying the type of the argument.
+#[derive(Default)]
+pub struct SubRelations {
+    map: FxHashMap<ty::TyVid, SubId>,
+    table: ut::UnificationTableStorage<SubId>,
+}
+
+impl SubRelations {
+    fn get_id<'tcx>(&mut self, infcx: &InferCtxt<'tcx>, vid: ty::TyVid) -> SubId {
+        let root_vid = infcx.root_var(vid);
+        *self.map.entry(root_vid).or_insert_with(|| self.table.with_log(&mut NoUndo).new_key(()))
+    }
+
+    pub fn add_constraints<'tcx>(
+        &mut self,
+        infcx: &InferCtxt<'tcx>,
+        obls: impl IntoIterator<Item = ty::Predicate<'tcx>>,
+    ) {
+        for p in obls {
+            let (a, b) = match p.kind().skip_binder() {
+                ty::PredicateKind::Subtype(ty::SubtypePredicate { a_is_expected: _, a, b }) => {
+                    (a, b)
+                }
+                ty::PredicateKind::Coerce(ty::CoercePredicate { a, b }) => (a, b),
+                _ => continue,
+            };
+
+            match (a.kind(), b.kind()) {
+                (&ty::Infer(ty::TyVar(a_vid)), &ty::Infer(ty::TyVar(b_vid))) => {
+                    let a = self.get_id(infcx, a_vid);
+                    let b = self.get_id(infcx, b_vid);
+                    self.table.with_log(&mut NoUndo).unify_var_var(a, b).unwrap();
+                }
+                _ => continue,
+            }
+        }
+    }
+
+    pub fn unified<'tcx>(&mut self, infcx: &InferCtxt<'tcx>, a: ty::TyVid, b: ty::TyVid) -> bool {
+        let a = self.get_id(infcx, a);
+        let b = self.get_id(infcx, b);
+        self.table.with_log(&mut NoUndo).unioned(a, b)
+    }
+}

compiler/rustc_infer/src/infer/mod.rs

@@ -762,6 +762,7 @@ impl<'tcx> InferCtxt<'tcx> {
     pub fn err_ctxt(&self) -> TypeErrCtxt<'_, 'tcx> {
         TypeErrCtxt {
             infcx: self,
+            sub_relations: Default::default(),
             typeck_results: None,
             fallback_has_occurred: false,
             normalize_fn_sig: Box::new(|fn_sig| fn_sig),
@@ -1029,7 +1030,6 @@ impl<'tcx> InferCtxt<'tcx> {
         let r_b = self.shallow_resolve(predicate.skip_binder().b);
         match (r_a.kind(), r_b.kind()) {
             (&ty::Infer(ty::TyVar(a_vid)), &ty::Infer(ty::TyVar(b_vid))) => {
-                self.inner.borrow_mut().type_variables().sub(a_vid, b_vid);
                 return Err((a_vid, b_vid));
             }
             _ => {}

compiler/rustc_infer/src/infer/relate/generalize.rs

@@ -217,10 +217,9 @@ impl<'tcx> InferCtxt<'tcx> {
     ) -> RelateResult<'tcx, Generalization<T>> {
         assert!(!source_term.has_escaping_bound_vars());
         let (for_universe, root_vid) = match target_vid.into() {
-            ty::TermVid::Ty(ty_vid) => (
-                self.probe_ty_var(ty_vid).unwrap_err(),
-                ty::TermVid::Ty(self.inner.borrow_mut().type_variables().sub_root_var(ty_vid)),
-            ),
+            ty::TermVid::Ty(ty_vid) => {
+                (self.probe_ty_var(ty_vid).unwrap_err(), ty::TermVid::Ty(self.root_var(ty_vid)))
+            }
             ty::TermVid::Const(ct_vid) => (
                 self.probe_const_var(ct_vid).unwrap_err(),
                 ty::TermVid::Const(
@@ -424,9 +423,7 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
             ty::Infer(ty::TyVar(vid)) => {
                 let mut inner = self.infcx.inner.borrow_mut();
                 let vid = inner.type_variables().root_var(vid);
-                let sub_vid = inner.type_variables().sub_root_var(vid);
-
-                if ty::TermVid::Ty(sub_vid) == self.root_vid {
+                if ty::TermVid::Ty(vid) == self.root_vid {
                     // If sub-roots are equal, then `root_vid` and
                     // `vid` are related via subtyping.
                     Err(self.cyclic_term_error())
@@ -461,11 +458,6 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
                             let new_var_id =
                                 inner.type_variables().new_var(self.for_universe, origin);
                             let u = Ty::new_var(self.tcx(), new_var_id);
-
-                            // Record that we replaced `vid` with `new_var_id` as part of a generalization
-                            // operation. This is needed to detect cyclic types. To see why, see the
-                            // docs in the `type_variables` module.
-                            inner.type_variables().sub(vid, new_var_id);
                             debug!("replacing original vid={:?} with new={:?}", vid, u);
                             Ok(u)
                         }