From ae29890ab6a572fca53f693cccdf6654dd07f7d5 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 26 Apr 2022 15:00:36 -0700 Subject: [PATCH 1/3] Add test of temporaries inside format_args of core/std macros --- src/test/ui/macros/format-args-temporaries.rs | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 src/test/ui/macros/format-args-temporaries.rs diff --git a/src/test/ui/macros/format-args-temporaries.rs b/src/test/ui/macros/format-args-temporaries.rs new file mode 100644 index 0000000000000..ddd4c9754bfa4 --- /dev/null +++ b/src/test/ui/macros/format-args-temporaries.rs @@ -0,0 +1,70 @@ +// check-pass + +use std::fmt::{self, Display}; + +struct Mutex; + +impl Mutex { + fn lock(&self) -> MutexGuard { + MutexGuard(self) + } +} + +struct MutexGuard<'a>(&'a Mutex); + +impl<'a> Drop for MutexGuard<'a> { + fn drop(&mut self) { + // Empty but this is a necessary part of the repro. Otherwise borrow + // checker is fine with 'a dangling at the time that MutexGuard goes out + // of scope. + } +} + +impl<'a> MutexGuard<'a> { + fn write_fmt(&self, _args: fmt::Arguments) {} +} + +impl<'a> Display for MutexGuard<'a> { + fn fmt(&self, _formatter: &mut fmt::Formatter) -> fmt::Result { + Ok(()) + } +} + +fn main() { + let _write = { + let out = Mutex; + let mutex = Mutex; + write!(out.lock(), "{}", mutex.lock()) /* no semicolon */ + }; + + let _writeln = { + let out = Mutex; + let mutex = Mutex; + writeln!(out.lock(), "{}", mutex.lock()) /* no semicolon */ + }; + + let _print = { + let mutex = Mutex; + print!("{}", mutex.lock()) /* no semicolon */ + }; + + let _println = { + let mutex = Mutex; + println!("{}", mutex.lock()) /* no semicolon */ + }; + + let _eprint = { + let mutex = Mutex; + eprint!("{}", mutex.lock()) /* no semicolon */ + }; + + let _eprintln = { + let mutex = Mutex; + eprintln!("{}", mutex.lock()) /* no semicolon */ + }; + + let _panic = { + let mutex = Mutex; + panic!("{}", mutex.lock()) /* no semicolon */ + }; +} From 0502496b1e5c53fe0f5c7e9a6bb67c7cc6777d33 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 26 Apr 2022 15:16:23 -0700 Subject: [PATCH 2/3] Make write/print macros eagerly drop temporaries --- library/core/src/macros/mod.rs | 14 ++++++++------ library/std/src/macros.rs | 12 ++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index a231f533d190b..dbc3d2923ed59 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -496,9 +496,10 @@ macro_rules! r#try { #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "write_macro")] macro_rules! write { - ($dst:expr, $($arg:tt)*) => { - $dst.write_fmt($crate::format_args!($($arg)*)) - }; + ($dst:expr, $($arg:tt)*) => {{ + let result = $dst.write_fmt($crate::format_args!($($arg)*)); + result + }}; } /// Write formatted data into a buffer, with a newline appended. @@ -553,9 +554,10 @@ macro_rules! writeln { ($dst:expr $(,)?) => { $crate::write!($dst, "\n") }; - ($dst:expr, $($arg:tt)*) => { - $dst.write_fmt($crate::format_args_nl!($($arg)*)) - }; + ($dst:expr, $($arg:tt)*) => {{ + let result = $dst.write_fmt($crate::format_args_nl!($($arg)*)); + result + }}; } /// Indicates unreachable code. diff --git a/library/std/src/macros.rs b/library/std/src/macros.rs index c7348951511c4..0cb21ef53b1eb 100644 --- a/library/std/src/macros.rs +++ b/library/std/src/macros.rs @@ -62,9 +62,9 @@ macro_rules! panic { #[cfg_attr(not(test), rustc_diagnostic_item = "print_macro")] #[allow_internal_unstable(print_internals)] macro_rules! print { - ($($arg:tt)*) => { - $crate::io::_print($crate::format_args!($($arg)*)) - }; + ($($arg:tt)*) => {{ + $crate::io::_print($crate::format_args!($($arg)*)); + }}; } /// Prints to the standard output, with a newline. @@ -133,9 +133,9 @@ macro_rules! println { #[cfg_attr(not(test), rustc_diagnostic_item = "eprint_macro")] #[allow_internal_unstable(print_internals)] macro_rules! eprint { - ($($arg:tt)*) => { - $crate::io::_eprint($crate::format_args!($($arg)*)) - }; + ($($arg:tt)*) => {{ + $crate::io::_eprint($crate::format_args!($($arg)*)); + }}; } /// Prints to the standard error, with a newline. From a6100988ff63ff9e2f9a0693f51cb8aa06f5cf9f Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 22 May 2022 17:39:44 -0700 Subject: [PATCH 3/3] Fix clippy explicit_write lint for new writeln implementation --- .../clippy/clippy_lints/src/explicit_write.rs | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/explicit_write.rs b/src/tools/clippy/clippy_lints/src/explicit_write.rs index 3e2217c28da3a..d8f765b288a6c 100644 --- a/src/tools/clippy/clippy_lints/src/explicit_write.rs +++ b/src/tools/clippy/clippy_lints/src/explicit_write.rs @@ -4,7 +4,8 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::{is_expn_of, match_function_call, paths}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; +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, declare_tool_lint}; use rustc_span::sym; @@ -39,7 +40,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { if let ExprKind::MethodCall(unwrap_fun, [write_call], _) = expr.kind; if unwrap_fun.ident.name == sym::unwrap; // match call to write_fmt - if let ExprKind::MethodCall(write_fun, [write_recv, write_arg], _) = write_call.kind; + if let ExprKind::MethodCall(write_fun, [write_recv, write_arg], _) = look_in_block(cx, &write_call.kind); if write_fun.ident.name == sym!(write_fmt); // match calls to std::io::stdout() / std::io::stderr () if let Some(dest_name) = if match_function_call(cx, write_recv, &paths::STDOUT).is_some() { @@ -100,3 +101,34 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { } } } + +/// If `kind` is a block that looks like `{ let result = $expr; result }` then +/// returns $expr. Otherwise returns `kind`. +fn look_in_block<'tcx, 'hir>(cx: &LateContext<'tcx>, kind: &'tcx ExprKind<'hir>) -> &'tcx ExprKind<'hir> { + if_chain! { + if let ExprKind::Block(block, _label @ None) = kind; + if let Block { + stmts: [Stmt { kind: StmtKind::Local(local), .. }], + expr: Some(expr_end_of_block), + rules: BlockCheckMode::DefaultBlock, + .. + } = block; + + // Find id of the local that expr_end_of_block resolves to + if let ExprKind::Path(QPath::Resolved(None, expr_path)) = expr_end_of_block.kind; + if let Res::Local(expr_res) = expr_path.res; + if let Some(Node::Binding(res_pat)) = cx.tcx.hir().find(expr_res); + + // Find id of the local we found in the block + if let PatKind::Binding(BindingAnnotation::Unannotated, local_hir_id, _ident, None) = local.pat.kind; + + // If those two are the same hir id + if res_pat.hir_id == local_hir_id; + + if let Some(init) = local.init; + then { + return &init.kind; + } + } + kind +}