Auto merge of rust-lang#13840 - lowr:fix/hir-callable-sig-escaping-boundvars, r=lowr

fix: handle lifetime variables in `CallableSig` query

Fixes rust-lang#13838

The problem is similar to rust-lang#13223: we've been skipping non-empty binders, letting lifetime bound variables escape.

I ended up refactoring `hir_ty::callable_sig_from_fnonce()`. Like rust-lang#13223, I chose to make use of `InferenceTable` which is capable of handling variables (I feel we should always use it when we solve trait-related stuff instead of manually building obligations/queries).

I couldn't make up a test that crashes without this patch (since the function I'm fixing is only used *outside* `hir-ty`, simple `hir-ty` test wouldn't cause crash), but at least I tested with my local build and made sure it doesn't crash with the code in the original issue. I'd appreciate any help to find a regression test.

Author: bors

crates/hir-ty/src/lib.rs

@@ -40,15 +40,17 @@ use std::sync::Arc;
 use chalk_ir::{
     fold::{Shift, TypeFoldable},
     interner::HasInterner,
-    NoSolution, UniverseIndex,
+    NoSolution,
 };
 use hir_def::{expr::ExprId, type_ref::Rawness, TypeOrConstParamId};
 use hir_expand::name;
 use itertools::Either;
 use traits::FnTrait;
 use utils::Generics;
 
-use crate::{consteval::unknown_const, db::HirDatabase, utils::generics};
+use crate::{
+    consteval::unknown_const, db::HirDatabase, infer::unify::InferenceTable, utils::generics,
+};
 
 pub use autoderef::autoderef;
 pub use builder::{ParamKind, TyBuilder};
@@ -533,53 +535,31 @@ pub fn callable_sig_from_fnonce(
     let fn_once_trait = FnTrait::FnOnce.get_id(db, krate)?;
     let output_assoc_type = db.trait_data(fn_once_trait).associated_type_by_name(&name![Output])?;
 
+    let mut table = InferenceTable::new(db, env.clone());
     let b = TyBuilder::trait_ref(db, fn_once_trait);
     if b.remaining() != 2 {
         return None;
     }
-    let fn_once = b.push(self_ty.clone()).fill_with_bound_vars(DebruijnIndex::INNERMOST, 0).build();
-    let kinds = fn_once
-        .substitution
-        .iter(Interner)
-        .skip(1)
-        .map(|x| {
-            let vk = match x.data(Interner) {
-                chalk_ir::GenericArgData::Ty(_) => {
-                    chalk_ir::VariableKind::Ty(chalk_ir::TyVariableKind::General)
-                }
-                chalk_ir::GenericArgData::Lifetime(_) => chalk_ir::VariableKind::Lifetime,
-                chalk_ir::GenericArgData::Const(c) => {
-                    chalk_ir::VariableKind::Const(c.data(Interner).ty.clone())
-                }
-            };
-            chalk_ir::WithKind::new(vk, UniverseIndex::ROOT)
-        })
-        .collect::<Vec<_>>();
-
-    // FIXME: chalk refuses to solve `<Self as FnOnce<^0.0>>::Output == ^0.1`, so we first solve
-    // `<Self as FnOnce<^0.0>>` and then replace `^0.0` with the concrete argument tuple.
-    let trait_env = env.env.clone();
-    let obligation = InEnvironment { goal: fn_once.cast(Interner), environment: trait_env };
-    let canonical =
-        Canonical { binders: CanonicalVarKinds::from_iter(Interner, kinds), value: obligation };
-    let subst = match db.trait_solve(krate, canonical) {
-        Some(Solution::Unique(vars)) => vars.value.subst,
-        _ => return None,
-    };
-    let args = subst.at(Interner, 0).ty(Interner)?;
-    let params = match args.kind(Interner) {
-        chalk_ir::TyKind::Tuple(_, subst) => {
-            subst.iter(Interner).filter_map(|arg| arg.ty(Interner).cloned()).collect::<Vec<_>>()
-        }
-        _ => return None,
-    };
 
-    let fn_once =
-        TyBuilder::trait_ref(db, fn_once_trait).push(self_ty.clone()).push(args.clone()).build();
-    let projection =
-        TyBuilder::assoc_type_projection(db, output_assoc_type, Some(fn_once.substitution)).build();
+    // Register two obligations:
+    // - Self: FnOnce<?args_ty>
+    // - <Self as FnOnce<?args_ty>>::Output == ?ret_ty
+    let args_ty = table.new_type_var();
+    let trait_ref = b.push(self_ty.clone()).push(args_ty.clone()).build();
+    let projection = TyBuilder::assoc_type_projection(
+        db,
+        output_assoc_type,
+        Some(trait_ref.substitution.clone()),
+    )
+    .build();
+    table.register_obligation(trait_ref.cast(Interner));
+    let ret_ty = table.normalize_projection_ty(projection);
+
+    let ret_ty = table.resolve_completely(ret_ty);
+    let args_ty = table.resolve_completely(args_ty);
 
-    let ret_ty = db.normalize_projection(projection, env);
+    let params =
+        args_ty.as_tuple()?.iter(Interner).map(|it| it.assert_ty_ref(Interner)).cloned().collect();
 
     Some(CallableSig::from_params_and_return(params, ret_ty, false, Safety::Safe))
 }

crates/ide/src/syntax_highlighting/tests.rs

@@ -1028,6 +1028,26 @@ macro_rules! test {}
     let _ = analysis.highlight(HL_CONFIG, file_id).unwrap();
 }
 
+#[test]
+fn highlight_callable_no_crash() {
+    // regression test for #13838.
+    let (analysis, file_id) = fixture::file(
+        r#"
+//- minicore: fn, sized
+impl<A, F: ?Sized> FnOnce<A> for &F
+where
+    F: Fn<A>,
+{
+    type Output = F::Output;
+}
+
+trait Trait {}
+fn foo(x: &fn(&dyn Trait)) {}
+"#,
+    );
+    let _ = analysis.highlight(HL_CONFIG, file_id).unwrap();
+}
+
 /// Highlights the code given by the `ra_fixture` argument, renders the
 /// result as HTML, and compares it with the HTML file given as `snapshot`.
 /// Note that the `snapshot` file is overwritten by the rendered HTML.