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

@@ -38,6 +38,12 @@ pub(crate) trait EnumerateAndAdjustIterator {
 }
 
 impl<T: ExactSizeIterator> EnumerateAndAdjustIterator for T {
+    /// When there is a list of items with a gap of an unknown length inside, and another list
+    /// of item it should be zipped against, this operates on the list with the gap and returns,
+    /// for each item, the index it should match in the other list.
+    ///
+    /// When compiling Rust, such situation often occurs for tuple structs/tuples with a rest pattern
+    /// that should be matched against the fields.
     fn enumerate_and_adjust(
         self,
         expected_len: usize,

crates/hir-ty/src/diagnostics/match_check/pat_util.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, "|")?,