format_args: insert implicit named arguments by format trait also

[fee1-dead]

This fixes #109576 by not using the same argument for implicit named arguments that have the same name but different traits.

[rustbot]

r? @lcnr

(rustbot has picked a reviewer for you, use r? to override)

[compiler-errors]

hmmmm this isn't likely to have perf implications, but perhaps?

@bors try @rust-timer queue

[bors]

⌛ Trying commit 4f6a26f with merge e6706a8a21f2a7f85312351326ede56fca10b6de...

[bors]

☀️ Try build successful - checks-actions
Build commit: e6706a8a21f2a7f85312351326ede56fca10b6de (e6706a8a21f2a7f85312351326ede56fca10b6de)

[rust-timer]

Finished benchmarking commit (e6706a8a21f2a7f85312351326ede56fca10b6de): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.3%, 1.3%]	1
Regressions ❌ (secondary)	4.9%	[4.9%, 4.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.3%	[1.3%, 1.3%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

[lcnr]

not familiar with this code

r? @m-ou-se

[m-ou-se]

So, before this PR, this:

format_args!("{x} {x:?}")

expanded to:

format_args!("{0} {0:?}", x)

where x has the span of the original {x} placeholder.

But after this PR, it expands to:

format_args!("{0} {1:?}", x, x)

with each x having their own span, which indeed solves the diagnostics issue.

Currently, that results in roughly the same expression down the line, because the arguments are split/duplicated by format trait anyway, so two pointers to x in both cases. However, in the near future, a new implementation will probably do that better, needing only one pointer to x (and two function pointers). And then, this PR would actually be a (small) regression, by duplicating the argument, adding an extra pointer to x. Then we'll end up in the situation where format_args("{x} {x:?}") results in less efficient code than format_args!("{x} {x:?}", x=x), even though the two were supposed to be equivalent.

I'm wondering if there's another way to do that that doesn't duplicate the argument, without making the patch much more complicated.

[fee1-dead]

It could be possible, by changing the AST->HIR lowering step to use the individual span of implicit captured arguments instead. In AST we might have stored this already, and in HIR we just need to wire the span in expansion.

[m-ou-se]

I think I found a much easier way to fix this that also improves the diagnostics in some other cases. I'll post a PR shortly if it works. ^^

[m-ou-se]

Now that github is finally back online, here is my alternative: #109664