Format core and std macro rules, removing needless surrounding blocks by dtolnay · Pull Request #94868 · rust-lang/rust (original) (raw)

dtolnay

Many of the asserting and printing macros in core and std are written with prehistoric-looking formatting, like this:

macro_rules! println {
() => ($crate::print!("\n"));
($($arg:tt)*) => ({
crate::io::print(crate::io::_print(crate::io::print(crate::format_args_nl!($($arg)*));
})
}

In modern Rust style this would conventionally be written as follows instead, always using braces and a trailing semicolon on the macro arms:

macro_rules! println {
() => {
$crate::print!("\n")
};
($($arg:tt)*) => {
crate::io::print(crate::io::_print(crate::io::print(crate::format_args_nl!($($arg)*))
};
}

Getting rid of the unneeded braces inside the expansion reduces extraneous indentation in macro-expanded code. For example:

println!("repro {}", true);

// before:

{ ::std::io::_print( ::core::fmt::Arguments::new_v1( &["repro ", "\n"], &[::core::fmt::ArgumentV1::new_display(&true)], ), ); };

// after:

::std::io::_print( ::core::fmt::Arguments::new_v1( &["repro ", "\n"], &[::core::fmt::ArgumentV1::new_display(&true)], ), );

@dtolnay

@rustbot rustbot added the T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

label

Mar 11, 2022

@rust-highfive

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

dtolnay

Comment on lines -15 to 16

LL ~ Foo(2, b) => println!("{}", b)
LL ~ Foo(2, b) => println!("{}", b),
LL + Foo(_, _) => todo!()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dylan-DPC

@Dylan-DPC

@bors

📌 Commit af53809 has been approved by Dylan-DPC

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Mar 13, 2022

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Mar 13, 2022

@matthiaskrgr

Format core and std macro rules, removing needless surrounding blocks

Many of the asserting and printing macros in core and std are written with prehistoric-looking formatting, like this:

https://github.com/rust-lang/rust/blob/335ffbfa547df94ac236f5c56130cecf99c8d82b/library/std/src/macros.rs#L96-L101

In modern Rust style this would conventionally be written as follows instead, always using braces and a trailing semicolon on the macro arms:

https://github.com/rust-lang/rust/blob/af53809c874e0afb7be966df4d3cfcaa05277c53/library/std/src/macros.rs#L98-L105

Getting rid of the unneeded braces inside the expansion reduces extraneous indentation in macro-expanded code. For example:

println!("repro {}", true);
// before:

{
    ::std::io::_print(
        ::core::fmt::Arguments::new_v1(
            &["repro ", "\n"],
            &[::core::fmt::ArgumentV1::new_display(&true)],
        ),
    );
};
// after:

::std::io::_print(
    ::core::fmt::Arguments::new_v1(
        &["repro ", "\n"],
        &[::core::fmt::ArgumentV1::new_display(&true)],
    ),
);

@bors

⌛ Testing commit af53809 with merge 28838f68c28e8f45725d715555a73c55dbd9323d...

@bors

@bors bors added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Mar 13, 2022

@rust-log-analyzer

This comment has been minimized.

dtolnay

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to reproduce the run-make-fulldeps/coverage-reports failure on aarch64-gnu, but I noticed this snippet in the logs which looks relevant, so I will try to make the change that it shows.

--- expected_show_coverage.closure.txt 2022-03-13 10:25:55.192734369 +0000 +++ /checkout/obj/build/aarch64-unknown-linux-gnu/test/run-make-fulldeps/coverage-reports/coverage-reports/actual_show_coverage.closure.txt 2022-03-13 11:10:50.435023772 +0000 @@ -116,8 +116,8 @@ 116| 1| 117| 1| let 118| 1| _unused_closure

@dtolnay

@dtolnay

@bors

📌 Commit ac5c657 has been approved by Dylan-DPC

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Mar 16, 2022

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 16, 2022

@bors

Rollup of 5 pull requests

Successful merges:

Failed merges:

r? @ghost @rustbot modify labels: rollup

This was referenced

Apr 26, 2022

bors added a commit to rust-lang-ci/rust that referenced this pull request

May 1, 2022

@bors

…lacrum

Make [e]println macros eagerly drop temporaries (for backport)

This PR extracts the subset of rust-lang#96455 which is only the parts necessary for fixing the 1.61-beta regressions in rust-lang#96434.

My larger PR rust-lang#96455 contains a few other changes relative to the pre-rust-lang#94868 behavior; those are not necessary to backport into 1.61.

argument position before rust-lang#94868 after rust-lang#94868 after this PR
write!($tmp, "…", …) 😡 😡 😡
write!(…, "…", $tmp) 😡 😡 😡
writeln!($tmp, "…", …) 😡 😡 😡
writeln!(…, "…", $tmp) 😡 😡 😡
print!("…", $tmp) 😡 😡 😡
println!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
eprint!("…", $tmp) 😡 😡 😡
eprintln!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
panic!("…", $tmp) :smiley_cat: :smiley_cat: :smiley_cat:

ehuss pushed a commit to ehuss/rust that referenced this pull request

May 4, 2022

@bors @ehuss

…lacrum

Make [e]println macros eagerly drop temporaries (for backport)

This PR extracts the subset of rust-lang#96455 which is only the parts necessary for fixing the 1.61-beta regressions in rust-lang#96434.

My larger PR rust-lang#96455 contains a few other changes relative to the pre-rust-lang#94868 behavior; those are not necessary to backport into 1.61.

argument position before rust-lang#94868 after rust-lang#94868 after this PR
write!($tmp, "…", …) 😡 😡 😡
write!(…, "…", $tmp) 😡 😡 😡
writeln!($tmp, "…", …) 😡 😡 😡
writeln!(…, "…", $tmp) 😡 😡 😡
print!("…", $tmp) 😡 😡 😡
println!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
eprint!("…", $tmp) 😡 😡 😡
eprintln!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
panic!("…", $tmp) :smiley_cat: :smiley_cat: :smiley_cat:

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

May 12, 2022

@GuillaumeGomez

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, "…", …) 😡 😡 :smiley_cat:
write!(…, "…", $tmp) 😡 😡 :smiley_cat:
writeln!($tmp, "…", …) 😡 😡 :smiley_cat:
writeln!(…, "…", $tmp) 😡 😡 :smiley_cat:
print!("…", $tmp) 😡 😡 :smiley_cat:
println!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
eprint!("…", $tmp) 😡 😡 :smiley_cat:
eprintln!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
panic!("…", $tmp) :smiley_cat: :smiley_cat: :smiley_cat:

Example of code that is affected by this change:

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.

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`

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

May 14, 2022

@GuillaumeGomez

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, "…", …) 😡 😡 :smiley_cat:
write!(…, "…", $tmp) 😡 😡 :smiley_cat:
writeln!($tmp, "…", …) 😡 😡 :smiley_cat:
writeln!(…, "…", $tmp) 😡 😡 :smiley_cat:
print!("…", $tmp) 😡 😡 :smiley_cat:
println!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
eprint!("…", $tmp) 😡 😡 :smiley_cat:
eprintln!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
panic!("…", $tmp) :smiley_cat: :smiley_cat: :smiley_cat:

Example of code that is affected by this change:

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.

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`

MabezDev pushed a commit to esp-rs/rust that referenced this pull request

May 17, 2022

@bors @MabezDev

…lacrum

Make [e]println macros eagerly drop temporaries (for backport)

This PR extracts the subset of rust-lang#96455 which is only the parts necessary for fixing the 1.61-beta regressions in rust-lang#96434.

My larger PR rust-lang#96455 contains a few other changes relative to the pre-rust-lang#94868 behavior; those are not necessary to backport into 1.61.

argument position before rust-lang#94868 after rust-lang#94868 after this PR
write!($tmp, "…", …) 😡 😡 😡
write!(…, "…", $tmp) 😡 😡 😡
writeln!($tmp, "…", …) 😡 😡 😡
writeln!(…, "…", $tmp) 😡 😡 😡
print!("…", $tmp) 😡 😡 😡
println!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
eprint!("…", $tmp) 😡 😡 😡
eprintln!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
panic!("…", $tmp) :smiley_cat: :smiley_cat: :smiley_cat:

bors added a commit to rust-lang-ci/rust that referenced this pull request

May 23, 2022

@bors

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, "…", …) 😡 😡 :smiley_cat:
write!(…, "…", $tmp) 😡 😡 :smiley_cat:
writeln!($tmp, "…", …) 😡 😡 :smiley_cat:
writeln!(…, "…", $tmp) 😡 😡 :smiley_cat:
print!("…", $tmp) 😡 😡 :smiley_cat:
println!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
eprint!("…", $tmp) 😡 😡 :smiley_cat:
eprintln!("…", $tmp) :smiley_cat: 😡 :smiley_cat:
panic!("…", $tmp) :smiley_cat: :smiley_cat: :smiley_cat:

Example of code that is affected by this change:

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.

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`

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request

Jul 28, 2022

@Dylan-DPC

Revert write! and writeln! to late drop temporaries

Closes (on master, but not on beta) rust-lang#99684 by reverting the write! and writeln! parts of rust-lang#96455.

argument position before
rust-lang#94868
after
rust-lang#94868
after
rust-lang#96455
after
this PR
desired
(unimplementable)
write!($tmp, "…", …) ⸺late ⸺late early⸺ ⸺late ⸺late
write!(…, "…", $tmp) ⸺late ⸺late early⸺ ⸺late early⸺
writeln!($tmp, "…", …) ⸺late ⸺late early⸺ ⸺late ⸺late
writeln!(…, "…", $tmp) ⸺late ⸺late early⸺ ⸺late early⸺
print!("…", $tmp) ⸺late ⸺late early⸺ early⸺ early⸺
println!("…", $tmp) early⸺ ⸺late early⸺ early⸺ early⸺
eprint!("…", $tmp) ⸺late ⸺late early⸺ early⸺ early⸺
eprintln!("…", $tmp) early⸺ ⸺late early⸺ early⸺ early⸺
panic!("…", $tmp) early⸺ early⸺ early⸺ early⸺ early⸺

"Late drop" refers to dropping temporaries at the nearest semicolon outside of the macro invocation.

"Early drop" refers to dropping temporaries inside of the macro invocation.

@dtolnay dtolnay added T-libs

Relevant to the library team, which will review and decide on the PR/issue.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Jan 14, 2025