Improve diagnostic messages for range patterns. #25743

michaelsproul · 2015-05-24T04:33:06Z

Part of #24407.

Currently the diagnostics for range patterns are a bit wrong:

fn main() {
    match 5u32 {
        0 ... 10 => (),
        'a' ... 10 => (),
        10 ... 'z' => (),
        "what" ... 10 => (),
        "what" ... "well" => (),
        10 ... "what" => ()
    }
}

range.rs:4:9: 4:19 error: mismatched types in range:
 expected integral variable,
    found char [E0211]
range.rs:4         'a' ... 10 => (),
                   ^~~~~~~~~~
range.rs:4:9: 4:16 error: only char and numeric types are allowed in range [E0029]
range.rs:4         'a' ... 10 => (),
                   ^~~~~~~
range.rs:4:9: 4:19 error: mismatched types:
 expected `u32`,
    found `char`
(expected u32,
    found char) [E0308]
range.rs:4         'a' ... 10 => (),
                   ^~~~~~~~~~
range.rs:5:9: 5:19 error: mismatched types in range:
 expected char,
    found integral variable [E0211]
range.rs:5         10 ... 'z' => (),
                   ^~~~~~~~~~
range.rs:5:9: 5:15 error: only char and numeric types are allowed in range [E0029]
range.rs:5         10 ... 'z' => (),
                   ^~~~~~
range.rs:6:9: 6:22 error: mismatched types in range:
 expected integral variable,
    found &-ptr [E0211]
range.rs:6         "what" ... 10 => (),
                   ^~~~~~~~~~~~~
range.rs:6:9: 6:19 error: only char and numeric types are allowed in range [E0029]
range.rs:6         "what" ... 10 => (),
                   ^~~~~~~~~~
range.rs:6:9: 6:22 error: mismatched types:
 expected `u32`,
    found `&'static str`
(expected u32,
    found &-ptr) [E0308]
range.rs:6         "what" ... 10 => (),
                   ^~~~~~~~~~~~~
range.rs:7:9: 7:19 error: only char and numeric types are allowed in range [E0029]
range.rs:7         "what" ... "well" => (),
                   ^~~~~~~~~~
range.rs:7:9: 7:26 error: mismatched types:
 expected `u32`,
    found `&'static str`
(expected u32,
    found &-ptr) [E0308]
range.rs:7         "what" ... "well" => (),
                   ^~~~~~~~~~~~~~~~~
range.rs:8:9: 8:22 error: mismatched types in range:
 expected &-ptr,
    found integral variable [E0211]
range.rs:8         10 ... "what" => ()
                   ^~~~~~~~~~~~~
range.rs:8:9: 8:15 error: only char and numeric types are allowed in range [E0029]
range.rs:8         10 ... "what" => ()
                   ^~~~~~
error: aborting due to 12 previous errors

The problems here are:

The type of the end of the range is used to predict the type of the start (only mildly counter intuitive).
E0029 is erroneously generated for char ... num and num ... char.
u32 is mentioned.
Errors which are essentially the same are reported multiple times.

I've attempted to fix this by checking the requirements in a different order. The output I've achieved for the above example is:

/home/michael/Temp/range.rs:4:17: 4:22 error: mismatched types in range:
 expected char,
    found integral variable [E0211]
/home/michael/Temp/range.rs:4         'a' ... 10 => (),
                                              ^~~~~
/home/michael/Temp/range.rs:5:16: 5:22 error: mismatched types in range:
 expected integral variable,
    found char [E0211]
/home/michael/Temp/range.rs:5         10 ... 'z' => (),
                                             ^~~~~~
/home/michael/Temp/range.rs:6:9: 6:19 error: only char and numeric types are allowed in range [E0029]
/home/michael/Temp/range.rs:6         "what" ... 10 => (),
                                      ^~~~~~~~~~
/home/michael/Temp/range.rs:6:9: 6:19 help: run `rustc --explain E0029` to see a detailed explanation
/home/michael/Temp/range.rs:6:9: 6:19 note: Start type: &'static str
End type: _
/home/michael/Temp/range.rs:6         "what" ... 10 => (),
                                      ^~~~~~~~~~
/home/michael/Temp/range.rs:7:9: 7:26 error: only char and numeric types are allowed in range [E0029]
/home/michael/Temp/range.rs:7         "what" ... "well" => (),
                                      ^~~~~~~~~~~~~~~~~
/home/michael/Temp/range.rs:7:9: 7:26 help: run `rustc --explain E0029` to see a detailed explanation
/home/michael/Temp/range.rs:7:9: 7:26 note: Start type: &'static str
End type: &'static str
/home/michael/Temp/range.rs:7         "what" ... "well" => (),
                                      ^~~~~~~~~~~~~~~~~
/home/michael/Temp/range.rs:8:16: 8:25 error: only char and numeric types are allowed in range [E0029]
/home/michael/Temp/range.rs:8         10 ... "what" => ()
                                             ^~~~~~~~~
/home/michael/Temp/range.rs:8:16: 8:25 help: run `rustc --explain E0029` to see a detailed explanation
/home/michael/Temp/range.rs:8:16: 8:25 note: Start type: _
End type: &'static str
/home/michael/Temp/range.rs:8         10 ... "what" => ()
                                             ^~~~~~~~~
error: aborting due to 5 previous errors

I think this is already tonnes better, but the Start type/End type stuff could be neater. I don't think there's really any need to start a note: block but I wanted to get some feedback on this. I'd also appreciate advice on how to print the integer types as something other than _.

rust-highfive · 2015-05-24T04:33:17Z

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

bors · 2015-05-26T18:32:52Z

☔ The latest upstream changes (presumably #25091) made this pull request unmergeable. Please resolve the merge conflicts.

nrc · 2015-05-28T06:32:00Z

I agree there is no need to start a 'note' block here.

As far as I know, you can't get a better representation of the int inference var than _, if the compiler can't infer the exact type, it is just a inference variable (well, type checking distinguishes between int vars and other inference vars, but error reporting doesn't).

This PR could do with some tests to make sure the errors don't regress.

(Also, needs a rebase)

nrc · 2015-05-28T06:35:15Z

src/librustc_typeck/check/_match.rs

@@ -89,30 +89,46 @@ pub fn check_pat<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
            let lhs_ty = fcx.expr_ty(&**begin);
            let rhs_ty = fcx.expr_ty(&**end);

-            let lhs_eq_rhs =
+            let compat = |t| ty::type_is_numeric(t) || ty::type_is_char(t);


numeric_or_char is a better name than compat

michaelsproul · 2015-05-29T03:46:37Z

I've addressed all your comments and pushed the commit, but I just noticed a failure in the compile-test results.

src/test/compile-fail/match-ill-type1.rs:13:9: 13:13 error: internal compiler error: error comparing literals in range pattern
src/test/compile-fail/match-ill-type1.rs:13         1...2_usize => 1, //~ ERROR mismatched types in range

Don't merge until I've fixed this...

michaelsproul · 2015-05-29T03:47:42Z

src/compiletest/runtest.rs

-        //                1234
-        // Should have 4 spaces: see issue 18946
-        line.starts_with("(")
+        line.starts_with(" ") || line.starts_with("(")


this change was required to support start type: and end type: (each with a preceding space).

michaelsproul · 2015-05-31T13:41:06Z

@nrc: I got it working! By computing a least-upper-bound for the begin and end types of the range and using it as a hint when evaluating and comparing the constants we get inference of the least-complete type from the most-complete one! As a bonus, this also removes the asymmetry whereby lhs_ty was used to type the whole pattern, because we can use the LUB instead. To reflect this change I moved the match-ill-typed example out of compile-fail and into run-pass, with a few added inference tests.

This is the output for the example in the PR description:

range.rs:4:9: 4:19 error: mismatched types in range:
 expected char,
    found integral variable [E0211]
range.rs:4         'a' ... 10 => (),
                   ^~~~~~~~~~
range.rs:5:9: 5:19 error: mismatched types in range:
 expected integral variable,
    found char [E0211]
range.rs:5         10 ... 'z' => (),
                   ^~~~~~~~~~
range.rs:6:9: 6:19 error: only char and numeric types are allowed in range patterns
 start type: &'static str
 end type: _ [E0029]
range.rs:6         "what" ... 10 => (),
                   ^~~~~~~~~~
range.rs:6:9: 6:19 help: run `rustc --explain E0029` to see a detailed explanation
range.rs:7:9: 7:26 error: only char and numeric types are allowed in range patterns
 start type: &'static str
 end type: &'static str [E0029]
range.rs:7         "what" ... "well" => (),
                   ^~~~~~~~~~~~~~~~~
range.rs:7:9: 7:26 help: run `rustc --explain E0029` to see a detailed explanation
range.rs:8:16: 8:25 error: only char and numeric types are allowed in range patterns
 start type: _
 end type: &'static str [E0029]
range.rs:8         10 ... "what" => ()
                          ^~~~~~~~~
range.rs:8:16: 8:25 help: run `rustc --explain E0029` to see a detailed explanation
error: aborting due to 5 previous errors

nrc · 2015-06-01T23:58:06Z

src/test/run-pass/match-range-infer.rs

@@ -8,9 +8,17 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.



Could you add a comment explaining what is being tested please?

nrc · 2015-06-02T00:43:49Z

Other than the small nits, this looks good. I wanted to ask around a few more people that unifying the type here is OK (I'm pretty sure it is). I'll try and get this r+'ed tomorrow.

michaelsproul · 2015-06-02T00:46:31Z

Ok great, cheers.

nikomatsakis · 2015-06-02T21:52:07Z

src/librustc_typeck/check/_match.rs

+            // guarantee that the types are concrete and identical. We need to use the most
+            // specific type as a type hint.
+            // For example if we have 0 ... 5usize, we need to infer 0 : usize.
+            let common_type = infer::common_supertype(


given that we've just unified the two types, computing a common supertype seems harmless but inefficient. If you just want to remove type variables from the type, resolve_type_vars_if_possible or some similar method seems preferable.

michaelsproul · 2015-06-03T06:17:10Z

src/librustc_typeck/check/_match.rs

-            fcx.write_ty(pat.id, lhs_ty);
+            // Now that we know the types can be unified we find the unified type and use
+            // it to type the entire expression.
+            let common_type = fcx.infcx().resolve_type_vars_if_possible(&lhs_ty);


@nikomatsakis, this works... there's no need to call it again with rhs_ty is there?

@michaelsproul nope, that looks good to me. (calling it rhs_ty would yield the same result)

nrc · 2015-06-03T22:53:27Z

@bors r+

bors · 2015-06-03T22:53:28Z

📌 Commit 25d0ef3 has been approved by nrc

Part of #24407. Currently the diagnostics for range patterns are a bit wrong: ```rust fn main() { match 5u32 { 0 ... 10 => (), 'a' ... 10 => (), 10 ... 'z' => (), "what" ... 10 => (), "what" ... "well" => (), 10 ... "what" => () } } ``` ``` range.rs:4:9: 4:19 error: mismatched types in range: expected integral variable, found char [E0211] range.rs:4 'a' ... 10 => (), ^~~~~~~~~~ range.rs:4:9: 4:16 error: only char and numeric types are allowed in range [E0029] range.rs:4 'a' ... 10 => (), ^~~~~~~ range.rs:4:9: 4:19 error: mismatched types: expected `u32`, found `char` (expected u32, found char) [E0308] range.rs:4 'a' ... 10 => (), ^~~~~~~~~~ range.rs:5:9: 5:19 error: mismatched types in range: expected char, found integral variable [E0211] range.rs:5 10 ... 'z' => (), ^~~~~~~~~~ range.rs:5:9: 5:15 error: only char and numeric types are allowed in range [E0029] range.rs:5 10 ... 'z' => (), ^~~~~~ range.rs:6:9: 6:22 error: mismatched types in range: expected integral variable, found &-ptr [E0211] range.rs:6 "what" ... 10 => (), ^~~~~~~~~~~~~ range.rs:6:9: 6:19 error: only char and numeric types are allowed in range [E0029] range.rs:6 "what" ... 10 => (), ^~~~~~~~~~ range.rs:6:9: 6:22 error: mismatched types: expected `u32`, found `&'static str` (expected u32, found &-ptr) [E0308] range.rs:6 "what" ... 10 => (), ^~~~~~~~~~~~~ range.rs:7:9: 7:19 error: only char and numeric types are allowed in range [E0029] range.rs:7 "what" ... "well" => (), ^~~~~~~~~~ range.rs:7:9: 7:26 error: mismatched types: expected `u32`, found `&'static str` (expected u32, found &-ptr) [E0308] range.rs:7 "what" ... "well" => (), ^~~~~~~~~~~~~~~~~ range.rs:8:9: 8:22 error: mismatched types in range: expected &-ptr, found integral variable [E0211] range.rs:8 10 ... "what" => () ^~~~~~~~~~~~~ range.rs:8:9: 8:15 error: only char and numeric types are allowed in range [E0029] range.rs:8 10 ... "what" => () ^~~~~~ error: aborting due to 12 previous errors ``` The problems here are: 1. The type of the end of the range is used to predict the type of the start (only mildly counter intuitive). 2. E0029 is erroneously generated for `char ... num` and `num ... char`. 2. `u32` is mentioned. 3. Errors which are essentially the same are reported multiple times. I've attempted to fix this by checking the requirements in a different order. The output I've achieved for the above example is: ``` /home/michael/Temp/range.rs:4:17: 4:22 error: mismatched types in range: expected char, found integral variable [E0211] /home/michael/Temp/range.rs:4 'a' ... 10 => (), ^~~~~ /home/michael/Temp/range.rs:5:16: 5:22 error: mismatched types in range: expected integral variable, found char [E0211] /home/michael/Temp/range.rs:5 10 ... 'z' => (), ^~~~~~ /home/michael/Temp/range.rs:6:9: 6:19 error: only char and numeric types are allowed in range [E0029] /home/michael/Temp/range.rs:6 "what" ... 10 => (), ^~~~~~~~~~ /home/michael/Temp/range.rs:6:9: 6:19 help: run `rustc --explain E0029` to see a detailed explanation /home/michael/Temp/range.rs:6:9: 6:19 note: Start type: &'static str End type: _ /home/michael/Temp/range.rs:6 "what" ... 10 => (), ^~~~~~~~~~ /home/michael/Temp/range.rs:7:9: 7:26 error: only char and numeric types are allowed in range [E0029] /home/michael/Temp/range.rs:7 "what" ... "well" => (), ^~~~~~~~~~~~~~~~~ /home/michael/Temp/range.rs:7:9: 7:26 help: run `rustc --explain E0029` to see a detailed explanation /home/michael/Temp/range.rs:7:9: 7:26 note: Start type: &'static str End type: &'static str /home/michael/Temp/range.rs:7 "what" ... "well" => (), ^~~~~~~~~~~~~~~~~ /home/michael/Temp/range.rs:8:16: 8:25 error: only char and numeric types are allowed in range [E0029] /home/michael/Temp/range.rs:8 10 ... "what" => () ^~~~~~~~~ /home/michael/Temp/range.rs:8:16: 8:25 help: run `rustc --explain E0029` to see a detailed explanation /home/michael/Temp/range.rs:8:16: 8:25 note: Start type: _ End type: &'static str /home/michael/Temp/range.rs:8 10 ... "what" => () ^~~~~~~~~ error: aborting due to 5 previous errors ``` I think this is already tonnes better, but the `Start type/End type` stuff could be neater. I don't think there's really any need to start a `note:` block but I wanted to get some feedback on this. I'd also appreciate advice on how to print the integer types as something other than `_`.

bors · 2015-06-04T01:02:42Z

⌛ Testing commit 25d0ef3 with merge 06c6b3c...

bors · 2015-06-04T04:11:23Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt

rust-highfive assigned nrc May 24, 2015

nrc reviewed May 28, 2015
View reviewed changes

michaelsproul reviewed May 29, 2015
View reviewed changes

nrc reviewed Jun 1, 2015
View reviewed changes

nikomatsakis reviewed Jun 2, 2015
View reviewed changes

Improve diagnostic messages for range patterns.

25d0ef3

michaelsproul reviewed Jun 3, 2015
View reviewed changes

bors merged commit 25d0ef3 into rust-lang:master Jun 4, 2015

michaelsproul deleted the match-diagnostics branch June 7, 2015 01:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve diagnostic messages for range patterns. #25743

Improve diagnostic messages for range patterns. #25743

michaelsproul commented May 24, 2015

rust-highfive commented May 24, 2015

bors commented May 26, 2015

nrc commented May 28, 2015

nrc May 28, 2015

michaelsproul commented May 29, 2015

michaelsproul May 29, 2015

michaelsproul commented May 31, 2015

nrc Jun 1, 2015

nrc commented Jun 2, 2015

michaelsproul commented Jun 2, 2015

nikomatsakis Jun 2, 2015

michaelsproul Jun 3, 2015

nikomatsakis Jun 3, 2015

nrc commented Jun 3, 2015

bors commented Jun 3, 2015

bors commented Jun 4, 2015

bors commented Jun 4, 2015

		@@ -8,9 +8,17 @@
		// option. This file may not be copied, modified, or distributed
		// except according to those terms.

Improve diagnostic messages for range patterns. #25743

Improve diagnostic messages for range patterns. #25743

Conversation

michaelsproul commented May 24, 2015

rust-highfive commented May 24, 2015

bors commented May 26, 2015

nrc commented May 28, 2015

nrc May 28, 2015

Choose a reason for hiding this comment

michaelsproul commented May 29, 2015

michaelsproul May 29, 2015

Choose a reason for hiding this comment

michaelsproul commented May 31, 2015

nrc Jun 1, 2015

Choose a reason for hiding this comment

nrc commented Jun 2, 2015

michaelsproul commented Jun 2, 2015

nikomatsakis Jun 2, 2015

Choose a reason for hiding this comment

michaelsproul Jun 3, 2015

Choose a reason for hiding this comment

nikomatsakis Jun 3, 2015

Choose a reason for hiding this comment

nrc commented Jun 3, 2015

bors commented Jun 3, 2015

bors commented Jun 4, 2015

bors commented Jun 4, 2015