diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 80db617c639..003a9995c15 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -4,7 +4,7 @@ use clippy_utils::is_diag_trait_item; use clippy_utils::macros::{ find_format_arg_expr, find_format_args, format_arg_removal_span, format_placeholder_format_span, is_assert_macro, - is_format_macro, is_panic, root_macro_call, root_macro_call_first_node, FormatParamUsage, MacroCall, + is_format_macro, is_panic, matching_root_macro_call, root_macro_call_first_node, FormatParamUsage, MacroCall, }; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{implements_trait, is_type_lang_item}; @@ -271,9 +271,7 @@ fn check_unused_format_specifier( let mut suggest_format = |spec| { let message = format!("for the {spec} to apply consider using `format!()`"); - if let Some(mac_call) = root_macro_call(arg_span) - && self.cx.tcx.is_diagnostic_item(sym::format_args_macro, mac_call.def_id) - { + if let Some(mac_call) = matching_root_macro_call(self.cx, arg_span, sym::format_args_macro) { diag.span_suggestion( self.cx.sess().source_map().span_until_char(mac_call.span, '!'), message, diff --git a/clippy_lints/src/large_stack_arrays.rs b/clippy_lints/src/large_stack_arrays.rs index afcb6745947..208d1bb6e68 100644 --- a/clippy_lints/src/large_stack_arrays.rs +++ b/clippy_lints/src/large_stack_arrays.rs @@ -1,10 +1,13 @@ -use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_from_proc_macro; +use clippy_utils::macros::macro_backtrace; use clippy_utils::source::snippet; -use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node}; +use rustc_hir::{ArrayLen, Expr, ExprKind, Item, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::{self, ConstKind}; use rustc_session::impl_lint_pass; +use rustc_span::{sym, Span}; declare_clippy_lint! { /// ### What it does @@ -25,20 +28,41 @@ pub struct LargeStackArrays { maximum_allowed_size: u128, + prev_vec_macro_callsite: Option, } impl LargeStackArrays { #[must_use] pub fn new(maximum_allowed_size: u128) -> Self { - Self { maximum_allowed_size } + Self { + maximum_allowed_size, + prev_vec_macro_callsite: None, + } + } + + /// Check if the given span of an expr is already in a `vec!` call. + fn is_from_vec_macro(&mut self, cx: &LateContext<'_>, span: Span) -> bool { + // First, we check if this is span is within the last encountered `vec!` macro's root callsite. + self.prev_vec_macro_callsite + .is_some_and(|vec_mac| vec_mac.contains(span)) + || { + // Then, we try backtracking the macro expansions, to see if there's a `vec!` macro, + // and update the `prev_vec_macro_callsite`. + let res = macro_backtrace(span).any(|mac| cx.tcx.is_diagnostic_item(sym::vec_macro, mac.def_id)); + if res { + self.prev_vec_macro_callsite = Some(span.source_callsite()); + } + res + } } } impl_lint_pass!(LargeStackArrays => [LARGE_STACK_ARRAYS]); impl<'tcx> LateLintPass<'tcx> for LargeStackArrays { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { if let ExprKind::Repeat(_, _) | ExprKind::Array(_) = expr.kind + && !self.is_from_vec_macro(cx, expr.span) && let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind() && let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind() && let Ok(element_count) = element_count.try_to_target_usize(cx.tcx) @@ -54,7 +78,7 @@ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { }) && self.maximum_allowed_size < u128::from(element_count) * u128::from(element_size) { - span_lint_and_help( + span_lint_and_then( cx, LARGE_STACK_ARRAYS, expr.span, @@ -62,12 +86,33 @@ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { "allocating a local array larger than {} bytes", self.maximum_allowed_size ), - None, - format!( - "consider allocating on the heap with `vec!{}.into_boxed_slice()`", - snippet(cx, expr.span, "[...]") - ), + |diag| { + if !might_be_expanded(cx, expr) { + diag.help(format!( + "consider allocating on the heap with `vec!{}.into_boxed_slice()`", + snippet(cx, expr.span, "[...]") + )); + } + }, ); } } } + +/// Only giving help messages if the expr does not contains macro expanded codes. +fn might_be_expanded<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool { + /// Check if the span of `ArrayLen` of a repeat expression is within the expr's span, + /// if not, meaning this repeat expr is definitely from some proc-macro. + /// + /// This is a fail-safe to a case where even the `is_from_proc_macro` is unable to determain the + /// correct result. + fn repeat_expr_might_be_expanded<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool { + let ExprKind::Repeat(_, ArrayLen::Body(anon_const)) = expr.kind else { + return false; + }; + let len_span = cx.tcx.def_span(anon_const.def_id); + !expr.span.contains(len_span) + } + + expr.span.from_expansion() || is_from_proc_macro(cx, expr) || repeat_expr_might_be_expanded(cx, expr) +} diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs index 76edbe8b755..d76b94eba23 100644 --- a/clippy_lints/src/manual_assert.rs +++ b/clippy_lints/src/manual_assert.rs @@ -1,12 +1,11 @@ use crate::rustc_lint::LintContext; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::macros::root_macro_call; +use clippy_utils::macros::{is_panic, root_macro_call}; use clippy_utils::{is_else_clause, is_parent_stmt, peel_blocks_with_stmt, span_extract_comment, sugg}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; -use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -42,7 +41,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { && !expr.span.from_expansion() && let then = peel_blocks_with_stmt(then) && let Some(macro_call) = root_macro_call(then.span) - && cx.tcx.item_name(macro_call.def_id) == sym::panic + && is_panic(cx, macro_call.def_id) && !cx.tcx.sess.source_map().is_multiline(cond.span) && let Ok(panic_snippet) = cx.sess().source_map().span_to_snippet(macro_call.span) && let Some(panic_snippet) = panic_snippet.strip_suffix(')') diff --git a/clippy_lints/src/manual_is_ascii_check.rs b/clippy_lints/src/manual_is_ascii_check.rs index 338a299740a..ed7eabd9b99 100644 --- a/clippy_lints/src/manual_is_ascii_check.rs +++ b/clippy_lints/src/manual_is_ascii_check.rs @@ -1,6 +1,6 @@ use clippy_config::msrvs::{self, Msrv}; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::macros::root_macro_call; +use clippy_utils::macros::matching_root_macro_call; use clippy_utils::sugg::Sugg; use clippy_utils::{higher, in_constant}; use rustc_ast::ast::RangeLimits; @@ -9,7 +9,6 @@ use rustc_hir::{BorrowKind, Expr, ExprKind, PatKind, RangeEnd}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; -use rustc_span::def_id::DefId; use rustc_span::{sym, Span}; declare_clippy_lint! { @@ -97,9 +96,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { return; } - if let Some(macro_call) = root_macro_call(expr.span) - && is_matches_macro(cx, macro_call.def_id) - { + if let Some(macro_call) = matching_root_macro_call(cx, expr.span, sym::matches_macro) { if let ExprKind::Match(recv, [arm, ..], _) = expr.kind { let range = check_pat(&arm.pat.kind); check_is_ascii(cx, macro_call.span, recv, &range); @@ -187,11 +184,3 @@ fn check_range(start: &Expr<'_>, end: &Expr<'_>) -> CharRange { CharRange::Otherwise } } - -fn is_matches_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool { - if let Some(name) = cx.tcx.get_diagnostic_name(macro_def_id) { - return sym::matches_macro == name; - } - - false -} diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 581e3b308c3..02a11257007 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::macros::{is_panic, root_macro_call}; +use clippy_utils::macros::{is_panic, matching_root_macro_call, root_macro_call}; use clippy_utils::source::{indent_of, reindent_multiline, snippet}; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{higher, is_trait_method, path_to_local_id, peel_blocks, SpanlessEq}; @@ -247,8 +247,7 @@ fn hir(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, filter_param_id: HirId) - } else { None } - } else if let Some(macro_call) = root_macro_call(expr.span) - && cx.tcx.get_diagnostic_name(macro_call.def_id) == Some(sym::matches_macro) + } else if matching_root_macro_call(cx, expr.span, sym::matches_macro).is_some() // we know for a fact that the wildcard pattern is the second arm && let ExprKind::Match(scrutinee, [arm, _], _) = expr.kind && path_to_local_id(scrutinee, filter_param_id) diff --git a/clippy_lints/src/repeat_vec_with_capacity.rs b/clippy_lints/src/repeat_vec_with_capacity.rs index a358881bf80..792d8fc88f0 100644 --- a/clippy_lints/src/repeat_vec_with_capacity.rs +++ b/clippy_lints/src/repeat_vec_with_capacity.rs @@ -1,7 +1,7 @@ use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::VecArgs; -use clippy_utils::macros::root_macro_call; +use clippy_utils::macros::matching_root_macro_call; use clippy_utils::source::snippet; use clippy_utils::{expr_or_init, fn_def_id, match_def_path, paths}; use rustc_errors::Applicability; @@ -65,8 +65,7 @@ fn emit_lint(cx: &LateContext<'_>, span: Span, kind: &str, note: &'static str, s /// Checks `vec![Vec::with_capacity(x); n]` fn check_vec_macro(cx: &LateContext<'_>, expr: &Expr<'_>) { - if let Some(mac_call) = root_macro_call(expr.span) - && cx.tcx.is_diagnostic_item(sym::vec_macro, mac_call.def_id) + if matching_root_macro_call(cx, expr.span, sym::vec_macro).is_some() && let Some(VecArgs::Repeat(repeat_expr, len_expr)) = VecArgs::hir(cx, expr) && fn_def_id(cx, repeat_expr).is_some_and(|did| match_def_path(cx, did, &paths::VEC_WITH_CAPACITY)) && !len_expr.span.from_expansion() diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index 8a9f02b6dcb..8a01f9d3e0a 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::macros::root_macro_call; +use clippy_utils::macros::matching_root_macro_call; use clippy_utils::sugg::Sugg; use clippy_utils::{ get_enclosing_block, is_expr_path_def_path, is_integer_literal, is_path_diagnostic_item, path_to_local, @@ -145,9 +145,7 @@ fn as_vec_initializer<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Opt // Generally don't warn if the vec initializer comes from an expansion, except for the vec! macro. // This lets us still warn on `vec![]`, while ignoring other kinds of macros that may output an // empty vec - if expr.span.from_expansion() - && root_macro_call(expr.span).map(|m| m.def_id) != cx.tcx.get_diagnostic_item(sym::vec_macro) - { + if expr.span.from_expansion() && matching_root_macro_call(cx, expr.span, sym::vec_macro).is_none() { return None; } diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index f166087dc3c..257dd76ab15 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -119,10 +119,20 @@ pub fn macro_backtrace(span: Span) -> impl Iterator { /// If the macro backtrace of `span` has a macro call at the root expansion /// (i.e. not a nested macro call), returns `Some` with the `MacroCall` +/// +/// If you only want to check whether the root macro has a specific name, +/// consider using [`matching_root_macro_call`] instead. pub fn root_macro_call(span: Span) -> Option { macro_backtrace(span).last() } +/// A combination of [`root_macro_call`] and +/// [`is_diagnostic_item`](rustc_middle::ty::TyCtxt::is_diagnostic_item) that returns a `MacroCall` +/// at the root expansion if only it matches the given name. +pub fn matching_root_macro_call(cx: &LateContext<'_>, span: Span, name: Symbol) -> Option { + root_macro_call(span).filter(|mc| cx.tcx.is_diagnostic_item(name, mc.def_id)) +} + /// Like [`root_macro_call`], but only returns `Some` if `node` is the "first node" /// produced by the macro call, as in [`first_node_in_macro`]. pub fn root_macro_call_first_node(cx: &LateContext<'_>, node: &impl HirNode) -> Option { diff --git a/tests/ui/auxiliary/proc_macros.rs b/tests/ui/auxiliary/proc_macros.rs index 3303eb14567..6e6919cd295 100644 --- a/tests/ui/auxiliary/proc_macros.rs +++ b/tests/ui/auxiliary/proc_macros.rs @@ -9,6 +9,7 @@ use proc_macro::Delimiter::{self, Brace, Parenthesis}; use proc_macro::Spacing::{self, Alone, Joint}; use proc_macro::{Group, Ident, Literal, Punct, Span, TokenStream, TokenTree as TT}; +use syn::spanned::Spanned; type Result = core::result::Result; @@ -124,6 +125,22 @@ fn write_with_span(s: Span, mut input: IntoIter, out: &mut TokenStream) -> Resul Ok(()) } +/// Takes an array repeat expression such as `[0_u32; 2]`, and return the tokens with 10 times the +/// original size, which turns to `[0_u32; 20]`. +#[proc_macro] +pub fn make_it_big(input: TokenStream) -> TokenStream { + let mut expr_repeat = syn::parse_macro_input!(input as syn::ExprRepeat); + let len_span = expr_repeat.len.span(); + if let syn::Expr::Lit(expr_lit) = &mut *expr_repeat.len { + if let syn::Lit::Int(lit_int) = &expr_lit.lit { + let orig_val = lit_int.base10_parse::().expect("not a valid length parameter"); + let new_val = orig_val.saturating_mul(10); + expr_lit.lit = syn::parse_quote_spanned!( len_span => #new_val); + } + } + quote::quote!(#expr_repeat).into() +} + /// Within the item this attribute is attached to, an `inline!` macro is available which expands the /// contained tokens as though they came from a macro expansion. /// diff --git a/tests/ui/large_stack_arrays.rs b/tests/ui/large_stack_arrays.rs index d5c4f95f8c4..6bcaf481c9f 100644 --- a/tests/ui/large_stack_arrays.rs +++ b/tests/ui/large_stack_arrays.rs @@ -1,6 +1,9 @@ +//@aux-build:proc_macros.rs #![warn(clippy::large_stack_arrays)] #![allow(clippy::large_enum_variant)] +extern crate proc_macros; + #[derive(Clone, Copy)] struct S { pub data: [u64; 32], @@ -55,3 +58,48 @@ fn main() { [(); 20_000_000], ); } + +#[allow(clippy::useless_vec)] +fn issue_12586() { + macro_rules! dummy { + ($n:expr) => { + $n + }; + // Weird rule to test help messages. + ($a:expr => $b:expr) => { + [$a, $b, $a, $b] + //~^ ERROR: allocating a local array larger than 512000 bytes + }; + ($id:ident; $n:literal) => { + dummy!(::std::vec![$id;$n]) + }; + ($($id:expr),+ $(,)?) => { + ::std::vec![$($id),*] + } + } + macro_rules! create_then_move { + ($id:ident; $n:literal) => {{ + let _x_ = [$id; $n]; + //~^ ERROR: allocating a local array larger than 512000 bytes + _x_ + }}; + } + + let x = [0u32; 50_000]; + let y = vec![x, x, x, x, x]; + let y = vec![dummy![x, x, x, x, x]]; + let y = vec![dummy![[x, x, x, x, x]]]; + let y = dummy![x, x, x, x, x]; + let y = [x, x, dummy!(x), x, x]; + //~^ ERROR: allocating a local array larger than 512000 bytes + let y = dummy![x => x]; + let y = dummy![x;5]; + let y = dummy!(vec![dummy![x, x, x, x, x]]); + let y = dummy![[x, x, x, x, x]]; + //~^ ERROR: allocating a local array larger than 512000 bytes + + let y = proc_macros::make_it_big!([x; 1]); + //~^ ERROR: allocating a local array larger than 512000 bytes + let y = vec![proc_macros::make_it_big!([x; 10])]; + let y = vec![create_then_move![x; 5]; 5]; +} diff --git a/tests/ui/large_stack_arrays.stderr b/tests/ui/large_stack_arrays.stderr index 007ca61c2de..06294ee8b8c 100644 --- a/tests/ui/large_stack_arrays.stderr +++ b/tests/ui/large_stack_arrays.stderr @@ -1,5 +1,5 @@ error: allocating a local array larger than 512000 bytes - --> tests/ui/large_stack_arrays.rs:29:14 + --> tests/ui/large_stack_arrays.rs:32:14 | LL | let _x = [build(); 3]; | ^^^^^^^^^^^^ @@ -9,7 +9,7 @@ LL | let _x = [build(); 3]; = help: to override `-D warnings` add `#[allow(clippy::large_stack_arrays)]` error: allocating a local array larger than 512000 bytes - --> tests/ui/large_stack_arrays.rs:32:14 + --> tests/ui/large_stack_arrays.rs:35:14 | LL | let _y = [build(), build(), build()]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,7 +17,7 @@ LL | let _y = [build(), build(), build()]; = help: consider allocating on the heap with `vec![build(), build(), build()].into_boxed_slice()` error: allocating a local array larger than 512000 bytes - --> tests/ui/large_stack_arrays.rs:38:9 + --> tests/ui/large_stack_arrays.rs:41:9 | LL | [0u32; 20_000_000], | ^^^^^^^^^^^^^^^^^^ @@ -25,7 +25,7 @@ LL | [0u32; 20_000_000], = help: consider allocating on the heap with `vec![0u32; 20_000_000].into_boxed_slice()` error: allocating a local array larger than 512000 bytes - --> tests/ui/large_stack_arrays.rs:40:9 + --> tests/ui/large_stack_arrays.rs:43:9 | LL | [S { data: [0; 32] }; 5000], | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -33,7 +33,7 @@ LL | [S { data: [0; 32] }; 5000], = help: consider allocating on the heap with `vec![S { data: [0; 32] }; 5000].into_boxed_slice()` error: allocating a local array larger than 512000 bytes - --> tests/ui/large_stack_arrays.rs:42:9 + --> tests/ui/large_stack_arrays.rs:45:9 | LL | [Some(""); 20_000_000], | ^^^^^^^^^^^^^^^^^^^^^^ @@ -41,7 +41,7 @@ LL | [Some(""); 20_000_000], = help: consider allocating on the heap with `vec![Some(""); 20_000_000].into_boxed_slice()` error: allocating a local array larger than 512000 bytes - --> tests/ui/large_stack_arrays.rs:44:9 + --> tests/ui/large_stack_arrays.rs:47:9 | LL | [E::T(0); 5000], | ^^^^^^^^^^^^^^^ @@ -49,12 +49,56 @@ LL | [E::T(0); 5000], = help: consider allocating on the heap with `vec![E::T(0); 5000].into_boxed_slice()` error: allocating a local array larger than 512000 bytes - --> tests/ui/large_stack_arrays.rs:46:9 + --> tests/ui/large_stack_arrays.rs:49:9 | LL | [0u8; usize::MAX], | ^^^^^^^^^^^^^^^^^ | = help: consider allocating on the heap with `vec![0u8; usize::MAX].into_boxed_slice()` -error: aborting due to 7 previous errors +error: allocating a local array larger than 512000 bytes + --> tests/ui/large_stack_arrays.rs:93:13 + | +LL | let y = [x, x, dummy!(x), x, x]; + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider allocating on the heap with `vec![x, x, dummy!(x), x, x].into_boxed_slice()` + +error: allocating a local array larger than 512000 bytes + --> tests/ui/large_stack_arrays.rs:70:13 + | +LL | [$a, $b, $a, $b] + | ^^^^^^^^^^^^^^^^ +... +LL | let y = dummy![x => x]; + | -------------- in this macro invocation + | + = note: this error originates in the macro `dummy` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: allocating a local array larger than 512000 bytes + --> tests/ui/large_stack_arrays.rs:98:20 + | +LL | let y = dummy![[x, x, x, x, x]]; + | ^^^^^^^^^^^^^^^ + | + = help: consider allocating on the heap with `vec![x, x, x, x, x].into_boxed_slice()` + +error: allocating a local array larger than 512000 bytes + --> tests/ui/large_stack_arrays.rs:101:39 + | +LL | let y = proc_macros::make_it_big!([x; 1]); + | ^^^^^^ + +error: allocating a local array larger than 512000 bytes + --> tests/ui/large_stack_arrays.rs:82:23 + | +LL | let _x_ = [$id; $n]; + | ^^^^^^^^^ +... +LL | let y = vec![create_then_move![x; 5]; 5]; + | ----------------------- in this macro invocation + | + = note: this error originates in the macro `create_then_move` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 12 previous errors