Skip to content

Commit

Permalink
Auto merge of #12567 - Alexendoo:format-args-storage, r=flip1995
Browse files Browse the repository at this point in the history
Fix `FormatArgs` storage when `-Zthreads` > 1

Fixes #11886

The initial way I thought of was a little gross so I never opened a PR for it, I thought of a nicer way today that no longer involves any `thread_local`s or `static`s

`rustc_data_strucutres::sync::{Lrc, OnceLock}` implement `DynSend` + `DynSync` so we can pass them to the lint passes that need the storage

changelog: none

r? `@flip1995`
  • Loading branch information
bors committed May 3, 2024
2 parents 60267b2 + c187bff commit 993d8ae
Show file tree
Hide file tree
Showing 10 changed files with 151 additions and 94 deletions.
20 changes: 15 additions & 5 deletions clippy_lints/src/explicit_write.rs
Original file line number Diff line number Diff line change
@@ -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::{BindingMode, 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! {
Expand Down Expand Up @@ -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<'_>) {
Expand All @@ -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;
};

Expand All @@ -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,
Expand Down
19 changes: 15 additions & 4 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
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};
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! {
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 8 additions & 5 deletions clippy_lints/src/format_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, matching_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, matching_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};
Expand Down Expand Up @@ -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,
}
Expand All @@ -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: &macro_call,
format_args: &format_args,
format_args,
ignore_mixed: self.ignore_mixed,
};

Expand Down
10 changes: 7 additions & 3 deletions clippy_lints/src/format_impl.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<FormatTraitNames>,
}

impl FormatImpl {
pub fn new() -> Self {
pub fn new(format_args: FormatArgsStorage) -> Self {
Self {
format_args,
format_trait_impl: None,
}
}
Expand All @@ -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,
};
Expand All @@ -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,
}
Expand Down Expand Up @@ -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
Expand Down
42 changes: 30 additions & 12 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down Expand Up @@ -384,6 +379,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
///
Expand Down Expand Up @@ -615,6 +614,14 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
}
}

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(),
))
});

// all the internal lints
#[cfg(feature = "internal")]
{
Expand Down Expand Up @@ -655,7 +662,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
.collect(),
))
});
store.register_early_pass(|| Box::<utils::format_args_collector::FormatArgsCollector>::default());
store.register_late_pass(|_| Box::new(utils::dump_hir::DumpHir));
store.register_late_pass(|_| Box::new(utils::author::Author));
store.register_late_pass(move |_| {
Expand Down Expand Up @@ -697,13 +703,15 @@ 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,
msrv(),
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())));
Expand Down Expand Up @@ -768,7 +776,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::<regex::Regex>::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::<new_without_default::NewWithoutDefault>::default());
Expand All @@ -792,7 +801,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(
Expand Down Expand Up @@ -834,7 +844,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));
Expand Down Expand Up @@ -960,8 +971,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));
Expand All @@ -972,7 +989,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::<only_used_in_recursion::OnlyUsedInRecursion>::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,
Expand Down
7 changes: 4 additions & 3 deletions clippy_lints/src/methods/expect_fun_call.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 14 additions & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -4087,12 +4088,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<String>,
format_args: FormatArgsStorage,
}

impl Methods {
Expand All @@ -4103,6 +4106,7 @@ impl Methods {
allow_expect_in_tests: bool,
allow_unwrap_in_tests: bool,
mut allowed_dotfiles: FxHashSet<String>,
format_args: FormatArgsStorage,
) -> Self {
allowed_dotfiles.extend(DEFAULT_ALLOWED_DOTFILES.iter().map(ToString::to_string));

Expand All @@ -4112,6 +4116,7 @@ impl Methods {
allow_expect_in_tests,
allow_unwrap_in_tests,
allowed_dotfiles,
format_args,
}
}
}
Expand Down Expand Up @@ -4281,7 +4286,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);
Expand Down
Loading

0 comments on commit 993d8ae

Please sign in to comment.