Give better suggestion when const Range*'s are used as patterns#76222
Give better suggestion when const Range*'s are used as patterns#76222bors merged 1 commit intorust-lang:masterfrom
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
There was a problem hiding this comment.
Ideally the output would look closer to the following:
error[E0308]: mismatched types
--> $DIR/issue-76191.rs:10:9
|
LL | const RANGE: RangeInclusive<i32> = 0..=255;
| ------------------------------------------- constant defined here
...
LL | match n {
| - this expression has type `i32`
LL | RANGE => {}
| ^^^^^
| |
| expected `i32`, found struct `std::ops::RangeInclusive`
| `RANGE` is interpreted as a constant, not a new binding
|
= note: expected type `i32`
found struct `std::ops::RangeInclusive<i32>`
= note: constants only support matching the same type
help: you may want to move the range into the match block
|
LL | 0..=255 => {}
| ^^^^^^^
The later suggestion might not be feasible (depending on whether you can get the hir::Expr corresponding to the const).
There was a problem hiding this comment.
Oh hmm, I can look into doing that, but would likely make that a separate pull request
|
Updated to take into account @estebank's comments, as well as only emit this new error when its const as well as a range, as well as (try) to fix the fmt ci failure |
|
Do I need to squash these commits? |
estebank
left a comment
There was a problem hiding this comment.
Can you squash your commits? r=me after that.
There was a problem hiding this comment.
| let msg = "constants only support matching by type, \ | |
| if you meant to match against a range of values, \ | |
| consider using a range pattern like `min ..= max` in the match block"; | |
| let msg = "constants only support matching by type, \ | |
| if you meant to match against a range of values, \ | |
| consider using a range pattern like `min ..= max` in the match block"; |
There was a problem hiding this comment.
If we take the DefId from the Res...
There was a problem hiding this comment.
...we can use that DefId here to do something like the following so that we could provide a structured suggestion
match self.tcx.hir().get_if_local(def_id)
hir::Node::Item(hir::Item { kind: hir::ItemKind::Const(_ty, body_id), ..}) => {
// use the body_id to get the `const`'s init pattern
}
_ => {
}
}But I don't think this needs to be in this PR.
There was a problem hiding this comment.
Ill make a personal note to make a followup, as I'm still learning all this stuff its nice to do things 1 bit at a time
|
bitten by: #76222, one sec! |
a690475 to
9cdbac3
Compare
|
@bors r+ |
|
📌 Commit 5e126c9 has been approved by |
Give better suggestion when const Range*'s are used as patterns Fixes rust-lang#76191 let me know if there is more/different information you want to show in this case
|
☀️ Test successful - checks-actions, checks-azure |
give *even better* suggestion when matching a const range notice that the err already has "constant defined here" so this is now *exceedingly clear* extension to rust-lang#76222 r? @estebank
give *even better* suggestion when matching a const range notice that the err already has "constant defined here" so this is now *exceedingly clear* extension to rust-lang#76222 r? @estebank
Fixes #76191
let me know if there is more/different information you want to show in this case