Implement suggestion for never type fallback lints by compiler-errors · Pull Request #132383 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation15 Commits6 Checks6 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Just opening this up for vibes; it's not done yet. I'd still like to make this suggestable in a few more cases before merge:
- Try to annotate
_->() - Try to annotate local variables if they're un-annotated:
let x = ...->let x: () = ... - Try to annotate the self type of a
Trait::method()-><() as Trait>::method().
The only other case we may want to suggest is a missing turbofish, like f() -> f::<()>(). That may be possible, but seems overly annoying.
This partly addresses #132358; the other half of fixing that would be to make the error message a bit better, perhaps just special casing the ? operator 🤔 I don't think I'll do that part.
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
yippee this is ready i think
We could, perhaps, even reuse this suggestion to tell users to annotate their code with () even after edition 2024.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the diagnostic changes, that all look very good!
I have a couple minor questions.
Comment on lines +607 to +618
| // We can't turbofish consts :( | |
|---|---|
| && args.iter().all(|arg | matches!(arg.unpack(), ty::GenericArgKind::Type(_) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand? this checks that there are no const genetics, but you can turbofish those?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to limit the complexity of the suggestion so it would just either have _ for all the non fallback args, but consts cant use _
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ic
| } |
|---|
| /// Try to collect a useful suggestion to preserve fallback to `()`. |
| struct VidVisitor<'a, 'tcx> { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe name it daunting else? I'm not sure what tho, but something that highlights that it finds possible suggestion places...
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya i can name it better and give it a pass for more comments
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me after that then
📌 Commit b4248ae has been approved by WaffleLapkin
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…k-sugg, r=WaffleLapkin
Implement suggestion for never type fallback lints
r? @WaffleLapkin
Just opening this up for vibes; it's not done yet. I'd still like to make this suggestable in a few more cases before merge:
- Try to annotate
_->() - Try to annotate local variables if they're un-annotated:
let x = ...->let x: () = ... - Try to annotate the self type of a
Trait::method()-><() as Trait>::method().
The only other case we may want to suggest is a missing turbofish, like f() -> f::<()>(). That may be possible, but seems overly annoying.
This partly addresses rust-lang#132358; the other half of fixing that would be to make the error message a bit better, perhaps just special casing the ? operator 🤔 I don't think I'll do that part.
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 6 pull requests
Successful merges:
- rust-lang#131829 (Remove support for
-Zprofile(gcov-style coverage instrumentation)) - rust-lang#132341 (Reject raw lifetime followed by
', like regular lifetimes do) - rust-lang#132369 (style-guide: Only use the new binop heuristic for assignments)
- rust-lang#132383 (Implement suggestion for never type fallback lints)
- rust-lang#132437 (coverage: Regression test for inlining into an uninstrumented crate)
- rust-lang#132438 (Remove unncessary option for default rust-analyzer setting)
r? @ghost
@rustbot modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request
…k-sugg, r=WaffleLapkin
Implement suggestion for never type fallback lints
r? @WaffleLapkin
Just opening this up for vibes; it's not done yet. I'd still like to make this suggestable in a few more cases before merge:
- Try to annotate
_->() - Try to annotate local variables if they're un-annotated:
let x = ...->let x: () = ... - Try to annotate the self type of a
Trait::method()-><() as Trait>::method().
The only other case we may want to suggest is a missing turbofish, like f() -> f::<()>(). That may be possible, but seems overly annoying.
This partly addresses rust-lang#132358; the other half of fixing that would be to make the error message a bit better, perhaps just special casing the ? operator 🤔 I don't think I'll do that part.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Rollup of 13 pull requests
Successful merges:
- rust-lang#131829 (Remove support for
-Zprofile(gcov-style coverage instrumentation)) - rust-lang#132369 (style-guide: Only use the new binop heuristic for assignments)
- rust-lang#132383 (Implement suggestion for never type fallback lints)
- rust-lang#132437 (coverage: Regression test for inlining into an uninstrumented crate)
- rust-lang#132438 (Remove unncessary option for default rust-analyzer setting)
- rust-lang#132439 (Add
f16andf128toinvalid_nan_comparison) - rust-lang#132445 (Cleanup attributes around unchecked shifts and unchecked negation in const)
- rust-lang#132448 (Add missing backtick)
- rust-lang#132450 (Show actual MIR when MIR building forgot to terminate block)
- rust-lang#132451 (remove some unnecessary rustc_allow_const_fn_unstable)
- rust-lang#132455 (make const_alloc_layout feature gate only about functions that are already stable)
- rust-lang#132456 (Move remaining inline assembly test files into asm directory)
- rust-lang#132459 (feat(byte_sub_ptr): unstably add ptr::byte_sub_ptr)
r? @ghost
@rustbot modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request
…llaumeGomez
Rollup of 14 pull requests
Successful merges:
- rust-lang#131829 (Remove support for
-Zprofile(gcov-style coverage instrumentation)) - rust-lang#132369 (style-guide: Only use the new binop heuristic for assignments)
- rust-lang#132383 (Implement suggestion for never type fallback lints)
- rust-lang#132413 (update offset_of! docs to reflect the stabilization of nesting)
- rust-lang#132438 (Remove unncessary option for default rust-analyzer setting)
- rust-lang#132439 (Add
f16andf128toinvalid_nan_comparison) - rust-lang#132444 (rustdoc: Directly use rustc_abi instead of reexports)
- rust-lang#132445 (Cleanup attributes around unchecked shifts and unchecked negation in const)
- rust-lang#132448 (Add missing backtick)
- rust-lang#132450 (Show actual MIR when MIR building forgot to terminate block)
- rust-lang#132451 (remove some unnecessary rustc_allow_const_fn_unstable)
- rust-lang#132455 (make const_alloc_layout feature gate only about functions that are already stable)
- rust-lang#132456 (Move remaining inline assembly test files into asm directory)
- rust-lang#132459 (feat(byte_sub_ptr): unstably add ptr::byte_sub_ptr)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#132383 - compiler-errors:never-type-fallback-sugg, r=WaffleLapkin
Implement suggestion for never type fallback lints
r? @WaffleLapkin
Just opening this up for vibes; it's not done yet. I'd still like to make this suggestable in a few more cases before merge:
- Try to annotate
_->() - Try to annotate local variables if they're un-annotated:
let x = ...->let x: () = ... - Try to annotate the self type of a
Trait::method()-><() as Trait>::method().
The only other case we may want to suggest is a missing turbofish, like f() -> f::<()>(). That may be possible, but seems overly annoying.
This partly addresses rust-lang#132358; the other half of fixing that would be to make the error message a bit better, perhaps just special casing the ? operator 🤔 I don't think I'll do that part.
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.