diff --git a/clippy_lints/src/large_futures.rs b/clippy_lints/src/large_futures.rs index b11e89a9b566..e56ede94a61e 100644 --- a/clippy_lints/src/large_futures.rs +++ b/clippy_lints/src/large_futures.rs @@ -1,12 +1,17 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::source::snippet; use clippy_utils::ty::implements_trait; use rustc_abi::Size; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, LangItem, MatchSource}; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::intravisit::{FnKind, Visitor, walk_body, walk_expr}; +use rustc_hir::{Body, Expr, FnDecl}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::nested_filter; +use rustc_middle::ty::Ty; use rustc_session::impl_lint_pass; +use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -14,7 +19,7 @@ declare_clippy_lint! { /// /// ### Why is this bad? /// Due to the current [unideal implementation](https://github.com/rust-lang/rust/issues/69826) of `Coroutine`, - /// large size of a `Future` may cause stack overflows. + /// large size of a `Future` may cause stack overflows or be inefficient. /// /// ### Example /// ```no_run @@ -25,7 +30,7 @@ declare_clippy_lint! { /// } /// ``` /// - /// `Box::pin` the big future instead. + /// `Box::pin` the big future or the captured elements inside the future instead. /// /// ```no_run /// async fn large_future(_x: [u8; 16 * 1024]) {} @@ -54,29 +59,106 @@ impl LargeFuture { impl_lint_pass!(LargeFuture => [LARGE_FUTURES]); -impl<'tcx> LateLintPass<'tcx> for LargeFuture { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if let ExprKind::Match(scrutinee, _, MatchSource::AwaitDesugar) = expr.kind - && let ExprKind::Call(func, [arg]) = scrutinee.kind - && let ExprKind::Path(qpath) = func.kind - && cx.tcx.qpath_is_lang_item(qpath, LangItem::IntoFutureIntoFuture) +struct AsyncFnVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + future_size_threshold: u64, + /// Depth of the visitor with value `0` denoting the top-level expression. + depth: usize, + /// Whether a clippy suggestion was emitted for deeper levels of expressions. + emitted: bool, +} + +impl<'tcx> Visitor<'tcx> for AsyncFnVisitor<'_, 'tcx> { + type NestedFilter = nested_filter::OnlyBodies; + + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + // For each expression, try to find the 'deepest' occurance of a too large a future. + // Chances are that subsequent less deep expression contain that too large a future, + // and hence it is redudant to also emit a suggestion for these expressions. + // Expressions on the same depth level can however emit a suggestion too. + + self.depth += 1; + let old_emitted = self.emitted; + self.emitted = false; // Reset emitted for deeper level, but recover it later. + walk_expr(self, expr); + self.depth -= 1; + + if !self.emitted && !expr.span.from_expansion() - && let ty = cx.typeck_results().expr_ty(arg) - && let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() - && implements_trait(cx, ty, future_trait_def_id, &[]) - && let Ok(layout) = cx.tcx.layout_of(cx.typing_env().as_query_input(ty)) - && let size = layout.layout.size() - && size >= Size::from_bytes(self.future_size_threshold) + && let Some(ty) = self.cx.typeck_results().expr_ty_opt(expr) + && let Some(size) = type_is_large_future(self.cx, ty, self.future_size_threshold) { - span_lint_and_sugg( - cx, - LARGE_FUTURES, - arg.span, - format!("large future with a size of {} bytes", size.bytes()), - "consider `Box::pin` on it", - format!("Box::pin({})", snippet(cx, arg.span, "..")), - Applicability::Unspecified, - ); + if self.depth == 0 { + // Special lint for top level fn bodies. + span_lint_hir_and_then( + self.cx, + LARGE_FUTURES, + expr.hir_id, + expr.span, + format!("definition for a large future with a size of {} bytes", size.bytes()), + |db| { + db.span_help(expr.span, "consider `Box::pin` when constructing it or reducing direct ownership of the items in scope and use references instead where possible"); + }, + ); + } else { + // Expressions within a fn body. + span_lint_hir_and_then( + self.cx, + LARGE_FUTURES, + expr.hir_id, + expr.span, + format!("usage of large future with a size of {} bytes", size.bytes()), + |db| { + db.span_suggestion( + expr.span, + "consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible", + format!("Box::pin({})", snippet(self.cx, expr.span, "..")), + Applicability::Unspecified, + ); + }, + ); + } + + self.emitted = true; } + + self.emitted |= old_emitted; + } + + fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt { + self.cx.tcx + } +} + +fn type_is_large_future<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, future_size_threshold: u64) -> Option { + if let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() + && implements_trait(cx, ty, future_trait_def_id, &[]) + && let Ok(layout) = cx.tcx.layout_of(cx.typing_env().as_query_input(ty)) + && let size = layout.layout.size() + && size >= Size::from_bytes(future_size_threshold) + { + Some(size) + } else { + None + } +} + +impl<'tcx> LateLintPass<'tcx> for LargeFuture { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: FnKind<'tcx>, + _: &'tcx FnDecl<'_>, + body: &'tcx Body<'_>, + _: Span, + _: LocalDefId, + ) { + let mut visitor = AsyncFnVisitor { + cx, + depth: 0, + future_size_threshold: self.future_size_threshold, + emitted: false, + }; + walk_body(&mut visitor, body); } } diff --git a/tests/ui/large_futures.rs b/tests/ui/large_futures.rs index 2b5860583f5e..f30069335b61 100644 --- a/tests/ui/large_futures.rs +++ b/tests/ui/large_futures.rs @@ -7,6 +7,7 @@ #![warn(clippy::large_futures)] async fn big_fut(_arg: [u8; 1024 * 16]) {} +//~^ large_futures async fn wait() { let f = async { @@ -45,16 +46,17 @@ pub fn foo() -> impl std::future::Future { async {}.await; dbg!(x); } + //~^ large_futures } pub async fn lines() { async { - //~^ large_futures - let x = [0i32; 1024 * 16]; async {}.await; + println!("{:?}", x); } + //~^ large_futures .await; } @@ -62,7 +64,6 @@ pub async fn macro_expn() { macro_rules! macro_ { () => { async { - //~^ large_futures let x = [0i32; 1024 * 16]; async {}.await; println!("macro: {:?}", x); @@ -71,5 +72,6 @@ pub async fn macro_expn() { } macro_!().await } +//~^ large_futures fn main() {} diff --git a/tests/ui/large_futures.stderr b/tests/ui/large_futures.stderr index 4280c9e2af28..770d279aedee 100644 --- a/tests/ui/large_futures.stderr +++ b/tests/ui/large_futures.stderr @@ -1,89 +1,290 @@ -error: large future with a size of 16385 bytes - --> tests/ui/large_futures.rs:13:9 +error: definition for a large future with a size of 16385 bytes + --> tests/ui/large_futures.rs:9:41 | -LL | big_fut([0u8; 1024 * 16]).await; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Box::pin` on it: `Box::pin(big_fut([0u8; 1024 * 16]))` +LL | async fn big_fut(_arg: [u8; 1024 * 16]) {} + | ^^ + | +help: consider `Box::pin` when constructing it or reducing direct ownership of the items in scope and use references instead where possible + --> tests/ui/large_futures.rs:9:41 | +LL | async fn big_fut(_arg: [u8; 1024 * 16]) {} + | ^^ = note: `-D clippy::large-futures` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::large_futures)]` -error: large future with a size of 16386 bytes - --> tests/ui/large_futures.rs:16:5 +error: usage of large future with a size of 16385 bytes + --> tests/ui/large_futures.rs:14:9 + | +LL | big_fut([0u8; 1024 * 16]).await; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - big_fut([0u8; 1024 * 16]).await; +LL + Box::pin(big_fut([0u8; 1024 * 16])).await; + | + +error: usage of large future with a size of 16386 bytes + --> tests/ui/large_futures.rs:17:5 | LL | f.await - | ^ help: consider `Box::pin` on it: `Box::pin(f)` + | ^ + | +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - f.await +LL + Box::pin(f).await + | -error: large future with a size of 16387 bytes - --> tests/ui/large_futures.rs:21:9 +error: usage of large future with a size of 16385 bytes + --> tests/ui/large_futures.rs:14:9 + | +LL | big_fut([0u8; 1024 * 16]).await; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - big_fut([0u8; 1024 * 16]).await; +LL + Box::pin(big_fut([0u8; 1024 * 16])).await; + | + +error: usage of large future with a size of 16386 bytes + --> tests/ui/large_futures.rs:17:5 + | +LL | f.await + | ^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - f.await +LL + Box::pin(f).await + | + +error: usage of large future with a size of 16385 bytes + --> tests/ui/large_futures.rs:14:9 + | +LL | big_fut([0u8; 1024 * 16]).await; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - big_fut([0u8; 1024 * 16]).await; +LL + Box::pin(big_fut([0u8; 1024 * 16])).await; + | + +error: usage of large future with a size of 16387 bytes + --> tests/ui/large_futures.rs:22:9 + | +LL | wait().await; + | ^^^^^^ + | +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - wait().await; +LL + Box::pin(wait()).await; + | + +error: usage of large future with a size of 16387 bytes + --> tests/ui/large_futures.rs:28:13 + | +LL | wait().await; + | ^^^^^^ + | +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - wait().await; +LL + Box::pin(wait()).await; + | + +error: usage of large future with a size of 16387 bytes + --> tests/ui/large_futures.rs:22:9 | LL | wait().await; - | ^^^^^^ help: consider `Box::pin` on it: `Box::pin(wait())` + | ^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - wait().await; +LL + Box::pin(wait()).await; + | -error: large future with a size of 16387 bytes - --> tests/ui/large_futures.rs:27:13 +error: usage of large future with a size of 16387 bytes + --> tests/ui/large_futures.rs:28:13 | LL | wait().await; - | ^^^^^^ help: consider `Box::pin` on it: `Box::pin(wait())` + | ^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - wait().await; +LL + Box::pin(wait()).await; + | + +error: usage of large future with a size of 16385 bytes + --> tests/ui/large_futures.rs:35:15 + | +LL | let fut = big_fut([0u8; 1024 * 16]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - let fut = big_fut([0u8; 1024 * 16]); +LL + let fut = Box::pin(big_fut([0u8; 1024 * 16])); + | + +error: usage of large future with a size of 65540 bytes + --> tests/ui/large_futures.rs:36:5 + | +LL | foo().await; + | ^^^^^ + | +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - foo().await; +LL + Box::pin(foo()).await; + | + +error: usage of large future with a size of 16385 bytes + --> tests/ui/large_futures.rs:39:15 + | +LL | calls_fut(fut).await; + | ^^^ + | +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - calls_fut(fut).await; +LL + calls_fut(Box::pin(fut)).await; + | -error: large future with a size of 65540 bytes - --> tests/ui/large_futures.rs:35:5 +error: usage of large future with a size of 16385 bytes + --> tests/ui/large_futures.rs:35:15 + | +LL | let fut = big_fut([0u8; 1024 * 16]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - let fut = big_fut([0u8; 1024 * 16]); +LL + let fut = Box::pin(big_fut([0u8; 1024 * 16])); + | + +error: usage of large future with a size of 65540 bytes + --> tests/ui/large_futures.rs:36:5 | LL | foo().await; - | ^^^^^ help: consider `Box::pin` on it: `Box::pin(foo())` + | ^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - foo().await; +LL + Box::pin(foo()).await; + | -error: large future with a size of 49159 bytes - --> tests/ui/large_futures.rs:38:5 +error: usage of large future with a size of 16385 bytes + --> tests/ui/large_futures.rs:39:15 | LL | calls_fut(fut).await; - | ^^^^^^^^^^^^^^ help: consider `Box::pin` on it: `Box::pin(calls_fut(fut))` + | ^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL - calls_fut(fut).await; +LL + calls_fut(Box::pin(fut)).await; + | -error: large future with a size of 65540 bytes - --> tests/ui/large_futures.rs:51:5 +error: usage of large future with a size of 65540 bytes + --> tests/ui/large_futures.rs:44:5 | LL | / async { -LL | | -LL | | LL | | let x = [0i32; 1024 * 16]; LL | | async {}.await; -LL | | println!("{:?}", x); +LL | | dbg!(x); LL | | } | |_____^ | -help: consider `Box::pin` on it +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible | LL ~ Box::pin(async { -LL + -LL + LL + let x = [0i32; 1024 * 16]; LL + async {}.await; +LL + dbg!(x); +LL + }) + | + +error: usage of large future with a size of 65540 bytes + --> tests/ui/large_futures.rs:53:5 + | +LL | / async { +LL | | let x = [0i32; 1024 * 16]; +LL | | async {}.await; +... | +LL | | } + | |_____^ + | +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL ~ Box::pin(async { +LL + let x = [0i32; 1024 * 16]; +LL + async {}.await; +LL + LL + println!("{:?}", x); LL + }) | -error: large future with a size of 65540 bytes - --> tests/ui/large_futures.rs:64:13 +error: usage of large future with a size of 65540 bytes + --> tests/ui/large_futures.rs:53:5 + | +LL | / async { +LL | | let x = [0i32; 1024 * 16]; +LL | | async {}.await; +... | +LL | | } + | |_____^ | -LL | / async { -LL | | -LL | | let x = [0i32; 1024 * 16]; -LL | | async {}.await; -LL | | println!("macro: {:?}", x); -LL | | } - | |_____________^ -... -LL | macro_!().await - | --------- in this macro invocation + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +help: consider `Box::pin` on it or reducing direct ownership of the items in scope and use references instead where possible + | +LL ~ Box::pin(async { +LL + let x = [0i32; 1024 * 16]; +LL + async {}.await; +LL + +LL + println!("{:?}", x); +LL + }) + | + +error: definition for a large future with a size of 65544 bytes + --> tests/ui/large_futures.rs:63:27 | - = note: this error originates in the macro `macro_` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider `Box::pin` on it +LL | pub async fn macro_expn() { + | ___________________________^ +LL | | macro_rules! macro_ { +LL | | () => { +LL | | async { +... | +LL | | macro_!().await +LL | | } + | |_^ | -LL ~ Box::pin(async { -LL + -LL + let x = [0i32; 1024 * 16]; -LL + async {}.await; -LL + println!("macro: {:?}", x); -LL + }) +help: consider `Box::pin` when constructing it or reducing direct ownership of the items in scope and use references instead where possible + --> tests/ui/large_futures.rs:63:27 | +LL | pub async fn macro_expn() { + | ___________________________^ +LL | | macro_rules! macro_ { +LL | | () => { +LL | | async { +... | +LL | | macro_!().await +LL | | } + | |_^ -error: aborting due to 8 previous errors +error: aborting due to 20 previous errors