Do not deduplicate captured args while expanding format_args!

[ShoyuVanilla]

Resolves #145739

I ran crater with #149291.
While there are still a few seemingly flaky, spurious results, no crates appear to be affected by this breaking change.

The only hit from the lint was
https://github.com/multiversx/mx-sdk-rs/blob/813927c03a7b512a3c6ef9a15690eaf87872cc5c/framework/meta-lib/src/tools/rustc_version_warning.rs#L19-L30,
which performs formatting on consts of type ::semver::Version. These constants contain a nested ::semver::Identifier (Version.pre.identifier) that has a custom destructor. However, this case is not impacted by the change, so no breakage is expected.

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_ast_lowering/src/format.rs

cc @m-ou-se

[rustbot]

r? @spastorino

rustbot has assigned @spastorino.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[ShoyuVanilla]

@rustbot author

[ShoyuVanilla]

@rustbot ready

[theemathas]

Nominating as per #145739 (comment)

[traviscross]

It'd be worth adding a test for the drop behavior.

[traviscross]

Given that this makes more sense for the language, along with the clean crater results and the intuition that it'd be surprising if anything actually leaned on this, I propose:

@rfcbot fcp merge lang

[rust-rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[m-ou-se]

I don't think we should do this. It will make the generated code for println!("{x} {x}"); less efficient, as it will get two separate arguments instead of one.

I don't want to end up in a situation where it would make sense for Clippy to suggest something like:

warning: using the same placeholder multiple times is inefficient as of Rust 1.94.0
 --> src/main.rs:3:5
  |
3 |     println!("{x} {x}");
  |     ^^^^^^^^^^^^^^^^^^^
  |
help: change this to
  |
3 -     println!("{x} {x}");
3 +     println!("{x} {x}", x = x);
  |

Adding , x = x shouldn't make a difference. If adding that makes the resulting code more efficient, I strongly feel like we've done something wrong.

@rust-rfcbot concern equivalence

[m-ou-se]

Also, this FCP should probably have included libs-api.

[RalfJung]

I don't think using the same variable multiple times in a format string is common enough to justify the non-trivial semantic special-casing that is required to make this kind of deduplication work.

[m-ou-se]

I don't think using the same constant with internal mutability in multiple placeholders is common enough do justify this change.

[m-ou-se]

I don't think using the same variable multiple times in a format string is common enough [..]

I think that situation is quite common actually. E.g.

println!("Accepted: {n_accepted}/{n_total}, Rejected: {n_rejected}/{n_total}");

format!("ip link {interface} down && ip link {interface} up");

println!("{indent}line 1\n{indent}line 2");

error!("Feature `{feature}` not enabled. To enable {feature}, add `#![feature({feature})]`.");

warn!("Integer literal out of range for {ty}. The range of {ty} is {min}{ty}..={max}{ty}.");

[workingjubilee]

The RFC specifically says that it should be equivalent to format_args!("{name}", name = name).

https://rust-lang.github.io/rfcs/2795-format-args-implicit-identifiers.html

[theemathas]

@workingjubilee This has been previously discussed at #145739 (comment)

[workingjubilee]

Yes, I'm stating that throwing out this equivalence because of wonkiness when using things that are often specifically highlighted as notoriously bad ideas is a bad idea.

[traviscross]

Could we not get what we want as a lang matter and keep most of the optimization benefit by doing the optimization only when it's guaranteed to not be observable, e.g. when values of the type don't need to be dropped and can't be interior mutable? Is that feasible?