From dcc2b5c5203aaa473f5d91a4a15dd12032c892db Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Tue, 26 Mar 2024 17:16:01 +0000 Subject: [PATCH] Fix `FormatArgs` storage when `-Zthreads` > 1 --- clippy_lints/src/explicit_write.rs | 20 +++-- clippy_lints/src/format.rs | 19 ++++- clippy_lints/src/format_args.rs | 13 ++-- clippy_lints/src/format_impl.rs | 10 ++- clippy_lints/src/lib.rs | 41 +++++++--- clippy_lints/src/methods/expect_fun_call.rs | 7 +- clippy_lints/src/methods/mod.rs | 15 +++- .../src/utils/format_args_collector.rs | 26 ++++--- clippy_lints/src/write.rs | 14 ++-- clippy_utils/src/macros.rs | 78 +++++++++---------- 10 files changed, 149 insertions(+), 94 deletions(-) diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs index 2e9bec6a7b08..737c66664e8d 100644 --- a/clippy_lints/src/explicit_write.rs +++ b/clippy_lints/src/explicit_write.rs @@ -1,12 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::macros::{find_format_args, format_args_inputs_span}; +use clippy_utils::macros::{format_args_inputs_span, FormatArgsStorage}; use clippy_utils::source::snippet_with_applicability; use clippy_utils::{is_expn_of, path_def_id}; use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{BindingAnnotation, Block, BlockCheckMode, Expr, ExprKind, Node, PatKind, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::declare_lint_pass; +use rustc_session::impl_lint_pass; use rustc_span::{sym, ExpnId}; declare_clippy_lint! { @@ -38,7 +38,17 @@ declare_clippy_lint! { "using the `write!()` family of functions instead of the `print!()` family of functions, when using the latter would work" } -declare_lint_pass!(ExplicitWrite => [EXPLICIT_WRITE]); +pub struct ExplicitWrite { + format_args: FormatArgsStorage, +} + +impl ExplicitWrite { + pub fn new(format_args: FormatArgsStorage) -> Self { + Self { format_args } + } +} + +impl_lint_pass!(ExplicitWrite => [EXPLICIT_WRITE]); impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { @@ -57,7 +67,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { Some(sym::io_stderr) => ("stderr", "e"), _ => return, }; - let Some(format_args) = find_format_args(cx, write_arg, ExpnId::root()) else { + let Some(format_args) = self.format_args.get(cx, write_arg, ExpnId::root()) else { return; }; @@ -83,7 +93,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { }; let mut applicability = Applicability::MachineApplicable; let inputs_snippet = - snippet_with_applicability(cx, format_args_inputs_span(&format_args), "..", &mut applicability); + snippet_with_applicability(cx, format_args_inputs_span(format_args), "..", &mut applicability); span_lint_and_sugg( cx, EXPLICIT_WRITE, diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 8a0cd155d211..0b248f784b76 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::macros::{find_format_arg_expr, find_format_args, root_macro_call_first_node}; +use clippy_utils::macros::{find_format_arg_expr, root_macro_call_first_node, FormatArgsStorage}; use clippy_utils::source::{snippet_opt, snippet_with_context}; use clippy_utils::sugg::Sugg; use rustc_ast::{FormatArgsPiece, FormatOptions, FormatTrait}; @@ -7,7 +7,7 @@ use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; -use rustc_session::declare_lint_pass; +use rustc_session::impl_lint_pass; use rustc_span::{sym, Span}; declare_clippy_lint! { @@ -39,13 +39,24 @@ declare_clippy_lint! { "useless use of `format!`" } -declare_lint_pass!(UselessFormat => [USELESS_FORMAT]); +#[allow(clippy::module_name_repetitions)] +pub struct UselessFormat { + format_args: FormatArgsStorage, +} + +impl UselessFormat { + pub fn new(format_args: FormatArgsStorage) -> Self { + Self { format_args } + } +} + +impl_lint_pass!(UselessFormat => [USELESS_FORMAT]); impl<'tcx> LateLintPass<'tcx> for UselessFormat { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let Some(macro_call) = root_macro_call_first_node(cx, expr) && cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id) - && let Some(format_args) = find_format_args(cx, expr, macro_call.expn) + && let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn) { let mut applicability = Applicability::MachineApplicable; let call_site = macro_call.span; diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 61f550ce0beb..91a2abfebdcf 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -3,8 +3,8 @@ use clippy_config::msrvs::{self, Msrv}; 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, + find_format_arg_expr, format_arg_removal_span, format_placeholder_format_span, is_assert_macro, is_format_macro, + is_panic, root_macro_call, root_macro_call_first_node, FormatArgsStorage, FormatParamUsage, MacroCall, }; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{implements_trait, is_type_lang_item}; @@ -167,15 +167,18 @@ impl_lint_pass!(FormatArgs => [ UNUSED_FORMAT_SPECS, ]); +#[allow(clippy::struct_field_names)] pub struct FormatArgs { + format_args: FormatArgsStorage, msrv: Msrv, ignore_mixed: bool, } impl FormatArgs { #[must_use] - pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self { + pub fn new(format_args: FormatArgsStorage, msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self { Self { + format_args, msrv, ignore_mixed: allow_mixed_uninlined_format_args, } @@ -186,13 +189,13 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if let Some(macro_call) = root_macro_call_first_node(cx, expr) && is_format_macro(cx, macro_call.def_id) - && let Some(format_args) = find_format_args(cx, expr, macro_call.expn) + && let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn) { let linter = FormatArgsExpr { cx, expr, macro_call: ¯o_call, - format_args: &format_args, + format_args, ignore_mixed: self.ignore_mixed, }; diff --git a/clippy_lints/src/format_impl.rs b/clippy_lints/src/format_impl.rs index 93517076cda0..0a106a7a9e21 100644 --- a/clippy_lints/src/format_impl.rs +++ b/clippy_lints/src/format_impl.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; -use clippy_utils::macros::{find_format_arg_expr, find_format_args, is_format_macro, root_macro_call_first_node}; +use clippy_utils::macros::{find_format_arg_expr, is_format_macro, root_macro_call_first_node, FormatArgsStorage}; use clippy_utils::{get_parent_as_impl, is_diag_trait_item, path_to_local, peel_ref_operators}; use rustc_ast::{FormatArgsPiece, FormatTrait}; use rustc_errors::Applicability; @@ -99,13 +99,15 @@ struct FormatTraitNames { #[derive(Default)] pub struct FormatImpl { + format_args: FormatArgsStorage, // Whether we are inside Display or Debug trait impl - None for neither format_trait_impl: Option, } impl FormatImpl { - pub fn new() -> Self { + pub fn new(format_args: FormatArgsStorage) -> Self { Self { + format_args, format_trait_impl: None, } } @@ -129,6 +131,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl { if let Some(format_trait_impl) = self.format_trait_impl { let linter = FormatImplExpr { cx, + format_args: &self.format_args, expr, format_trait_impl, }; @@ -141,6 +144,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl { struct FormatImplExpr<'a, 'tcx> { cx: &'a LateContext<'tcx>, + format_args: &'a FormatArgsStorage, expr: &'tcx Expr<'tcx>, format_trait_impl: FormatTraitNames, } @@ -175,7 +179,7 @@ impl<'a, 'tcx> FormatImplExpr<'a, 'tcx> { if let Some(outer_macro) = root_macro_call_first_node(self.cx, self.expr) && let macro_def_id = outer_macro.def_id && is_format_macro(self.cx, macro_def_id) - && let Some(format_args) = find_format_args(self.cx, self.expr, outer_macro.expn) + && let Some(format_args) = self.format_args.get(self.cx, self.expr, outer_macro.expn) { for piece in &format_args.template { if let FormatArgsPiece::Placeholder(placeholder) = piece diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 169e51e2cb93..90d262299dab 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -60,11 +60,6 @@ extern crate clippy_utils; #[macro_use] extern crate declare_clippy_lint; -use std::collections::BTreeMap; - -use rustc_data_structures::fx::FxHashSet; -use rustc_lint::{Lint, LintId}; - #[cfg(feature = "internal")] pub mod deprecated_lints; #[cfg_attr(feature = "internal", allow(clippy::missing_clippy_version_attribute))] @@ -383,6 +378,10 @@ mod zero_sized_map_values; // end lints modules, do not remove this comment, it’s used in `update_lints` use clippy_config::{get_configuration_metadata, Conf}; +use clippy_utils::macros::FormatArgsStorage; +use rustc_data_structures::fx::FxHashSet; +use rustc_lint::{Lint, LintId}; +use std::collections::BTreeMap; /// Register all pre expansion lints /// @@ -652,7 +651,13 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { .collect(), )) }); - store.register_early_pass(|| Box::::default()); + let format_args_storage = FormatArgsStorage::default(); + let format_args = format_args_storage.clone(); + store.register_early_pass(move || { + Box::new(utils::format_args_collector::FormatArgsCollector::new( + format_args.clone(), + )) + }); store.register_late_pass(|_| Box::new(utils::dump_hir::DumpHir)); store.register_late_pass(|_| Box::new(utils::author::Author)); store.register_late_pass(move |_| { @@ -694,6 +699,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(non_octal_unix_permissions::NonOctalUnixPermissions)); store.register_early_pass(|| Box::new(unnecessary_self_imports::UnnecessarySelfImports)); store.register_late_pass(move |_| Box::new(approx_const::ApproxConstant::new(msrv()))); + let format_args = format_args_storage.clone(); store.register_late_pass(move |_| { Box::new(methods::Methods::new( avoid_breaking_exported_api, @@ -701,6 +707,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { allow_expect_in_tests, allow_unwrap_in_tests, allowed_dotfiles.clone(), + format_args.clone(), )) }); store.register_late_pass(move |_| Box::new(matches::Matches::new(msrv()))); @@ -764,7 +771,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::::default()); store.register_late_pass(move |_| Box::new(copies::CopyAndPaste::new(ignore_interior_mutability.clone()))); store.register_late_pass(|_| Box::new(copy_iterator::CopyIterator)); - store.register_late_pass(|_| Box::new(format::UselessFormat)); + let format_args = format_args_storage.clone(); + store.register_late_pass(move |_| Box::new(format::UselessFormat::new(format_args.clone()))); store.register_late_pass(|_| Box::new(swap::Swap)); store.register_late_pass(|_| Box::new(overflow_check_conditional::OverflowCheckConditional)); store.register_late_pass(|_| Box::::default()); @@ -788,7 +796,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(partialeq_ne_impl::PartialEqNeImpl)); store.register_late_pass(|_| Box::new(unused_io_amount::UnusedIoAmount)); store.register_late_pass(move |_| Box::new(large_enum_variant::LargeEnumVariant::new(enum_variant_size_threshold))); - store.register_late_pass(|_| Box::new(explicit_write::ExplicitWrite)); + let format_args = format_args_storage.clone(); + store.register_late_pass(move |_| Box::new(explicit_write::ExplicitWrite::new(format_args.clone()))); store.register_late_pass(|_| Box::new(needless_pass_by_value::NeedlessPassByValue)); store.register_late_pass(move |tcx| { Box::new(pass_by_ref_or_value::PassByRefOrValue::new( @@ -830,7 +839,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| Box::new(mut_key::MutableKeyType::new(ignore_interior_mutability.clone()))); store.register_early_pass(|| Box::new(reference::DerefAddrOf)); store.register_early_pass(|| Box::new(double_parens::DoubleParens)); - store.register_late_pass(|_| Box::new(format_impl::FormatImpl::new())); + let format_args = format_args_storage.clone(); + store.register_late_pass(move |_| Box::new(format_impl::FormatImpl::new(format_args.clone()))); store.register_early_pass(|| Box::new(unsafe_removed_from_name::UnsafeNameRemoval)); store.register_early_pass(|| Box::new(else_if_without_else::ElseIfWithoutElse)); store.register_early_pass(|| Box::new(int_plus_one::IntPlusOne)); @@ -955,8 +965,14 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { accept_comment_above_attributes, )) }); - store - .register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv(), allow_mixed_uninlined_format_args))); + let format_args = format_args_storage.clone(); + store.register_late_pass(move |_| { + Box::new(format_args::FormatArgs::new( + format_args.clone(), + msrv(), + allow_mixed_uninlined_format_args, + )) + }); store.register_late_pass(|_| Box::new(trailing_empty_array::TrailingEmptyArray)); store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes)); store.register_late_pass(|_| Box::new(needless_late_init::NeedlessLateInit)); @@ -967,7 +983,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(default_union_representation::DefaultUnionRepresentation)); store.register_late_pass(|_| Box::::default()); store.register_late_pass(move |_| Box::new(dbg_macro::DbgMacro::new(allow_dbg_in_tests))); - store.register_late_pass(move |_| Box::new(write::Write::new(allow_print_in_tests))); + let format_args = format_args_storage.clone(); + store.register_late_pass(move |_| Box::new(write::Write::new(format_args.clone(), allow_print_in_tests))); store.register_late_pass(move |_| { Box::new(cargo::Cargo { ignore_publish: cargo_ignore_publish, diff --git a/clippy_lints/src/methods/expect_fun_call.rs b/clippy_lints/src/methods/expect_fun_call.rs index 4d8fb217f7f5..f48a068e8c47 100644 --- a/clippy_lints/src/methods/expect_fun_call.rs +++ b/clippy_lints/src/methods/expect_fun_call.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::macros::{find_format_args, format_args_inputs_span, root_macro_call_first_node}; +use clippy_utils::macros::{format_args_inputs_span, root_macro_call_first_node, FormatArgsStorage}; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; use rustc_errors::Applicability; @@ -16,6 +16,7 @@ use super::EXPECT_FUN_CALL; #[allow(clippy::too_many_lines)] pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, + format_args_storage: &FormatArgsStorage, expr: &hir::Expr<'_>, method_span: Span, name: &str, @@ -134,9 +135,9 @@ pub(super) fn check<'tcx>( // Special handling for `format!` as arg_root if let Some(macro_call) = root_macro_call_first_node(cx, arg_root) { if cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id) - && let Some(format_args) = find_format_args(cx, arg_root, macro_call.expn) + && let Some(format_args) = format_args_storage.get(cx, arg_root, macro_call.expn) { - let span = format_args_inputs_span(&format_args); + let span = format_args_inputs_span(format_args); let sugg = snippet_with_applicability(cx, span, "..", &mut applicability); span_lint_and_sugg( cx, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index df5dad0423cf..805ab8cd30bd 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -133,6 +133,7 @@ use bind_instead_of_map::BindInsteadOfMap; use clippy_config::msrvs::{self, Msrv}; use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; +use clippy_utils::macros::FormatArgsStorage; use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item}; use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty}; pub use path_ends_with_ext::DEFAULT_ALLOWED_DOTFILES; @@ -4074,12 +4075,14 @@ declare_clippy_lint! { suspicious, "is_empty() called on strings known at compile time" } + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, allow_expect_in_tests: bool, allow_unwrap_in_tests: bool, allowed_dotfiles: FxHashSet, + format_args: FormatArgsStorage, } impl Methods { @@ -4090,6 +4093,7 @@ impl Methods { allow_expect_in_tests: bool, allow_unwrap_in_tests: bool, mut allowed_dotfiles: FxHashSet, + format_args: FormatArgsStorage, ) -> Self { allowed_dotfiles.extend(DEFAULT_ALLOWED_DOTFILES.iter().map(ToString::to_string)); @@ -4099,6 +4103,7 @@ impl Methods { allow_expect_in_tests, allow_unwrap_in_tests, allowed_dotfiles, + format_args, } } } @@ -4268,7 +4273,15 @@ impl<'tcx> LateLintPass<'tcx> for Methods { ExprKind::MethodCall(method_call, receiver, args, _) => { let method_span = method_call.ident.span; or_fun_call::check(cx, expr, method_span, method_call.ident.as_str(), receiver, args); - expect_fun_call::check(cx, expr, method_span, method_call.ident.as_str(), receiver, args); + expect_fun_call::check( + cx, + &self.format_args, + expr, + method_span, + method_call.ident.as_str(), + receiver, + args, + ); clone_on_copy::check(cx, expr, method_call.ident.name, receiver, args); clone_on_ref_ptr::check(cx, expr, method_call.ident.name, receiver, args); inefficient_to_string::check(cx, expr, method_call.ident.name, receiver, args); diff --git a/clippy_lints/src/utils/format_args_collector.rs b/clippy_lints/src/utils/format_args_collector.rs index 58e66c9f9b95..4f8d2aff5bca 100644 --- a/clippy_lints/src/utils/format_args_collector.rs +++ b/clippy_lints/src/utils/format_args_collector.rs @@ -1,4 +1,4 @@ -use clippy_utils::macros::AST_FORMAT_ARGS; +use clippy_utils::macros::FormatArgsStorage; use clippy_utils::source::snippet_opt; use itertools::Itertools; use rustc_ast::{Crate, Expr, ExprKind, FormatArgs}; @@ -9,13 +9,19 @@ use rustc_session::impl_lint_pass; use rustc_span::{hygiene, Span}; use std::iter::once; use std::mem; -use std::rc::Rc; -/// Collects [`rustc_ast::FormatArgs`] so that future late passes can call -/// [`clippy_utils::macros::find_format_args`] -#[derive(Default)] pub struct FormatArgsCollector { - format_args: FxHashMap>, + format_args: FxHashMap, + storage: FormatArgsStorage, +} + +impl FormatArgsCollector { + pub fn new(storage: FormatArgsStorage) -> Self { + Self { + format_args: FxHashMap::default(), + storage, + } + } } impl_lint_pass!(FormatArgsCollector => []); @@ -27,16 +33,12 @@ impl EarlyLintPass for FormatArgsCollector { return; } - self.format_args - .insert(expr.span.with_parent(None), Rc::new((**args).clone())); + self.format_args.insert(expr.span.with_parent(None), (**args).clone()); } } fn check_crate_post(&mut self, _: &EarlyContext<'_>, _: &Crate) { - AST_FORMAT_ARGS.with(|ast_format_args| { - let result = ast_format_args.set(mem::take(&mut self.format_args)); - debug_assert!(result.is_ok()); - }); + self.storage.set(mem::take(&mut self.format_args)); } } diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index be16d2e5cc30..c185fe987845 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; -use clippy_utils::macros::{find_format_args, format_arg_removal_span, root_macro_call_first_node, MacroCall}; +use clippy_utils::macros::{format_arg_removal_span, root_macro_call_first_node, FormatArgsStorage, MacroCall}; use clippy_utils::source::{expand_past_previous_comma, snippet_opt}; use clippy_utils::{is_in_cfg_test, is_in_test_function}; use rustc_ast::token::LitKind; @@ -236,13 +236,15 @@ declare_clippy_lint! { #[derive(Default)] pub struct Write { + format_args: FormatArgsStorage, in_debug_impl: bool, allow_print_in_tests: bool, } impl Write { - pub fn new(allow_print_in_tests: bool) -> Self { + pub fn new(format_args: FormatArgsStorage, allow_print_in_tests: bool) -> Self { Self { + format_args, allow_print_in_tests, ..Default::default() } @@ -307,7 +309,7 @@ impl<'tcx> LateLintPass<'tcx> for Write { _ => return, } - if let Some(format_args) = find_format_args(cx, expr, macro_call.expn) { + if let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn) { // ignore `writeln!(w)` and `write!(v, some_macro!())` if format_args.span.from_expansion() { return; @@ -315,15 +317,15 @@ impl<'tcx> LateLintPass<'tcx> for Write { match diag_name { sym::print_macro | sym::eprint_macro | sym::write_macro => { - check_newline(cx, &format_args, ¯o_call, name); + check_newline(cx, format_args, ¯o_call, name); }, sym::println_macro | sym::eprintln_macro | sym::writeln_macro => { - check_empty_string(cx, &format_args, ¯o_call, name); + check_empty_string(cx, format_args, ¯o_call, name); }, _ => {}, } - check_literal(cx, &format_args, name); + check_literal(cx, format_args, name); if !self.in_debug_impl { for piece in &format_args.template { diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index f166087dc3ca..eb195cffbcea 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -5,15 +5,13 @@ use crate::visitors::{for_each_expr, Descend}; use arrayvec::ArrayVec; use rustc_ast::{FormatArgs, FormatArgument, FormatPlaceholder}; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::sync::{Lrc, OnceLock}; use rustc_hir::{self as hir, Expr, ExprKind, HirId, Node, QPath}; use rustc_lint::LateContext; use rustc_span::def_id::DefId; use rustc_span::hygiene::{self, MacroKind, SyntaxContext}; use rustc_span::{sym, BytePos, ExpnData, ExpnId, ExpnKind, Span, SpanData, Symbol}; -use std::cell::OnceCell; use std::ops::ControlFlow; -use std::rc::Rc; -use std::sync::atomic::{AtomicBool, Ordering}; const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[ sym::assert_eq_macro, @@ -378,50 +376,44 @@ fn is_assert_arg(cx: &LateContext<'_>, expr: &Expr<'_>, assert_expn: ExpnId) -> } } -thread_local! { - /// We preserve the [`FormatArgs`] structs from the early pass for use in the late pass to be - /// able to access the many features of a [`LateContext`]. - /// - /// A thread local is used because [`FormatArgs`] is `!Send` and `!Sync`, we are making an - /// assumption that the early pass that populates the map and the later late passes will all be - /// running on the same thread. - #[doc(hidden)] - pub static AST_FORMAT_ARGS: OnceCell>> = { - static CALLED: AtomicBool = AtomicBool::new(false); - debug_assert!( - !CALLED.swap(true, Ordering::SeqCst), - "incorrect assumption: `AST_FORMAT_ARGS` should only be accessed by a single thread", - ); - - OnceCell::new() - }; -} +/// Stores AST [`FormatArgs`] nodes for use in late lint passes, as they are in a desugared form in +/// the HIR +#[derive(Default, Clone)] +pub struct FormatArgsStorage(Lrc>>); -/// Returns an AST [`FormatArgs`] node if a `format_args` expansion is found as a descendant of -/// `expn_id` -pub fn find_format_args(cx: &LateContext<'_>, start: &Expr<'_>, expn_id: ExpnId) -> Option> { - let format_args_expr = for_each_expr(start, |expr| { - let ctxt = expr.span.ctxt(); - if ctxt.outer_expn().is_descendant_of(expn_id) { - if macro_backtrace(expr.span) - .map(|macro_call| cx.tcx.item_name(macro_call.def_id)) - .any(|name| matches!(name, sym::const_format_args | sym::format_args | sym::format_args_nl)) - { - ControlFlow::Break(expr) +impl FormatArgsStorage { + /// Returns an AST [`FormatArgs`] node if a `format_args` expansion is found as a descendant of + /// `expn_id` + /// + /// See also [`find_format_arg_expr`] + pub fn get(&self, cx: &LateContext<'_>, start: &Expr<'_>, expn_id: ExpnId) -> Option<&FormatArgs> { + let format_args_expr = for_each_expr(start, |expr| { + let ctxt = expr.span.ctxt(); + if ctxt.outer_expn().is_descendant_of(expn_id) { + if macro_backtrace(expr.span) + .map(|macro_call| cx.tcx.item_name(macro_call.def_id)) + .any(|name| matches!(name, sym::const_format_args | sym::format_args | sym::format_args_nl)) + { + ControlFlow::Break(expr) + } else { + ControlFlow::Continue(Descend::Yes) + } } else { - ControlFlow::Continue(Descend::Yes) + ControlFlow::Continue(Descend::No) } - } else { - ControlFlow::Continue(Descend::No) - } - })?; + })?; - AST_FORMAT_ARGS.with(|ast_format_args| { - ast_format_args - .get()? - .get(&format_args_expr.span.with_parent(None)) - .cloned() - }) + debug_assert!(self.0.get().is_some(), "`FormatArgsStorage` not yet populated"); + + self.0.get()?.get(&format_args_expr.span.with_parent(None)) + } + + /// Should only be called by `FormatArgsCollector` + pub fn set(&self, format_args: FxHashMap) { + self.0 + .set(format_args) + .expect("`FormatArgsStorage::set` should only be called once"); + } } /// Attempt to find the [`rustc_hir::Expr`] that corresponds to the [`FormatArgument`]'s value, if