Give better suggestion when const Range*'s are used as patterns #76222

guswynn · 2020-09-01T22:56:19Z

let me know if there is more/different information you want to show in this case

rust-highfive · 2020-09-01T22:56:22Z

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.

src/test/ui/issues/issue-76191.rs

compiler/rustc_typeck/src/check/pat.rs

estebank · 2020-09-03T20:51:16Z

src/test/ui/issues/issue-76191.stderr

+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
+   |         help: introduce a new binding instead: `other_range`
+   |
+   = note: expected type `i32`
+            found struct `std::ops::RangeInclusive<i32>`
+   = note: Constants only support matching the same type. If you are using range syntax (i.e. ..= or similar) in your constant you may want to move the range into the match block


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).

(This is not a blocker).

Oh hmm, I can look into doing that, but would likely make that a separate pull request

src/test/ui/issues/issue-76191.stderr

guswynn · 2020-09-03T21:14:48Z

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

guswynn · 2020-09-11T18:51:15Z

Do I need to squash these commits?

estebank

Can you squash your commits? r=me after that.

estebank · 2020-09-11T20:53:50Z

compiler/rustc_typeck/src/check/pat.rs

+                            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";


Suggested change

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";

estebank · 2020-09-11T21:01:11Z

compiler/rustc_typeck/src/check/pat.rs

-                        e.span_suggestion(ident.span, msg, sugg, Applicability::HasPlaceholders);
+                        let const_def_id = match pat_ty.kind {
+                            Adt(def, _) => match res {
+                                Res::Def(DefKind::Const, _) => Some(def.did),


If we take the DefId from the Res...

estebank · 2020-09-11T21:07:14Z

compiler/rustc_typeck/src/check/pat.rs

+                            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";
+                            e.note(msg);


...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.

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

Sounds good to me

guswynn · 2020-09-11T21:45:48Z

bitten by: #76222, one sec!

estebank · 2020-09-11T23:45:59Z

@bors r+

bors · 2020-09-11T23:46:01Z

📌 Commit 5e126c9 has been approved by estebank

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

bors · 2020-09-12T07:45:39Z

⌛ Testing commit 5e126c9 with merge 2e2e7de...

bors · 2020-09-12T09:34:30Z

☀️ Test successful - checks-actions, checks-azure
Approved by: estebank
Pushing 2e2e7de to master...

@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

@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

rust-highfive assigned nikomatsakis Sep 1, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 1, 2020

guswynn force-pushed the const_diag branch from 8273283 to 2797351 Compare September 1, 2020 22:58

jonas-schievink reviewed Sep 1, 2020

View reviewed changes

src/test/ui/issues/issue-76191.rs Outdated Show resolved Hide resolved

compiler/rustc_typeck/src/check/pat.rs Outdated Show resolved Hide resolved

compiler/rustc_typeck/src/check/pat.rs Outdated Show resolved Hide resolved

guswynn changed the title ~~Link to tracking issue when const's are used as patterns~~ Give better suggest when const Range*'s are used as patterns Sep 3, 2020

guswynn changed the title ~~Give better suggest when const Range*'s are used as patterns~~ Give better suggestion when const Range*'s are used as patterns Sep 3, 2020

jonas-schievink reviewed Sep 3, 2020

View reviewed changes

compiler/rustc_typeck/src/check/pat.rs Outdated Show resolved Hide resolved

compiler/rustc_typeck/src/check/pat.rs Outdated Show resolved Hide resolved

estebank approved these changes Sep 3, 2020

View reviewed changes

jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 8, 2020

estebank approved these changes Sep 11, 2020

View reviewed changes

guswynn force-pushed the const_diag branch from 763aa9f to 7685a80 Compare September 11, 2020 21:31

guswynn force-pushed the const_diag branch 2 times, most recently from a690475 to 9cdbac3 Compare September 11, 2020 22:01

better diag when const ranges are used in patterns

5e126c9

guswynn force-pushed the const_diag branch from 9cdbac3 to 5e126c9 Compare September 11, 2020 22:02

bors added S-waiting-on-bors 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 Sep 11, 2020

Dylan-DPC-zz mentioned this pull request Sep 12, 2020

Rollup of 11 pull requests #76621

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 12, 2020

bors merged commit 2e2e7de into rust-lang:master Sep 12, 2020

rustbot added this to the 1.48.0 milestone Sep 12, 2020

guswynn deleted the const_diag branch September 15, 2020 15:27

guswynn mentioned this pull request Sep 15, 2020

give *even better* suggestion when matching a const range #76749

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give better suggestion when const Range*'s are used as patterns #76222

Give better suggestion when const Range*'s are used as patterns #76222

guswynn commented Sep 1, 2020

rust-highfive commented Sep 1, 2020

estebank Sep 3, 2020

estebank Sep 3, 2020

guswynn Sep 3, 2020

guswynn commented Sep 3, 2020

guswynn commented Sep 11, 2020

estebank left a comment

estebank Sep 11, 2020

estebank Sep 11, 2020

estebank Sep 11, 2020

guswynn Sep 11, 2020

estebank Sep 11, 2020

guswynn commented Sep 11, 2020

estebank commented Sep 11, 2020

bors commented Sep 11, 2020

bors commented Sep 12, 2020

bors commented Sep 12, 2020

Give better suggestion when const Range*'s are used as patterns #76222

Give better suggestion when const Range*'s are used as patterns #76222

Conversation

guswynn commented Sep 1, 2020

rust-highfive commented Sep 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guswynn commented Sep 3, 2020

guswynn commented Sep 11, 2020

estebank left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guswynn commented Sep 11, 2020

estebank commented Sep 11, 2020

bors commented Sep 11, 2020

bors commented Sep 12, 2020

bors commented Sep 12, 2020