-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Fix ICE in named_arguments_used_positionally
lint
#99263
Conversation
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
@compiler-errors Thank you for fixing this! Sorry I missed this case, but I have a slight suspicion of why the indices don't match. I'll look into that -- should I create an issue for it? |
Yeah, the more issues the merrier. I'll probably only be working on getting this ICE closed out, in case you want some follow-up work 😀 |
@compiler-errors Thank you! I have opened #99266 and will look into it. |
There seems to be a few duplicates of #99261, so this fixes an issue that a number of projects are hitting on nightly (by depending on the popular crate |
f571630
to
2902b92
Compare
Just a heads up, #99271 is also a duplicate. |
@bors p=20 (should land before rollups) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me unless you want approval from someone familiar with this specific lint
☀️ Test successful - checks-actions |
Finished benchmarking commit (5635158): comparison url. Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
Fixes #99261
Fixes #99289
Fixes #99284
Fixes #99273
Fixes #99297
Fixes #99271
Fixes #99327
Fixes #99330
This match pattern:
does not account for when both
width
andprecision
are bothCount::CountIsName
, so split the check for these two fields into two separateif let
.Also, remove any future potential for ICEs by removing the index operator altogether.
It is still suspicious that this indexing was broken and caused the ICE, as opposed to just causing a spurious lint message.
cc @PrestonFrom, who may be familiar with this code because of implementing the lint this touches, perhaps you'd like to look into why named arguments in
FormatSpec.precision
seem to have indices that don't correspond to a span inContext.arg_spans
?Edit: Opened #99265 to track a (related?) incorrect argument indexing issue.