Skip to content

Commit

Permalink
Improve diagnostic messages for range patterns.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelsproul committed Jun 3, 2015
1 parent 448ce12 commit 25d0ef3
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 71 deletions.
15 changes: 5 additions & 10 deletions src/compiletest/runtest.rs
Expand Up @@ -24,7 +24,6 @@ use std::fmt;
use std::fs::{self, File};
use std::io::BufReader;
use std::io::prelude::*;
use std::iter::repeat;
use std::net::TcpStream;
use std::path::{Path, PathBuf};
use std::process::{Command, Output, ExitStatus};
Expand Down Expand Up @@ -928,12 +927,12 @@ fn check_forbid_output(props: &TestProps,
}
}

fn check_expected_errors(expected_errors: Vec<errors::ExpectedError> ,
fn check_expected_errors(expected_errors: Vec<errors::ExpectedError>,
testfile: &Path,
proc_res: &ProcRes) {

// true if we found the error in question
let mut found_flags: Vec<_> = repeat(false).take(expected_errors.len()).collect();
let mut found_flags = vec![false; expected_errors.len()];

if proc_res.status.success() {
fatal("process did not return an error status");
Expand All @@ -954,14 +953,10 @@ fn check_expected_errors(expected_errors: Vec<errors::ExpectedError> ,
}
}

// A multi-line error will have followup lines which will always
// start with one of these strings.
// A multi-line error will have followup lines which start with a space
// or open paren.
fn continuation( line: &str) -> bool {
line.starts_with(" expected") ||
line.starts_with(" found") ||
// 1234
// Should have 4 spaces: see issue 18946
line.starts_with("(")
line.starts_with(" ") || line.starts_with("(")
}

// Scan and extract our error/warning messages,
Expand Down
89 changes: 56 additions & 33 deletions src/librustc_typeck/check/_match.rs
Expand Up @@ -83,41 +83,64 @@ pub fn check_pat<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
demand::suptype(fcx, pat.span, expected, pat_ty);
}
ast::PatRange(ref begin, ref end) => {
check_expr(fcx, &**begin);
check_expr(fcx, &**end);

let lhs_ty = fcx.expr_ty(&**begin);
let rhs_ty = fcx.expr_ty(&**end);

let lhs_eq_rhs =
require_same_types(
tcx, Some(fcx.infcx()), false, pat.span, lhs_ty, rhs_ty,
|| "mismatched types in range".to_string());

let numeric_or_char =
lhs_eq_rhs && (ty::type_is_numeric(lhs_ty) || ty::type_is_char(lhs_ty));

if numeric_or_char {
match const_eval::compare_lit_exprs(tcx, &**begin, &**end, Some(lhs_ty),
|id| {fcx.item_substs()[&id].substs
.clone()}) {
Some(Ordering::Less) |
Some(Ordering::Equal) => {}
Some(Ordering::Greater) => {
span_err!(tcx.sess, begin.span, E0030,
"lower range bound must be less than upper");
}
None => {
span_err!(tcx.sess, begin.span, E0031,
"mismatched types in range");
}
}
} else {
span_err!(tcx.sess, begin.span, E0029,
"only char and numeric types are allowed in range");
check_expr(fcx, begin);
check_expr(fcx, end);

let lhs_ty = fcx.expr_ty(begin);
let rhs_ty = fcx.expr_ty(end);

// Check that both end-points are of numeric or char type.
let numeric_or_char = |t| ty::type_is_numeric(t) || ty::type_is_char(t);
let lhs_compat = numeric_or_char(lhs_ty);
let rhs_compat = numeric_or_char(rhs_ty);

if !lhs_compat || !rhs_compat {
let span = if !lhs_compat && !rhs_compat {
pat.span
} else if !lhs_compat {
begin.span
} else {
end.span
};

// Note: spacing here is intentional, we want a space before "start" and "end".
span_err!(tcx.sess, span, E0029,
"only char and numeric types are allowed in range patterns\n \
start type: {}\n end type: {}",
fcx.infcx().ty_to_string(lhs_ty),
fcx.infcx().ty_to_string(rhs_ty)
);
return;
}

// Check that the types of the end-points can be unified.
let types_unify = require_same_types(
tcx, Some(fcx.infcx()), false, pat.span, rhs_ty, lhs_ty,
|| "mismatched types in range".to_string()
);

// It's ok to return without a message as `require_same_types` prints an error.
if !types_unify {
return;
}

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

fcx.write_ty(pat.id, common_type);

// Finally we evaluate the constants and check that the range is non-empty.
let get_substs = |id| fcx.item_substs()[&id].substs.clone();
match const_eval::compare_lit_exprs(tcx, begin, end, Some(&common_type), get_substs) {
Some(Ordering::Less) |
Some(Ordering::Equal) => {}
Some(Ordering::Greater) => {
span_err!(tcx.sess, begin.span, E0030,
"lower range bound must be less than or equal to upper");
}
None => tcx.sess.span_bug(begin.span, "literals of different types in range pat")
}

// subtyping doesn't matter here, as the value is some kind of scalar
demand::eqtype(fcx, pat.span, expected, lhs_ty);
Expand Down
44 changes: 41 additions & 3 deletions src/librustc_typeck/diagnostics.rs
Expand Up @@ -146,6 +146,47 @@ match d {
```
"##,

E0029: r##"
In a match expression, only numbers and characters can be matched against a
range. This is because the compiler checks that the range is non-empty at
compile-time, and is unable to evaluate arbitrary comparison functions. If you
want to capture values of an orderable type between two end-points, you can use
a guard.
```
// The ordering relation for strings can't be evaluated at compile time,
// so this doesn't work:
match string {
"hello" ... "world" => ...
_ => ...
}
// This is a more general version, using a guard:
match string {
s if s >= "hello" && s <= "world" => ...
_ => ...
}
```
"##,

E0030: r##"
When matching against a range, the compiler verifies that the range is
non-empty. Range patterns include both end-points, so this is equivalent to
requiring the start of the range to be less than or equal to the end of the
range.
For example:
```
match 5u32 {
// This range is ok, albeit pointless.
1 ... 1 => ...
// This range is empty, and the compiler can tell.
1000 ... 5 => ...
}
```
"##,

E0033: r##"
This error indicates that a pointer to a trait type cannot be implicitly
dereferenced by a pattern. Every trait defines a type, but because the
Expand Down Expand Up @@ -1107,9 +1148,6 @@ For more information see the [opt-in builtin traits RFC](https://github.com/rust
}

register_diagnostics! {
E0029,
E0030,
E0031,
E0034, // multiple applicable methods in scope
E0035, // does not take type parameters
E0036, // incorrect number of type parameters given for this method
Expand Down
16 changes: 0 additions & 16 deletions src/test/compile-fail/match-ill-type1.rs

This file was deleted.

28 changes: 19 additions & 9 deletions src/test/compile-fail/match-range-fail.rs
Expand Up @@ -8,22 +8,32 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//error-pattern: lower range bound
//error-pattern: only char and numeric types
//error-pattern: mismatched types

fn main() {
match 5 {
6 ... 1 => { }
_ => { }
6 ... 1 => { }
_ => { }
};
//~^^^ ERROR lower range bound must be less than or equal to upper

match "wow" {
"bar" ... "foo" => { }
};
//~^^ ERROR only char and numeric types are allowed in range
//~| start type: &'static str
//~| end type: &'static str

match "wow" {
"bar" ... "foo" => { }
10 ... "what" => ()
};
//~^^ ERROR only char and numeric types are allowed in range
//~| start type: _
//~| end type: &'static str

match 5 {
'c' ... 100 => { }
_ => { }
'c' ... 100 => { }
_ => { }
};
//~^^^ ERROR mismatched types in range
//~| expected char
//~| found integral variable
}
26 changes: 26 additions & 0 deletions src/test/run-pass/match-range-infer.rs
@@ -0,0 +1,26 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test that type inference for range patterns works correctly (is bi-directional).

pub fn main() {
match 1 {
1 ... 3 => {}
_ => panic!("should match range")
}
match 1 {
1 ... 3u16 => {}
_ => panic!("should match range with inferred start type")
}
match 1 {
1u16 ... 3 => {}
_ => panic!("should match range with inferred end type")
}
}

0 comments on commit 25d0ef3

Please sign in to comment.