-
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
Implement suggestion for never type fallback lints #132383
Implement suggestion for never type fallback lints #132383
Conversation
ccb4fde
to
b4a6549
Compare
yippee this is ready i think |
We could, perhaps, even reuse this suggestion to tell users to annotate their code with |
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.
I love the diagnostic changes, that all look very good!
I have a couple minor questions.
// We can't turbofish consts :( | ||
&& args.iter().all(|arg| matches!(arg.unpack(), ty::GenericArgKind::Type(_) | ty::GenericArgKind::Lifetime(_))) |
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.
I'm not sure I understand? this checks that there are no const genetics, but you can turbofish those?
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.
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 _
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.
Ah, ic
} | ||
|
||
/// Try to collect a useful suggestion to preserve fallback to `()`. | ||
struct VidVisitor<'a, 'tcx> { |
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.
maybe name it daunting else? I'm not sure what tho, but something that highlights that it finds possible suggestion places...
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.
Ya i can name it better and give it a pass for more comments
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 after that then
b4a6549
to
b4248ae
Compare
@bors r=WaffleLapkin |
…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: - [x] Try to annotate `_` -> `()` - [x] Try to annotate local variables if they're un-annotated: `let x = ...` -> `let x: () = ...` - [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.
…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
…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: - [x] Try to annotate `_` -> `()` - [x] Try to annotate local variables if they're un-annotated: `let x = ...` -> `let x: () = ...` - [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.
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 `f16` and `f128` to `invalid_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
…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 `f16` and `f128` to `invalid_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
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: - [x] Try to annotate `_` -> `()` - [x] Try to annotate local variables if they're un-annotated: `let x = ...` -> `let x: () = ...` - [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.
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:
_
->()
let x = ...
->let x: () = ...
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.