Auto merge of #96455 - dtolnay:writetmp, r=m-ou-se

bors · bors · commit c186f7c07912 · 2022-05-23T02:50:50.000Z
Make write/print macros eagerly drop temporaries This PR fixes the 2 regressions in #96434 (`println` and `eprintln`) and changes all the other similar macros (`write`, `writeln`, `print`, `eprint`) to match the old pre-#94868 behavior of `println` and `eprintln`. argument position | before #94868 | after #94868 | after this PR --- |:---:|:---:|:---: `write!($tmp, "…", …)` | :rage: | :rage: | :smiley_cat: `write!(…, "…", $tmp)` | :rage: | :rage: | :smiley_cat: `writeln!($tmp, "…", …)` | :rage: | :rage: | :smiley_cat: `writeln!(…, "…", $tmp)` | :rage: | :rage: | :smiley_cat: `print!("…", $tmp)` | :rage: | :rage: | :smiley_cat: `println!("…", $tmp)` | :smiley_cat: | :rage: | :smiley_cat: `eprint!("…", $tmp)` | :rage: | :rage: | :smiley_cat: `eprintln!("…", $tmp)` | :smiley_cat: | :rage: | :smiley_cat: `panic!("…", $tmp)` | :smiley_cat: | :smiley_cat: | :smiley_cat: Example of code that is affected by this change: ```rust use std::sync::Mutex; fn main() { let mutex = Mutex::new(0); print!("{}", mutex.lock().unwrap()) /* no semicolon */ } ``` You can see several real-world examples like this in the Crater links at the top of #96434. This code failed to compile prior to this PR as follows, but works after this PR. ```console error[E0597]: `mutex` does not live long enough --> src/main.rs:5:18 | 5 | print!("{}", mutex.lock().unwrap()) /* no semicolon */ | ^^^^^^^^^^^^--------- | | | borrowed value does not live long enough | a temporary with access to the borrow is created here ... 6 | } | - | | | `mutex` dropped here while still borrowed | ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `MutexGuard` ```
diff --git 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
@@ -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.
diff --git a/src/test/ui/macros/format-args-temporaries.rs 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 */
+    };
+}
diff --git 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
+}
