Skip to content

Commit

Permalink
Auto merge of rust-lang#96455 - dtolnay:writetmp, r=m-ou-se
Browse files Browse the repository at this point in the history
Make write/print macros eagerly drop temporaries

This PR fixes the 2 regressions in rust-lang#96434 (`println` and `eprintln`) and changes all the other similar macros (`write`, `writeln`, `print`, `eprint`) to match the old pre-rust-lang#94868 behavior of `println` and `eprintln`.

argument position | before rust-lang#94868 | after rust-lang#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 rust-lang#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`
```
  • Loading branch information
bors committed May 23, 2022
2 parents d125574 + a610098 commit c186f7c
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 14 deletions.
14 changes: 8 additions & 6 deletions library/core/src/macros/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions library/std/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
70 changes: 70 additions & 0 deletions src/test/ui/macros/format-args-temporaries.rs
Original file line number Diff line number Diff line change
@@ -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 */
};
}
36 changes: 34 additions & 2 deletions src/tools/clippy/clippy_lints/src/explicit_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
}

0 comments on commit c186f7c

Please sign in to comment.