Deduplicate panic_fmt by nbdd0121 · Pull Request #88860 · rust-lang/rust · GitHu...

Ahh, begin_panic_fmt get params by reference, when panic_fmt takes by value. It's actually interesting to change panic_fmt to get it by reference too, as reference should be shorter than Arguments struct.

It's an existing inconsistency between libcore and libstd API. The inconsistency actually makes the compiler/rustc_const_eval/src/const_eval/machine.rs's begin_panic_fmt -> const_panic_fmt replacement unsound (but since const panic doesn't allow arguments today this unsound path is never hit).

Ahh, begin_panic_fmt get params by reference, when panic_fmt takes by value. It's actually interesting to change panic_fmt to get it by reference too, as reference should be shorter that Arguments struct.

I actually don't think there'll be any meaningful difference. Size of Arguments would make it be passed by reference anyway.

Plus, many existing methods like write_fmt by value and Arguments is Copy so I think it's actually good to take it by value for consistency.

What are you suggesting by listing the numbers? As I've said, the size of Arguments (is larger than 2 usize) would make it be passed by reference. So codegen will behave exactly the same with or without the & (https://godbolt.org/z/h69MKjdsn), and for semantics and consistency it should be Arguments rather than &Arguments.

Sorry, i didn't knew that there some optimization that pass big Copy types by ref even if i explicitly pass it by value.

Value passed into PanicInfo::internal_constructor by Option'ing reference anyway, so there is no sense to copying it before that. (this comment about pub fn panic_fmt(fmt: fmt::Arguments<'_>) -> !)

Tried to check that in small example, looks like parameter really isn't copied in panic_fmt (didn't checked this in stdlib). I remember that i played somewhere with fmt::Arguments and saw needless copying in stdlib.