Skip to content

Commit 666e2f2

Browse files
J-ZhengLiJ-ZhengLi
committed
fix [large_stack_arrays] linting in vec macro & add matching_root_macro_call function in clippy_utils 
1 parent f5e2501 commit 666e2f2

File tree

10 files changed

+110
-38
lines changed

10 files changed

+110
-38
lines changed

‎clippy_lints/src/format_args.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
44
use clippy_utils::is_diag_trait_item;
55
use clippy_utils::macros::{
66
find_format_arg_expr, find_format_args, format_arg_removal_span, format_placeholder_format_span, is_assert_macro,
7-
is_format_macro, is_panic, root_macro_call, root_macro_call_first_node, FormatParamUsage, MacroCall,
7+
is_format_macro, is_panic, matching_root_macro_call, root_macro_call_first_node, FormatParamUsage, MacroCall,
88
};
99
use clippy_utils::source::snippet_opt;
1010
use clippy_utils::ty::{implements_trait, is_type_lang_item};
@@ -271,9 +271,7 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
271271
let mut suggest_format = |spec| {
272272
let message = format!("for the {spec} to apply consider using `format!()`");
273273

274-
if let Some(mac_call) = root_macro_call(arg_span)
275-
&& self.cx.tcx.is_diagnostic_item(sym::format_args_macro, mac_call.def_id)
276-
{
274+
if let Some(mac_call) = matching_root_macro_call(self.cx, arg_span, sym::format_args_macro) {
277275
diag.span_suggestion(
278276
self.cx.sess().source_map().span_until_char(mac_call.span, '!'),
279277
message,

‎clippy_lints/src/large_stack_arrays.rs‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
use clippy_utils::diagnostics::span_lint_and_help;
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::macros::macro_backtrace;
23
use clippy_utils::source::snippet;
34
use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node};
45
use rustc_lint::{LateContext, LateLintPass};
56
use rustc_middle::ty::layout::LayoutOf;
67
use rustc_middle::ty::{self, ConstKind};
78
use rustc_session::impl_lint_pass;
9+
use rustc_span::{sym, Span};
810

911
declare_clippy_lint! {
1012
/// ### What it does
@@ -39,6 +41,7 @@ impl_lint_pass!(LargeStackArrays => [LARGE_STACK_ARRAYS]);
3941
impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
4042
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
4143
if let ExprKind::Repeat(_, _) | ExprKind::Array(_) = expr.kind
44+
&& !is_from_vec_macro(cx, expr.span)
4245
&& let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind()
4346
&& let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind()
4447
&& let Ok(element_count) = element_count.try_to_target_usize(cx.tcx)
@@ -54,20 +57,28 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
5457
})
5558
&& self.maximum_allowed_size < u128::from(element_count) * u128::from(element_size)
5659
{
57-
span_lint_and_help(
60+
span_lint_and_then(
5861
cx,
5962
LARGE_STACK_ARRAYS,
6063
expr.span,
6164
format!(
6265
"allocating a local array larger than {} bytes",
6366
self.maximum_allowed_size
6467
),
65-
None,
66-
format!(
67-
"consider allocating on the heap with `vec!{}.into_boxed_slice()`",
68-
snippet(cx, expr.span, "[...]")
69-
),
68+
|diag| {
69+
if !expr.span.from_expansion() {
70+
diag.help(format!(
71+
"consider allocating on the heap with `vec!{}.into_boxed_slice()`",
72+
snippet(cx, expr.span, "[...]")
73+
));
74+
}
75+
},
7076
);
7177
}
7278
}
7379
}
80+
81+
/// We shouldn't lint messages if the expr is already in a `vec!` call
82+
fn is_from_vec_macro(cx: &LateContext<'_>, expr_span: Span) -> bool {
83+
macro_backtrace(expr_span).any(|mac| cx.tcx.is_diagnostic_item(sym::vec_macro, mac.def_id))
84+
}

‎clippy_lints/src/manual_assert.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
use crate::rustc_lint::LintContext;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::macros::root_macro_call;
3+
use clippy_utils::macros::{is_panic, root_macro_call};
44
use clippy_utils::{is_else_clause, is_parent_stmt, peel_blocks_with_stmt, span_extract_comment, sugg};
55
use rustc_errors::Applicability;
66
use rustc_hir::{Expr, ExprKind, UnOp};
77
use rustc_lint::{LateContext, LateLintPass};
88
use rustc_session::declare_lint_pass;
9-
use rustc_span::sym;
109

1110
declare_clippy_lint! {
1211
/// ### What it does
@@ -42,7 +41,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
4241
&& !expr.span.from_expansion()
4342
&& let then = peel_blocks_with_stmt(then)
4443
&& let Some(macro_call) = root_macro_call(then.span)
45-
&& cx.tcx.item_name(macro_call.def_id) == sym::panic
44+
&& is_panic(cx, macro_call.def_id)
4645
&& !cx.tcx.sess.source_map().is_multiline(cond.span)
4746
&& let Ok(panic_snippet) = cx.sess().source_map().span_to_snippet(macro_call.span)
4847
&& let Some(panic_snippet) = panic_snippet.strip_suffix(')')

‎clippy_lints/src/manual_is_ascii_check.rs‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_config::msrvs::{self, Msrv};
22
use clippy_utils::diagnostics::span_lint_and_sugg;
3-
use clippy_utils::macros::root_macro_call;
3+
use clippy_utils::macros::matching_root_macro_call;
44
use clippy_utils::sugg::Sugg;
55
use clippy_utils::{higher, in_constant};
66
use rustc_ast::ast::RangeLimits;
@@ -9,7 +9,6 @@ use rustc_errors::Applicability;
99
use rustc_hir::{BorrowKind, Expr, ExprKind, PatKind, RangeEnd};
1010
use rustc_lint::{LateContext, LateLintPass};
1111
use rustc_session::impl_lint_pass;
12-
use rustc_span::def_id::DefId;
1312
use rustc_span::{sym, Span};
1413

1514
declare_clippy_lint! {
@@ -97,9 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
9796
return;
9897
}
9998

100-
if let Some(macro_call) = root_macro_call(expr.span)
101-
&& is_matches_macro(cx, macro_call.def_id)
102-
{
99+
if let Some(macro_call) = matching_root_macro_call(cx, expr.span, sym::matches_macro) {
103100
if let ExprKind::Match(recv, [arm, ..], _) = expr.kind {
104101
let range = check_pat(&arm.pat.kind);
105102
check_is_ascii(cx, macro_call.span, recv, &range);
@@ -187,11 +184,3 @@ fn check_range(start: &Expr<'_>, end: &Expr<'_>) -> CharRange {
187184
CharRange::Otherwise
188185
}
189186
}
190-
191-
fn is_matches_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool {
192-
if let Some(name) = cx.tcx.get_diagnostic_name(macro_def_id) {
193-
return sym::matches_macro == name;
194-
}
195-
196-
false
197-
}

‎clippy_lints/src/methods/filter_map.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
2-
use clippy_utils::macros::{is_panic, root_macro_call};
2+
use clippy_utils::macros::{is_panic, matching_root_macro_call, root_macro_call};
33
use clippy_utils::source::{indent_of, reindent_multiline, snippet};
44
use clippy_utils::ty::is_type_diagnostic_item;
55
use clippy_utils::{higher, is_trait_method, path_to_local_id, peel_blocks, SpanlessEq};
@@ -247,8 +247,7 @@ impl<'tcx> OffendingFilterExpr<'tcx> {
247247
} else {
248248
None
249249
}
250-
} else if let Some(macro_call) = root_macro_call(expr.span)
251-
&& cx.tcx.get_diagnostic_name(macro_call.def_id) == Some(sym::matches_macro)
250+
} else if matching_root_macro_call(cx, expr.span, sym::matches_macro).is_some()
252251
// we know for a fact that the wildcard pattern is the second arm
253252
&& let ExprKind::Match(scrutinee, [arm, _], _) = expr.kind
254253
&& path_to_local_id(scrutinee, filter_param_id)

‎clippy_lints/src/repeat_vec_with_capacity.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::consts::{constant, Constant};
22
use clippy_utils::diagnostics::span_lint_and_then;
33
use clippy_utils::higher::VecArgs;
4-
use clippy_utils::macros::root_macro_call;
4+
use clippy_utils::macros::matching_root_macro_call;
55
use clippy_utils::source::snippet;
66
use clippy_utils::{expr_or_init, fn_def_id, match_def_path, paths};
77
use rustc_errors::Applicability;
@@ -65,8 +65,7 @@ fn emit_lint(cx: &LateContext<'_>, span: Span, kind: &str, note: &'static str, s
6565

6666
/// Checks `vec![Vec::with_capacity(x); n]`
6767
fn check_vec_macro(cx: &LateContext<'_>, expr: &Expr<'_>) {
68-
if let Some(mac_call) = root_macro_call(expr.span)
69-
&& cx.tcx.is_diagnostic_item(sym::vec_macro, mac_call.def_id)
68+
if matching_root_macro_call(cx, expr.span, sym::vec_macro).is_some()
7069
&& let Some(VecArgs::Repeat(repeat_expr, len_expr)) = VecArgs::hir(cx, expr)
7170
&& fn_def_id(cx, repeat_expr).is_some_and(|did| match_def_path(cx, did, &paths::VEC_WITH_CAPACITY))
7271
&& !len_expr.span.from_expansion()

‎clippy_lints/src/slow_vector_initialization.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::macros::root_macro_call;
2+
use clippy_utils::macros::matching_root_macro_call;
33
use clippy_utils::sugg::Sugg;
44
use clippy_utils::{
55
get_enclosing_block, is_expr_path_def_path, is_integer_literal, is_path_diagnostic_item, path_to_local,
@@ -145,9 +145,7 @@ impl SlowVectorInit {
145145
// Generally don't warn if the vec initializer comes from an expansion, except for the vec! macro.
146146
// This lets us still warn on `vec![]`, while ignoring other kinds of macros that may output an
147147
// empty vec
148-
if expr.span.from_expansion()
149-
&& root_macro_call(expr.span).map(|m| m.def_id) != cx.tcx.get_diagnostic_item(sym::vec_macro)
150-
{
148+
if expr.span.from_expansion() && matching_root_macro_call(cx, expr.span, sym::vec_macro).is_none() {
151149
return None;
152150
}
153151

‎clippy_utils/src/macros.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,20 @@ pub fn macro_backtrace(span: Span) -> impl Iterator<Item = MacroCall> {
119119

120120
/// If the macro backtrace of `span` has a macro call at the root expansion
121121
/// (i.e. not a nested macro call), returns `Some` with the `MacroCall`
122+
///
123+
/// If you only want to check whether the root macro has a specific name,
124+
/// consider using [`matching_root_macro_call`] instead.
122125
pub fn root_macro_call(span: Span) -> Option<MacroCall> {
123126
macro_backtrace(span).last()
124127
}
125128

129+
/// A combination of [`root_macro_call`] and
130+
/// [`is_diagnostic_item`](rustc_middle::ty::TyCtxt::is_diagnostic_item) that returns a `MacroCall`
131+
/// at the root expansion if only it matches the given name.
132+
pub fn matching_root_macro_call(cx: &LateContext<'_>, span: Span, name: Symbol) -> Option<MacroCall> {
133+
root_macro_call(span).filter(|mc| cx.tcx.is_diagnostic_item(name, mc.def_id))
134+
}
135+
126136
/// Like [`root_macro_call`], but only returns `Some` if `node` is the "first node"
127137
/// produced by the macro call, as in [`first_node_in_macro`].
128138
pub fn root_macro_call_first_node(cx: &LateContext<'_>, node: &impl HirNode) -> Option<MacroCall> {

‎tests/ui/large_stack_arrays.rs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,37 @@ fn main() {
5555
[(); 20_000_000],
5656
);
5757
}
58+
59+
#[allow(clippy::useless_vec)]
60+
fn issue_12586() {
61+
macro_rules! dummy {
62+
($n:expr) => {
63+
$n
64+
};
65+
// Weird rule to test help messages.
66+
($a:expr => $b:expr) => {
67+
[$a, $b, $a, $b]
68+
//~^ ERROR: allocating a local array larger than 512000 bytes
69+
};
70+
($id:ident; $n:literal) => {
71+
dummy!(::std::vec![$id;$n])
72+
};
73+
($($id:expr),+ $(,)?) => {
74+
::std::vec![$($id),*]
75+
}
76+
}
77+
78+
let x = [0u32; 50_000];
79+
let y = vec![x, x, x, x, x];
80+
let y = vec![dummy![x, x, x, x, x]];
81+
let y = vec![dummy![[x, x, x, x, x]]];
82+
//~^ ERROR: allocating a local array larger than 512000 bytes
83+
let y = dummy![x, x, x, x, x];
84+
let y = [x, x, dummy!(x), x, x];
85+
//~^ ERROR: allocating a local array larger than 512000 bytes
86+
let y = dummy![x => x];
87+
let y = dummy![x;5];
88+
let y = dummy!(vec![dummy![x, x, x, x, x]]);
89+
let y = dummy![[x, x, x, x, x]];
90+
//~^ ERROR: allocating a local array larger than 512000 bytes
91+
}

‎tests/ui/large_stack_arrays.stderr‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,40 @@ LL | [0u8; usize::MAX],
5656
|
5757
= help: consider allocating on the heap with `vec![0u8; usize::MAX].into_boxed_slice()`
5858

59-
error: aborting due to 7 previous errors
59+
error: allocating a local array larger than 512000 bytes
60+
--> tests/ui/large_stack_arrays.rs:81:25
61+
|
62+
LL | let y = vec![dummy![[x, x, x, x, x]]];
63+
| ^^^^^^^^^^^^^^^
64+
|
65+
= help: consider allocating on the heap with `vec![x, x, x, x, x].into_boxed_slice()`
66+
67+
error: allocating a local array larger than 512000 bytes
68+
--> tests/ui/large_stack_arrays.rs:84:13
69+
|
70+
LL | let y = [x, x, dummy!(x), x, x];
71+
| ^^^^^^^^^^^^^^^^^^^^^^^
72+
|
73+
= help: consider allocating on the heap with `vec![x, x, dummy!(x), x, x].into_boxed_slice()`
74+
75+
error: allocating a local array larger than 512000 bytes
76+
--> tests/ui/large_stack_arrays.rs:67:13
77+
|
78+
LL | [$a, $b, $a, $b]
79+
| ^^^^^^^^^^^^^^^^
80+
...
81+
LL | let y = dummy![x => x];
82+
| -------------- in this macro invocation
83+
|
84+
= note: this error originates in the macro `dummy` (in Nightly builds, run with -Z macro-backtrace for more info)
85+
86+
error: allocating a local array larger than 512000 bytes
87+
--> tests/ui/large_stack_arrays.rs:89:20
88+
|
89+
LL | let y = dummy![[x, x, x, x, x]];
90+
| ^^^^^^^^^^^^^^^
91+
|
92+
= help: consider allocating on the heap with `vec![x, x, x, x, x].into_boxed_slice()`
93+
94+
error: aborting due to 11 previous errors
6095

0 commit comments

Comments
 (0)