Auto merge of #119258 - compiler-errors:closure-kind, r=eholk

Make closures carry their own ClosureKind

Right now, we use the "`movability`" field of `hir::Closure` to distinguish a closure and a coroutine. This is paired together with the `CoroutineKind`, which is located not in the `hir::Closure`, but the `hir::Body`. This is strange and redundant.

This PR introduces `ClosureKind` with two variants -- `Closure` and `Coroutine`, which is put into `hir::Closure`. The `CoroutineKind` is thus removed from `hir::Body`, and `Option<Movability>` no longer needs to be a stand-in for "is this a closure or a coroutine".

r? eholk

clippy_lints/src/async_yields_async.rs

@@ -2,7 +2,9 @@ use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::source::snippet;
 use clippy_utils::ty::implements_trait;
 use rustc_errors::Applicability;
-use rustc_hir::{Body, BodyId, CoroutineKind, CoroutineSource, CoroutineDesugaring, ExprKind, QPath};
+use rustc_hir::{
+    Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, QPath,
+};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 
@@ -44,15 +46,22 @@ declare_clippy_lint! {
 declare_lint_pass!(AsyncYieldsAsync => [ASYNC_YIELDS_ASYNC]);
 
 impl<'tcx> LateLintPass<'tcx> for AsyncYieldsAsync {
-    fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
         // For functions, with explicitly defined types, don't warn.
         // XXXkhuey maybe we should?
-        if let Some(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Block | CoroutineSource::Closure)) = body.coroutine_kind {
+        if let ExprKind::Closure(Closure {
+            kind:
+                ClosureKind::Coroutine(CoroutineKind::Desugared(
+                    CoroutineDesugaring::Async,
+                    CoroutineSource::Block | CoroutineSource::Closure,
+                )),
+            body: body_id,
+            ..
+        }) = expr.kind
+        {
             if let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() {
-                let body_id = BodyId {
-                    hir_id: body.value.hir_id,
-                };
-                let typeck_results = cx.tcx.typeck_body(body_id);
+                let typeck_results = cx.tcx.typeck_body(*body_id);
+                let body = cx.tcx.hir().body(*body_id);
                 let expr_ty = typeck_results.expr_ty(body.value);
 
                 if implements_trait(cx, expr_ty, future_trait_def_id, &[]) {

clippy_lints/src/await_holding_invalid.rs

@@ -2,8 +2,8 @@ use clippy_config::types::DisallowedPath;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::{match_def_path, paths};
 use rustc_data_structures::fx::FxHashMap;
+use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
-use rustc_hir::{Body, CoroutineKind, CoroutineDesugaring};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::mir::CoroutineLayout;
 use rustc_session::impl_lint_pass;
@@ -183,8 +183,8 @@ impl AwaitHolding {
     }
 }
 
-impl LateLintPass<'_> for AwaitHolding {
-    fn check_crate(&mut self, cx: &LateContext<'_>) {
+impl<'tcx> LateLintPass<'tcx> for AwaitHolding {
+    fn check_crate(&mut self, cx: &LateContext<'tcx>) {
         for conf in &self.conf_invalid_types {
             let segs: Vec<_> = conf.path().split("::").collect();
             for id in clippy_utils::def_path_def_ids(cx, &segs) {
@@ -193,10 +193,14 @@ impl LateLintPass<'_> for AwaitHolding {
         }
     }
 
-    fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
-        if let Some(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)) = body.coroutine_kind {
-            let def_id = cx.tcx.hir().body_owner_def_id(body.id());
-            if let Some(coroutine_layout) = cx.tcx.mir_coroutine_witnesses(def_id) {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
+        if let hir::ExprKind::Closure(hir::Closure {
+            kind: hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, _)),
+            def_id,
+            ..
+        }) = expr.kind
+        {
+            if let Some(coroutine_layout) = cx.tcx.mir_coroutine_witnesses(*def_id) {
                 self.check_interior_types(cx, coroutine_layout);
             }
         }

clippy_lints/src/manual_async_fn.rs

@@ -3,8 +3,9 @@ use clippy_utils::source::{position_before_rarrow, snippet_block, snippet_opt};
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::FnKind;
 use rustc_hir::{
-    Block, Body, Closure, CoroutineKind, CoroutineSource, CoroutineDesugaring, Expr, ExprKind, FnDecl, FnRetTy, GenericArg, GenericBound,
-    ImplItem, Item, ItemKind, LifetimeName, Node, Term, TraitRef, Ty, TyKind, TypeBindingKind,
+    Block, Body, Closure, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, FnDecl, FnRetTy,
+    GenericArg, GenericBound, ImplItem, Item, ItemKind, LifetimeName, Node, Term, TraitRef, Ty, TyKind,
+    TypeBindingKind, ClosureKind,
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
@@ -171,16 +172,25 @@ fn captures_all_lifetimes(inputs: &[Ty<'_>], output_lifetimes: &[LifetimeName])
             .all(|in_lt| output_lifetimes.iter().any(|out_lt| in_lt == out_lt))
 }
 
-fn desugared_async_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) -> Option<&'tcx Body<'tcx>> {
-    if let Some(block_expr) = block.expr
-        && let Expr {
-            kind: ExprKind::Closure(&Closure { body, .. }),
-            ..
-        } = block_expr
-        && let closure_body = cx.tcx.hir().body(body)
-        && closure_body.coroutine_kind == Some(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Block))
+fn desugared_async_block<'tcx>(
+    cx: &LateContext<'tcx>,
+    block: &'tcx Block<'tcx>,
+) -> Option<&'tcx Body<'tcx>> {
+    if let Some(Expr {
+        kind:
+            ExprKind::Closure(&Closure {
+                kind:
+                    ClosureKind::Coroutine(CoroutineKind::Desugared(
+                        CoroutineDesugaring::Async,
+                        CoroutineSource::Block,
+                    )),
+                body,
+                ..
+            }),
+        ..
+    }) = block.expr
     {
-        return Some(closure_body);
+        return Some(cx.tcx.hir().body(body));
     }
 
     None

clippy_lints/src/methods/iter_kv_map.rs

@@ -32,7 +32,6 @@ pub(super) fn check<'tcx>(
         && let Body {
             params: [p],
             value: body_expr,
-            coroutine_kind: _,
         } = cx.tcx.hir().body(c.body)
         && let PatKind::Tuple([key_pat, val_pat], _) = p.pat.kind
         && let (replacement_kind, annotation, bound_ident) = match (&key_pat.kind, &val_pat.kind) {

clippy_lints/src/needless_question_mark.rs

@@ -3,7 +3,7 @@ use clippy_utils::path_res;
 use clippy_utils::source::snippet;
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::{Block, Body, CoroutineKind, CoroutineSource, CoroutineDesugaring, Expr, ExprKind, LangItem, MatchSource, QPath};
+use rustc_hir::{Block, Body, Expr, ExprKind, LangItem, MatchSource, QPath};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 
@@ -86,22 +86,20 @@ impl LateLintPass<'_> for NeedlessQuestionMark {
     }
 
     fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
-        if let Some(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Fn)) = body.coroutine_kind {
-            if let ExprKind::Block(
-                Block {
-                    expr:
-                        Some(Expr {
-                            kind: ExprKind::DropTemps(async_body),
-                            ..
-                        }),
-                    ..
-                },
-                _,
-            ) = body.value.kind
-            {
-                if let ExprKind::Block(Block { expr: Some(expr), .. }, ..) = async_body.kind {
-                    check(cx, expr);
-                }
+        if let ExprKind::Block(
+            Block {
+                expr:
+                    Some(Expr {
+                        kind: ExprKind::DropTemps(async_body),
+                        ..
+                    }),
+                ..
+            },
+            _,
+        ) = body.value.kind
+        {
+            if let ExprKind::Block(Block { expr: Some(expr), .. }, ..) = async_body.kind {
+                check(cx, expr.peel_blocks());
             }
         } else {
             check(cx, body.value.peel_blocks());

clippy_lints/src/redundant_async_block.rs

@@ -5,7 +5,7 @@ use clippy_utils::peel_blocks;
 use clippy_utils::source::{snippet, walk_span_to_context};
 use clippy_utils::visitors::for_each_expr;
 use rustc_errors::Applicability;
-use rustc_hir::{Closure, CoroutineKind, CoroutineSource, CoroutineDesugaring, Expr, ExprKind, MatchSource};
+use rustc_hir::{Closure, ClosureKind, CoroutineKind, CoroutineSource, CoroutineDesugaring, Expr, ExprKind, MatchSource};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::UpvarCapture;
@@ -69,9 +69,9 @@ impl<'tcx> LateLintPass<'tcx> for RedundantAsyncBlock {
 /// If `expr` is a desugared `async` block, return the original expression if it does not capture
 /// any variable by ref.
 fn desugar_async_block<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
-    if let ExprKind::Closure(Closure { body, def_id, .. }) = expr.kind
+    if let ExprKind::Closure(Closure { body, def_id, kind, .. }) = expr.kind
         && let body = cx.tcx.hir().body(*body)
-        && matches!(body.coroutine_kind, Some(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Block)))
+        && matches!(kind, ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Block)))
     {
         cx.typeck_results()
             .closure_min_captures

clippy_lints/src/redundant_closure_call.rs

@@ -5,7 +5,7 @@ use clippy_utils::sugg::Sugg;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_hir::intravisit::{Visitor as HirVisitor, Visitor};
-use rustc_hir::{intravisit as hir_visit, CoroutineKind, CoroutineSource, CoroutineDesugaring, Node};
+use rustc_hir::{intravisit as hir_visit, CoroutineKind, CoroutineSource, CoroutineDesugaring, Node, ClosureKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::nested_filter;
 use rustc_middle::lint::in_external_macro;
@@ -63,11 +63,10 @@ impl<'tcx> Visitor<'tcx> for ReturnVisitor {
 /// Checks if the body is owned by an async closure.
 /// Returns true for `async || whatever_expression`, but false for `|| async { whatever_expression
 /// }`.
-fn is_async_closure(cx: &LateContext<'_>, body: &hir::Body<'_>) -> bool {
+fn is_async_closure(body: &hir::Body<'_>) -> bool {
     if let hir::ExprKind::Closure(innermost_closure_generated_by_desugar) = body.value.kind
-        && let desugared_inner_closure_body = cx.tcx.hir().body(innermost_closure_generated_by_desugar.body)
         // checks whether it is `async || whatever_expression`
-        && let Some(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Closure)) = desugared_inner_closure_body.coroutine_kind
+        && let ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Closure)) = innermost_closure_generated_by_desugar.kind
     {
         true
     } else {
@@ -103,7 +102,7 @@ fn find_innermost_closure<'tcx>(
         data = Some((
             body.value,
             closure.fn_decl,
-            if is_async_closure(cx, body) {
+            if is_async_closure(body) {
                 ty::Asyncness::Yes
             } else {
                 ty::Asyncness::No

clippy_lints/src/unused_async.rs

@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::is_def_id_trait_method;
 use rustc_hir::def::DefKind;
-use rustc_hir::intravisit::{walk_body, walk_expr, walk_fn, FnKind, Visitor};
+use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, Visitor};
 use rustc_hir::{Body, Expr, ExprKind, FnDecl, Node, YieldSource};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::nested_filter;
@@ -78,32 +78,32 @@ impl<'a, 'tcx> Visitor<'tcx> for AsyncFnVisitor<'a, 'tcx> {
                 self.await_in_async_block = Some(ex.span);
             }
         }
-        walk_expr(self, ex);
-    }
 
-    fn nested_visit_map(&mut self) -> Self::Map {
-        self.cx.tcx.hir()
-    }
-
-    fn visit_body(&mut self, b: &'tcx Body<'tcx>) {
         let is_async_block = matches!(
-            b.coroutine_kind,
-            Some(rustc_hir::CoroutineKind::Desugared(
-                rustc_hir::CoroutineDesugaring::Async,
-                _
-            ))
+            ex.kind,
+            ExprKind::Closure(rustc_hir::Closure {
+                kind: rustc_hir::ClosureKind::Coroutine(rustc_hir::CoroutineKind::Desugared(
+                    rustc_hir::CoroutineDesugaring::Async,
+                    _
+                )),
+                ..
+            })
         );
 
         if is_async_block {
             self.async_depth += 1;
         }
 
-        walk_body(self, b);
+        walk_expr(self, ex);
 
         if is_async_block {
             self.async_depth -= 1;
         }
     }
+
+    fn nested_visit_map(&mut self) -> Self::Map {
+        self.cx.tcx.hir()
+    }
 }
 
 impl<'tcx> LateLintPass<'tcx> for UnusedAsync {

clippy_lints/src/utils/author.rs

@@ -7,7 +7,8 @@ use rustc_ast::LitIntType;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
 use rustc_hir::{
-    ArrayLen, BindingAnnotation, CaptureBy, Closure, ExprKind, FnRetTy, HirId, Lit, PatKind, QPath, StmtKind, TyKind,
+    ArrayLen, BindingAnnotation, CaptureBy, Closure, ClosureKind, CoroutineKind, ExprKind, FnRetTy, HirId, Lit,
+    PatKind, QPath, StmtKind, TyKind,
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::declare_lint_pass;
@@ -476,7 +477,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
                 capture_clause,
                 fn_decl,
                 body: body_id,
-                movability,
+                kind,
                 ..
             }) => {
                 let capture_clause = match capture_clause {
@@ -484,7 +485,17 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
                     CaptureBy::Ref => "Ref",
                 };
 
-                let movability = OptionPat::new(movability.map(|m| format!("Movability::{m:?}")));
+                let closure_kind = match kind {
+                    ClosureKind::Closure => "ClosureKind::Closure".to_string(),
+                    ClosureKind::Coroutine(coroutine_kind) => match coroutine_kind {
+                        CoroutineKind::Desugared(desugaring, source) => format!(
+                            "ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::{desugaring:?}, CoroutineSource::{source:?}))"
+                        ),
+                        CoroutineKind::Coroutine(movability) => {
+                            format!("ClosureKind::Coroutine(CoroutineKind::Coroutine(Movability::{movability:?})")
+                        },
+                    },
+                };
 
                 let ret_ty = match fn_decl.output {
                     FnRetTy::DefaultReturn(_) => "FnRetTy::DefaultReturn(_)",
@@ -492,7 +503,9 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
                 };
 
                 bind!(self, fn_decl, body_id);
-                kind!("Closure(CaptureBy::{capture_clause}, {fn_decl}, {body_id}, _, {movability})");
+                kind!(
+                    "Closure {{ capture_clause: CaptureBy::{capture_clause}, fn_decl: {fn_decl}, body: {body_id}, closure_kind: {closure_kind}, .. }}"
+                );
                 chain!(self, "let {ret_ty} = {fn_decl}.output");
                 self.body(body_id);
             },

tests/ui/author/blocks.stdout

@@ -40,10 +40,10 @@ if let ExprKind::Block(block, None) = expr.kind
 {
     // report your lint here
 }
-if let ExprKind::Closure(CaptureBy::Value { .. }, fn_decl, body_id, _, None) = expr.kind
+if let ExprKind::Closure { capture_clause: CaptureBy::Value { .. }, fn_decl: fn_decl, body: body_id, closure_kind: ClosureKind::Closure, .. } = expr.kind
     && let FnRetTy::DefaultReturn(_) = fn_decl.output
     && expr1 = &cx.tcx.hir().body(body_id).value
-    && let ExprKind::Closure(CaptureBy::Value { .. }, fn_decl1, body_id1, _, Some(Movability::Static)) = expr1.kind
+    && let ExprKind::Closure { capture_clause: CaptureBy::Value { .. }, fn_decl: fn_decl1, body: body_id1, closure_kind: ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Closure)), .. } = expr1.kind
     && let FnRetTy::DefaultReturn(_) = fn_decl1.output
     && expr2 = &cx.tcx.hir().body(body_id1).value
     && let ExprKind::Block(block, None) = expr2.kind

tests/ui/author/macro_in_closure.stdout

@@ -1,6 +1,6 @@
 if let StmtKind::Local(local) = stmt.kind
     && let Some(init) = local.init
-    && let ExprKind::Closure(CaptureBy::Ref, fn_decl, body_id, _, None) = init.kind
+    && let ExprKind::Closure { capture_clause: CaptureBy::Ref, fn_decl: fn_decl, body: body_id, closure_kind: ClosureKind::Closure, .. } = init.kind
     && let FnRetTy::DefaultReturn(_) = fn_decl.output
     && expr = &cx.tcx.hir().body(body_id).value
     && let ExprKind::Block(block, None) = expr.kind

tests/ui/needless_question_mark.fixed

@@ -135,3 +135,7 @@ async fn async_deref_ref(s: Option<&String>) -> Option<&str> {
 async fn async_result_bad(s: TR) -> Result<usize, bool> {
     s.magic
 }
+
+async fn async_wrapped<T>(a: Option<T>) -> Option<T> {
+    { a }
+}

tests/ui/needless_question_mark.rs

@@ -135,3 +135,7 @@ async fn async_deref_ref(s: Option<&String>) -> Option<&str> {
 async fn async_result_bad(s: TR) -> Result<usize, bool> {
     Ok(s.magic?)
 }
+
+async fn async_wrapped<T>(a: Option<T>) -> Option<T> {
+    { Some(a?) }
+}

tests/ui/needless_question_mark.stderr

@@ -90,5 +90,11 @@ error: question mark operator is useless here
 LL |     Ok(s.magic?)
    |     ^^^^^^^^^^^^ help: try removing question mark and `Ok()`: `s.magic`
 
-error: aborting due to 14 previous errors
+error: question mark operator is useless here
+  --> $DIR/needless_question_mark.rs:140:7
+   |
+LL |     { Some(a?) }
+   |       ^^^^^^^^ help: try removing question mark and `Some()`: `a`
+
+error: aborting due to 15 previous errors