Port call expr type checking and closure upvar inference from rustc

Author: ChayimFriedman2

Cargo.lock

@@ -1,3 +1,4 @@
+//- minicore: fn
 use crate::ast::{AngleBracketedArgs, ParenthesizedArgs, AttrStyle, BareFnTy};
 use crate::ast::{GenericBound, TraitBoundModifier};
 use crate::ast::Unsafety;

bench_data/glorious_old_parser

@@ -929,6 +929,13 @@ impl ExpressionStore {
         // We keep the async closure exactly one expr before.
         ExprId::from_raw(la_arena::RawIdx::from_u32(coroutine_closure.into_raw().into_u32() - 1))
     }
+
+    /// The opposite of [`Self::coroutine_for_closure()`].
+    #[inline]
+    pub fn closure_for_coroutine(coroutine: ExprId) -> ExprId {
+        // We keep the async closure exactly one expr before.
+        ExprId::from_raw(la_arena::RawIdx::from_u32(coroutine.into_raw().into_u32() + 1))
+    }
 }
 
 impl Index<ExprId> for ExpressionStore {

crates/hir-def/src/expr_store.rs

@@ -945,12 +945,19 @@ impl<'db> ExprCollector<'db> {
         })
     }
 
-    /// An `async fn` needs to capture all parameters in the generated `async` block, even if they have
-    /// non-captured patterns such as wildcards (to ensure consistent drop order).
-    fn lower_async_fn(&mut self, params: &mut Vec<PatId>, body: ExprId) -> ExprId {
+    /// Lowers a desugared coroutine body after moving all of the arguments
+    /// into the body. This is to make sure that the future actually owns the
+    /// arguments that are passed to the function, and to ensure things like
+    /// drop order are stable.
+    fn lower_async_block_with_moved_arguments(
+        &mut self,
+        params: &mut [PatId],
+        body: ExprId,
+        coroutine_source: CoroutineSource,
+    ) -> ExprId {
         let mut statements = Vec::new();
         for param in params {
-            let name = match self.store.pats[*param] {
+            let (name, hygiene) = match self.store.pats[*param] {
                 Pat::Bind { id, .. }
                     if matches!(
                         self.store.bindings[id].mode,
@@ -962,14 +969,16 @@ impl<'db> ExprCollector<'db> {
                 }
                 Pat::Bind { id, .. } => {
                     // If this is a `ref` binding, we can't leave it as is but we can at least reuse the name, for better display.
-                    self.store.bindings[id].name.clone()
+                    (self.store.bindings[id].name.clone(), self.store.bindings[id].hygiene)
                 }
-                _ => self.generate_new_name(),
+                _ => (self.generate_new_name(), HygieneId::ROOT),
             };
-            let binding_id =
-                self.alloc_binding(name.clone(), BindingAnnotation::Mutable, HygieneId::ROOT);
+            let binding_id = self.alloc_binding(name.clone(), BindingAnnotation::Mutable, hygiene);
             let pat_id = self.alloc_pat_desugared(Pat::Bind { id: binding_id, subpat: None });
             let expr = self.alloc_expr_desugared(Expr::Path(name.into()));
+            if !hygiene.is_root() {
+                self.store.ident_hygiene.insert(expr.into(), hygiene);
+            }
             statements.push(Statement::Let {
                 pat: *param,
                 type_ref: None,
@@ -980,12 +989,17 @@ impl<'db> ExprCollector<'db> {
         }
 
         let async_ = self.async_block(
-            CoroutineSource::Fn,
-            CaptureBy::Value,
+            coroutine_source,
+            // The default capture mode here is by-ref. Later on during upvar analysis,
+            // we will force the captured arguments to by-move, but for async closures,
+            // we want to make sure that we avoid unnecessarily moving captures, or else
+            // all async closures would default to `FnOnce` as their calling mode.
+            CaptureBy::Ref,
             None,
             statements.into_boxed_slice(),
             Some(body),
         );
+        // It's important that this comes last, see the lowering of async closures for why.
         self.alloc_expr_desugared(async_)
     }
 
@@ -1010,14 +1024,18 @@ impl<'db> ExprCollector<'db> {
 
     fn collect(
         &mut self,
-        params: &mut Vec<PatId>,
+        params: &mut [PatId],
         expr: Option<ast::Expr>,
         awaitable: Awaitable,
     ) -> ExprId {
         self.awaitable_context.replace(awaitable);
         self.with_label_rib(RibKind::Closure, |this| {
             let body = this.collect_expr_opt(expr);
-            if awaitable == Awaitable::Yes { this.lower_async_fn(params, body) } else { body }
+            if awaitable == Awaitable::Yes {
+                this.lower_async_block_with_moved_arguments(params, body, CoroutineSource::Fn)
+            } else {
+                body
+            }
         })
     }
 
@@ -1450,18 +1468,11 @@ impl<'db> ExprCollector<'db> {
                     } else if e.async_token().is_some() {
                         // It's important that this expr is allocated immediately before the closure.
                         // We rely on it for `coroutine_for_closure()`.
-                        body = this.alloc_expr_desugared(Expr::Closure {
-                            args: Box::default(),
-                            arg_types: Box::default(),
-                            ret_type: None,
+                        body = this.lower_async_block_with_moved_arguments(
+                            &mut args,
                             body,
-                            closure_kind: ClosureKind::AsyncBlock {
-                                source: CoroutineSource::Closure,
-                            },
-                            // The block may need to capture by move, but we cannot know it now.
-                            // It will be fixed in capture analysis.
-                            capture_by: CaptureBy::Ref,
-                        });
+                            CoroutineSource::Closure,
+                        );
                         body_is_bindings_owner = true;
 
                         ClosureKind::AsyncClosure

crates/hir-def/src/expr_store/lower.rs

@@ -652,7 +652,7 @@ fn async_fn_weird_param_patterns() {
 async fn main(&self, param1: i32, ref mut param2: i32, _: i32, param4 @ _: i32, 123: i32) {}
 "#,
         expect![[r#"
-            fn main(self, param1, mut param2, mut <ra@gennew>0, param4 @ _, mut <ra@gennew>1) async move {
+            fn main(self, param1, mut param2, mut <ra@gennew>0, param4 @ _, mut <ra@gennew>1) async {
                 let ref mut param2 = param2;
                 let _ = <ra@gennew>0;
                 let 123 = <ra@gennew>1;

crates/hir-def/src/expr_store/tests/body.rs

@@ -306,6 +306,7 @@ language_item_table! { LangItems =>
     /// Trait injected by `#[derive(Eq)]`, (i.e. "Total EQ"; no, I will not apologize).
     StructuralTeq,           sym::structural_teq,      TraitId;
     Copy,                    sym::copy,                TraitId;
+    UseCloned,               sym::use_cloned,          TraitId;
     Clone,                   sym::clone,               TraitId;
     TrivialClone,            sym::trivial_clone,       TraitId;
     Sync,                    sym::sync,                TraitId;

crates/hir-def/src/lang_item.rs

@@ -1792,14 +1792,14 @@ const GOAL: i32 = {
 fn closure_capture_unsized_type() {
     check_number(
         r#"
-    //- minicore: fn, copy, slice, index, coerce_unsized
+    //- minicore: fn, copy, slice, index, coerce_unsized, sized
     fn f<T: A>(x: &<T as A>::Ty) -> &<T as A>::Ty {
         let c = || &*x;
         c()
     }
 
     trait A {
-        type Ty;
+        type Ty: ?Sized;
     }
 
     impl A for i32 {
@@ -1810,7 +1810,7 @@ fn closure_capture_unsized_type() {
         let k: &[u8] = &[1, 2, 3];
         let k = f::<i32>(k);
         k[0] + k[1] + k[2]
-    }
+    };
     "#,
         6,
     );

crates/hir-ty/src/consteval/tests.rs

@@ -238,8 +238,7 @@ impl<'db> ExprValidator<'db> {
             if (pat_ty == scrut_ty
                 || scrut_ty
                     .as_reference()
-                    .map(|(match_expr_ty, ..)| match_expr_ty == pat_ty)
-                    .unwrap_or(false))
+                    .is_none_or(|(match_expr_ty, ..)| match_expr_ty == pat_ty))
                 && types_of_subpatterns_do_match(arm.pat, self.body, self.infer)
             {
                 // If we had a NotUsefulMatchArm diagnostic, we could

crates/hir-ty/src/diagnostics/expr.rs

@@ -47,7 +47,7 @@ use span::Edition;
 use stdx::never;
 
 use crate::{
-    CallableDefId, FnAbi, ImplTraitId, InferenceResult, MemoryMap, ParamEnvAndCrate, consteval,
+    CallableDefId, FnAbi, ImplTraitId, MemoryMap, ParamEnvAndCrate, consteval,
     db::{HirDatabase, InternedClosure},
     generics::generics,
     layout::Layout,
@@ -1495,9 +1495,7 @@ impl<'db> HirDisplay<'db> for Ty<'db> {
                 }
                 let sig = interner.signature_unclosure(substs.as_closure().sig(), Safety::Safe);
                 let sig = sig.skip_binder();
-                let InternedClosure(owner, _) = id.loc(db);
-                let infer = InferenceResult::of(db, owner);
-                let (_, kind) = infer.closure_info(id);
+                let kind = substs.as_closure().kind();
                 match f.closure_style {
                     ClosureStyle::ImplFn => write!(f, "impl {kind:?}(")?,
                     ClosureStyle::RANotation => write!(f, "|")?,

crates/hir-ty/src/display.rs

@@ -1,15 +1,16 @@
 //! Utilities for computing drop info about types.
 
 use hir_def::{
-    AdtId,
+    AdtId, ImplId,
     signatures::{StructFlags, StructSignature},
 };
 use rustc_hash::FxHashSet;
-use rustc_type_ir::inherent::{AdtDef, IntoKind};
+use rustc_type_ir::inherent::{AdtDef, GenericArgs as _, IntoKind};
 use stdx::never;
 
 use crate::{
-    InferenceResult, consteval,
+    consteval,
+    db::HirDatabase,
     method_resolution::TraitImpls,
     next_solver::{
         DbInterner, ParamEnv, SimplifiedType, Ty, TyKind,
@@ -18,24 +19,23 @@ use crate::{
     },
 };
 
-fn has_destructor(interner: DbInterner<'_>, adt: AdtId) -> bool {
-    let db = interner.db;
+#[salsa::tracked]
+pub fn destructor(db: &dyn HirDatabase, adt: AdtId) -> Option<ImplId> {
     let module = match adt {
         AdtId::EnumId(id) => db.lookup_intern_enum(id).container,
         AdtId::StructId(id) => db.lookup_intern_struct(id).container,
         AdtId::UnionId(id) => db.lookup_intern_union(id).container,
     };
-    let Some(drop_trait) = interner.lang_items().Drop else {
-        return false;
-    };
+    let interner = DbInterner::new_with(db, module.krate(db));
+    let drop_trait = interner.lang_items().Drop?;
     let impls = match module.block(db) {
         Some(block) => match TraitImpls::for_block(db, block) {
             Some(it) => &**it,
-            None => return false,
+            None => return None,
         },
         None => TraitImpls::for_crate(db, module.krate(db)),
     };
-    !impls.for_trait_and_self_ty(drop_trait, &SimplifiedType::Adt(adt.into())).0.is_empty()
+    impls.for_trait_and_self_ty(drop_trait, &SimplifiedType::Adt(adt.into())).0.first().copied()
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
@@ -71,7 +71,7 @@ fn has_drop_glue_impl<'db>(
     match ty.kind() {
         TyKind::Adt(adt_def, subst) => {
             let adt_id = adt_def.def_id().0;
-            if has_destructor(infcx.interner, adt_id) {
+            if adt_def.destructor(infcx.interner).is_some() {
                 return DropGlue::HasDropGlue;
             }
             match adt_id {
@@ -132,21 +132,17 @@ fn has_drop_glue_impl<'db>(
             has_drop_glue_impl(infcx, ty, env, visited)
         }
         TyKind::Slice(ty) => has_drop_glue_impl(infcx, ty, env, visited),
-        TyKind::Closure(closure_id, subst) => {
-            let owner = closure_id.0.loc(db).0;
-            let infer = InferenceResult::of(db, owner);
-            let (captures, _) = infer.closure_info(closure_id.0);
-            let env = db.trait_environment(owner);
-            captures
-                .iter()
-                .map(|capture| has_drop_glue_impl(infcx, capture.ty(db, subst), env, visited))
-                .max()
-                .unwrap_or(DropGlue::None)
+        TyKind::Closure(_, args) => {
+            has_drop_glue_impl(infcx, args.as_closure().tupled_upvars_ty(), env, visited)
         }
-        // FIXME: Handle coroutines.
-        TyKind::Coroutine(..) | TyKind::CoroutineWitness(..) | TyKind::CoroutineClosure(..) => {
-            DropGlue::None
+        TyKind::Coroutine(_, args) => {
+            has_drop_glue_impl(infcx, args.as_coroutine().tupled_upvars_ty(), env, visited)
+        }
+        TyKind::CoroutineClosure(_, args) => {
+            has_drop_glue_impl(infcx, args.as_coroutine_closure().tupled_upvars_ty(), env, visited)
         }
+        // FIXME: Coroutine witness.
+        TyKind::CoroutineWitness(..) => DropGlue::None,
         TyKind::Ref(..)
         | TyKind::RawPtr(..)
         | TyKind::FnDef(..)