-
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
Detect Python-like slicing and suggest how to fix #111133
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
| ~~ | ||
help: maybe write a path separator here | ||
| | ||
LL | &[1, 2, 3][1::2]; |
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.
it would be nice if we could get rid of this suggestion
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.
Sounds fair, enclosing the code that triggers this suggestion with else
-block should get the job done
// if a previous and next token of the current one is | ||
// integer literal (e.g. `1:42`), it's likely a range | ||
// expression for Pythonistas and we can suggest so. | ||
if self.prev_token.is_integer_lit() |
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.
can you add a call to self.may_recover() just to be safe?
0c14387
to
b8e582e
Compare
"maybe write a path separator here", | ||
"::", | ||
self.token.span, | ||
"you might have meant to make a slice with range index", |
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.
the error message is too specific for the help being generic. It's sadly not easy to only trigger this only inside indexing expressions, so we should make the message less specific (while still being useful). Maybe you might have meant a range expression
? I think that within the context of indexing, that message should still be useful enough. If you don't think so, you could also add , which can be used for indexing
at the end, that should really cover it (but being a little verbose).
other than that, I like that change and would be happy to merge it after you improve the message
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.
That's a good point. As of now that the diagnostic will be triggered in a broader context than indexing, your suggestion feels just right
@rustbot author |
@hkmatsumoto any updates on this? |
r? compiler |
Oops, so sorry for taking long to respond. This was buried under a pile of notifications I got during AFK for exams... |
Also addressed merge conflicts upon rebasing.
b8e582e
to
730d299
Compare
@rustbot review |
@bors r+ rollup |
… r=TaKO8Ki Detect Python-like slicing and suggest how to fix Fix rust-lang#108215
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#111133 (Detect Python-like slicing and suggest how to fix) - rust-lang#114708 (Allow setting `rla` labels via `rustbot`) - rust-lang#117526 (Account for `!` arm in tail `match` expr) - rust-lang#118282 (effects: Run `enforce_context_effects` for all method calls) - rust-lang#118366 (Detect and reject malformed `repr(Rust)` hints) - rust-lang#118375 (Add -Zunpretty=stable-mir output test) r? `@ghost` `@rustbot` modify labels: rollup
… r=TaKO8Ki Detect Python-like slicing and suggest how to fix Fix rust-lang#108215
…llaumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#111133 (Detect Python-like slicing and suggest how to fix) - rust-lang#114708 (Allow setting `rla` labels via `rustbot`) - rust-lang#117526 (Account for `!` arm in tail `match` expr) - rust-lang#118341 (Simplify indenting in THIR printing) - rust-lang#118366 (Detect and reject malformed `repr(Rust)` hints) - rust-lang#118375 (Add -Zunpretty=stable-mir output test) - rust-lang#118381 (rustc_span: Use correct edit distance start length for suggestions) - rust-lang#118384 (Address unused tuple struct fields in rustdoc) r? `@ghost` `@rustbot` modify labels: rollup
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#111133 (Detect Python-like slicing and suggest how to fix) - rust-lang#114708 (Allow setting `rla` labels via `rustbot`) - rust-lang#117526 (Account for `!` arm in tail `match` expr) - rust-lang#118172 (Add `pretty_terminator` to pretty stable-mir) - rust-lang#118202 (Added linker_arg(s) Linker trait methods for link-arg to be prefixed "-Wl," for cc-like linker args and not verbatim) - rust-lang#118374 (QueryContext: rename try_collect_active_jobs -> collect_active_jobs, change return type from Option<QueryMap> to QueryMap) - rust-lang#118381 (rustc_span: Use correct edit distance start length for suggestions) - rust-lang#118382 (Address unused tuple struct fields in the compiler) - rust-lang#118384 (Address unused tuple struct fields in rustdoc) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#111133 - hkmatsumoto:handle-python-slicing, r=TaKO8Ki Detect Python-like slicing and suggest how to fix Fix rust-lang#108215
Fix #108215