Use UnsafeArg in Arguments::new_v1

[camsteffen]

...instead of having a panic case.

Continues #89139

r? @Mark-Simulacrum

[rust-highfive]

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

[camsteffen]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 8999391ffd611c6603d17d7c6a864086c524eac4 with merge 2d0cbf47213faa5e914df67c6ecff357704e6c60...

[camsteffen]

@rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[camsteffen]

@rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[camsteffen]

@rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[camsteffen]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit a54bc5873b02d0774f787ecc381965048efb8fca with merge 05b81d65ee137dd24915e693fee7bab00f70b0d1...

[bors]

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

[rust-timer]

Queued 05b81d65ee137dd24915e693fee7bab00f70b0d1 with parent 840acd3, future comparison URL.

[rust-timer]

Finished benchmarking commit (05b81d65ee137dd24915e693fee7bab00f70b0d1): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

• Large improvement in instruction counts (up to -2.5% on full builds of keccak)
• Moderate regression in instruction counts (up to 0.9% on incr-unchanged builds of html5ever)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

[camsteffen]

@bors try @rust-timer queue

(probably should have waited for #89139 to be merged in the first place)

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[Mark-Simulacrum]

Looking at the regressions in the perf run above, they're mostly down to some slight codegen time regressions. Looking at the IR generated by format_args!, though, it seems like it's pretty much the same or maybe slightly better at most -- I think the primary difference is that the UnsafeArg unsafe constructor creates an extra basic block in the MIR and LLVM IR (since call is a terminator), even though it's not actually codegen'd into LLVM IR even without optimizations.

My suspicion is that these are related to UnsafeArg::new being const now -- I've adjusted to just apply that change for another evaluation.

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 69cdb0d38780c849ae2dc636a464e89b4a8aa690 with merge 8d71823382b5b3f1a45de0524158120e3a504461...

[Mark-Simulacrum]

@bors try

[bors]

⌛ Trying commit d753e34e94a4c707e15a4cd006e02ea234ad2e5e with merge 4790386457cce89d275a3d6ec0c4cb81c71cd565...

[bors]

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

[rust-timer]

Queued 4790386457cce89d275a3d6ec0c4cb81c71cd565 with parent 2b6ed3b, future comparison URL.

[rust-timer]

Finished benchmarking commit (4790386457cce89d275a3d6ec0c4cb81c71cd565): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led to changes in compiler perf.

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

[Mark-Simulacrum]

@bors try @rust-timer queue

Let's try the original set again, just in case.

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 86b42ab with merge b16fcaca1209afedd97a9dd4db06d5abdadac403...

[bors]

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

[rust-timer]

Queued b16fcaca1209afedd97a9dd4db06d5abdadac403 with parent 8a12be7, future comparison URL.

[rust-timer]

Finished benchmarking commit (b16fcaca1209afedd97a9dd4db06d5abdadac403): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

• Moderate improvement in instruction counts (up to -3.4% on incr-full builds of ctfe-stress-4)
• Large regression in instruction counts (up to 1.5% on full builds of cranelift-codegen)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

[Mark-Simulacrum]

I believe the regressions, both in incremental and non-incremental benchmarks, are largely due to added queries checking that the UnsafeArg arguments being added are well-formed and such. That seems like a cost of this change, for sure, but one that seems not too unexpected.

I don't really have an explanation for the improvements, though. It seems like they are due to a large amount of instructions in process_obligations no longer running -- but that doesn't seem like it should be tied into this patch. It might be shuffling of instructions or something into different basic blocks which causes changes to optimizers, but I have no real proof of this. They don't seem as important to investigate regardless.

I am a little hesitant to proceed with accepting the regressions, particularly when it seems like they don't actually really benefit us (and indeed the API is "needlessly" made unsafe...). I think as of now, I'd prefer that we close this PR, but I am open to being convinced otherwise. I don't think there's an approach to this that avoids paying the cost for the extra work getting expanded into a bunch of user code (even if the runtime cost is ~0).