Skip to content

Commit

Permalink
Auto merge of rust-lang#12624 - J-ZhengLi:issue12586, r=xFrednet
Browse files Browse the repository at this point in the history
fix [`large_stack_arrays`] linting in `vec` macro

fixes: rust-lang#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
  • Loading branch information
bors committed Apr 27, 2024
2 parents de4fce8 + 2861729 commit c6bf954
Show file tree
Hide file tree
Showing 11 changed files with 194 additions and 48 deletions.
6 changes: 2 additions & 4 deletions clippy_lints/src/format_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
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};
Expand Down Expand Up @@ -271,9 +271,7 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
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,
Expand Down
65 changes: 55 additions & 10 deletions clippy_lints/src/large_stack_arrays.rs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -25,20 +28,41 @@ declare_clippy_lint! {

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)
Expand All @@ -54,20 +78,41 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
})
&& 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,
format!(
"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)
}
5 changes: 2 additions & 3 deletions clippy_lints/src/manual_assert.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -42,7 +41,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
&& !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(')')
Expand Down
15 changes: 2 additions & 13 deletions clippy_lints/src/manual_is_ascii_check.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -9,7 +9,6 @@ use rustc_errors::Applicability;
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! {
Expand Down Expand Up @@ -97,9 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
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);
Expand Down Expand Up @@ -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 changes: 2 additions & 3 deletions clippy_lints/src/methods/filter_map.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -247,8 +247,7 @@ impl<'tcx> OffendingFilterExpr<'tcx> {
} 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)
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/repeat_vec_with_capacity.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 2 additions & 4 deletions clippy_lints/src/slow_vector_initialization.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -145,9 +145,7 @@ impl SlowVectorInit {
// 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;
}

Expand Down
10 changes: 10 additions & 0 deletions clippy_utils/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/auxiliary/proc_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use proc_macro::token_stream::IntoIter;
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>;

Expand Down Expand Up @@ -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.
///
Expand Down
48 changes: 48 additions & 0 deletions tests/ui/large_stack_arrays.rs
Original file line number Diff line number Diff line change
@@ -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],
Expand Down Expand Up @@ -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];
}
Loading

0 comments on commit c6bf954

Please sign in to comment.