mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #12624 - J-ZhengLi:issue12586, r=xFrednet

Browse Source 
fix [`large_stack_arrays`] linting in `vec` macro

fixes: #12586

this PR also adds a wrapper function `matching_root_macro_call` to `clippy_utils::macros`, considering how often that same pattern appears in the codebase.

(I'm always very indecisive towards naming, so, if anyone have better idea of how that function should be named, feel free to suggest it)

---

changelog: fix [`large_stack_arrays`] linting in `vec` macro; add `matching_root_macro_call` to clippy_utils
This commit is contained in:
bors 2024-04-27 09:30:20 +00:00
commit c6bf9548d5
11 changed files with 194 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
clippy_lints/src/format_args.rs
View File

@ -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,

61
clippy_lints/src/large_stack_arrays.rs
View File

@ -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<Span>,
}
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!(
|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)
}

5
clippy_lints/src/manual_assert.rs
View File

@ -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(')')

15
clippy_lints/src/manual_is_ascii_check.rs
View File

@ -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
}

5
clippy_lints/src/methods/filter_map.rs
View File

@ -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)

5
clippy_lints/src/repeat_vec_with_capacity.rs
View File

@ -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()

6
clippy_lints/src/slow_vector_initialization.rs
View File

@ -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;
}

10
clippy_utils/src/macros.rs
View File

@ -119,10 +119,20 @@ pub fn macro_backtrace(span: Span) -> impl Iterator<Item = MacroCall> {
/// 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<MacroCall> {
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<MacroCall> {
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<MacroCall> {

17
tests/ui/auxiliary/proc_macros.rs
View File

@ -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<T> = core::result::Result<T, TokenStream>;
@ -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::<usize>().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.
///

48
tests/ui/large_stack_arrays.rs
View File

@ -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];
}

60
tests/ui/large_stack_arrays.stderr
View File

@ -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