-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Track caller of slice split and swap #91961
Conversation
r? @kennytm (rust-highfive has picked a reviewer for you, use r? to override) |
@bors r+ rollup=never |
📌 Commit 084ea21 has been approved by |
I'm not clear on the motivation for this change - aiui, |
The reason is exactly the same as for unwrap: these methods are also panicking, and the panic needs a proper location. Currently these methods report invalid input as their own internal failure:
Which is unhelpful and from user perspective shows an irrelevant location. Other slice methods like I think from usability perspective track_caller should be used on all std functions that check user input using assertions. However, I'm not adding these wholesale out of caution, in case it added runtime overhead. For the functions I've checked that it doesn't. |
Thanks for the explanation, so to paraphrase for my own understanding, the plan is to add |
Yes, I think it would be good to add However, I'm sensing you're pushing back against this, so I'm not insisting that all of them need to be changed, I'm just focusing on the cases I've ran into in the real world. |
⌛ Testing commit 084ea21 with merge 3f5b002ff05dc01626010327a03456bbc2d122a5... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
It needs a "bless" of UI tests on Windows. Unfortunately, I'm having problems getting |
You might want to ask on zulip to find a windows user. |
☀️ Test successful - checks-actions |
Finished benchmarking commit (82418f9): 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. @rustbot label: -perf-regression |
Improves error location for
slice.split_at*()
andslice.swap()
.These are generic inline functions, so the
#[track_caller]
on them is free — only changes a value of an argument already passed to panicking code.