auto merge of #14444 : huonw/rust/nice-for-errors, r=alexcrichton
Change `for` desugaring & make refutable pattern errors more precise This changes for to desugar to the `let`-based pattern match as described in #14390, and adjusts the compiler to use this information for error messages that even mention that it's in a `for` loop. Also, it makes the compiler record the exact positions of refutable parts of a pattern, to point to exactly them in error messages.
This commit is contained in:
commit
1fc29ef0c8
@ -863,9 +863,18 @@ fn default(cx: &MatchCheckCtxt, r: &[@Pat]) -> Option<Vec<@Pat> > {
|
||||
|
||||
fn check_local(cx: &mut MatchCheckCtxt, loc: &Local) {
|
||||
visit::walk_local(cx, loc, ());
|
||||
if is_refutable(cx, loc.pat) {
|
||||
cx.tcx.sess.span_err(loc.pat.span,
|
||||
"refutable pattern in local binding");
|
||||
|
||||
let name = match loc.source {
|
||||
LocalLet => "local",
|
||||
LocalFor => "`for` loop"
|
||||
};
|
||||
|
||||
let mut spans = vec![];
|
||||
find_refutable(cx, loc.pat, &mut spans);
|
||||
|
||||
for span in spans.iter() {
|
||||
cx.tcx.sess.span_err(*span,
|
||||
format!("refutable pattern in {} binding", name).as_slice());
|
||||
}
|
||||
|
||||
// Check legality of move bindings.
|
||||
@ -879,53 +888,65 @@ fn check_fn(cx: &mut MatchCheckCtxt,
|
||||
sp: Span) {
|
||||
visit::walk_fn(cx, kind, decl, body, sp, ());
|
||||
for input in decl.inputs.iter() {
|
||||
if is_refutable(cx, input.pat) {
|
||||
cx.tcx.sess.span_err(input.pat.span,
|
||||
let mut spans = vec![];
|
||||
find_refutable(cx, input.pat, &mut spans);
|
||||
|
||||
for span in spans.iter() {
|
||||
cx.tcx.sess.span_err(*span,
|
||||
"refutable pattern in function argument");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_refutable(cx: &MatchCheckCtxt, pat: &Pat) -> bool {
|
||||
fn find_refutable(cx: &MatchCheckCtxt, pat: &Pat, spans: &mut Vec<Span>) {
|
||||
macro_rules! this_pattern {
|
||||
() => {
|
||||
{
|
||||
spans.push(pat.span);
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
let opt_def = cx.tcx.def_map.borrow().find_copy(&pat.id);
|
||||
match opt_def {
|
||||
Some(DefVariant(enum_id, _, _)) => {
|
||||
if ty::enum_variants(cx.tcx, enum_id).len() != 1u {
|
||||
return true;
|
||||
this_pattern!()
|
||||
}
|
||||
}
|
||||
Some(DefStatic(..)) => return true,
|
||||
Some(DefStatic(..)) => this_pattern!(),
|
||||
_ => ()
|
||||
}
|
||||
|
||||
match pat.node {
|
||||
PatUniq(sub) | PatRegion(sub) | PatIdent(_, _, Some(sub)) => {
|
||||
is_refutable(cx, sub)
|
||||
find_refutable(cx, sub, spans)
|
||||
}
|
||||
PatWild | PatWildMulti | PatIdent(_, _, None) => { false }
|
||||
PatWild | PatWildMulti | PatIdent(_, _, None) => {}
|
||||
PatLit(lit) => {
|
||||
match lit.node {
|
||||
ExprLit(lit) => {
|
||||
match lit.node {
|
||||
LitNil => false, // `()`
|
||||
_ => true,
|
||||
LitNil => {} // `()`
|
||||
_ => this_pattern!(),
|
||||
}
|
||||
}
|
||||
_ => true,
|
||||
_ => this_pattern!(),
|
||||
}
|
||||
}
|
||||
PatRange(_, _) => { true }
|
||||
PatRange(_, _) => { this_pattern!() }
|
||||
PatStruct(_, ref fields, _) => {
|
||||
fields.iter().any(|f| is_refutable(cx, f.pat))
|
||||
for f in fields.iter() {
|
||||
find_refutable(cx, f.pat, spans);
|
||||
}
|
||||
}
|
||||
PatTup(ref elts) => {
|
||||
elts.iter().any(|elt| is_refutable(cx, *elt))
|
||||
PatTup(ref elts) | PatEnum(_, Some(ref elts))=> {
|
||||
for elt in elts.iter() {
|
||||
find_refutable(cx, *elt, spans)
|
||||
}
|
||||
}
|
||||
PatEnum(_, Some(ref args)) => {
|
||||
args.iter().any(|a| is_refutable(cx, *a))
|
||||
}
|
||||
PatEnum(_,_) => { false }
|
||||
PatVec(..) => { true }
|
||||
PatEnum(_,_) => {}
|
||||
PatVec(..) => { this_pattern!() }
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -417,6 +417,14 @@ pub enum Stmt_ {
|
||||
StmtMac(Mac, bool),
|
||||
}
|
||||
|
||||
/// Where a local declaration came from: either a true `let ... =
|
||||
/// ...;`, or one desugared from the pattern of a for loop.
|
||||
#[deriving(Clone, Eq, TotalEq, Encodable, Decodable, Hash)]
|
||||
pub enum LocalSource {
|
||||
LocalLet,
|
||||
LocalFor,
|
||||
}
|
||||
|
||||
// FIXME (pending discussion of #1697, #2178...): local should really be
|
||||
// a refinement on pat.
|
||||
/// Local represents a `let` statement, e.g., `let <pat>:<ty> = <expr>;`
|
||||
@ -427,6 +435,7 @@ pub struct Local {
|
||||
pub init: Option<@Expr>,
|
||||
pub id: NodeId,
|
||||
pub span: Span,
|
||||
pub source: LocalSource,
|
||||
}
|
||||
|
||||
pub type Decl = Spanned<Decl_>;
|
||||
|
@ -439,6 +439,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
|
||||
init: Some(ex),
|
||||
id: ast::DUMMY_NODE_ID,
|
||||
span: sp,
|
||||
source: ast::LocalLet,
|
||||
};
|
||||
let decl = respan(sp, ast::DeclLocal(local));
|
||||
@respan(sp, ast::StmtDecl(@decl, ast::DUMMY_NODE_ID))
|
||||
@ -462,6 +463,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
|
||||
init: Some(ex),
|
||||
id: ast::DUMMY_NODE_ID,
|
||||
span: sp,
|
||||
source: ast::LocalLet,
|
||||
};
|
||||
let decl = respan(sp, ast::DeclLocal(local));
|
||||
@respan(sp, ast::StmtDecl(@decl, ast::DUMMY_NODE_ID))
|
||||
|
@ -147,11 +147,17 @@ pub fn expand_expr(e: @ast::Expr, fld: &mut MacroExpander) -> @ast::Expr {
|
||||
// ['<ident>:] loop {
|
||||
// match i.next() {
|
||||
// None => break,
|
||||
// Some(<src_pat>) => <src_loop_block>
|
||||
// Some(mut value) => {
|
||||
// let <src_pat> = value;
|
||||
// <src_loop_block>
|
||||
// }
|
||||
// }
|
||||
// }
|
||||
// }
|
||||
// }
|
||||
//
|
||||
// (The use of the `let` is to give better error messages
|
||||
// when the pattern is refutable.)
|
||||
|
||||
let local_ident = token::gensym_ident("__i"); // FIXME #13573
|
||||
let next_ident = fld.cx.ident_of("next");
|
||||
@ -167,11 +173,33 @@ pub fn expand_expr(e: @ast::Expr, fld: &mut MacroExpander) -> @ast::Expr {
|
||||
fld.cx.arm(span, vec!(none_pat), break_expr)
|
||||
};
|
||||
|
||||
// `Some(<src_pat>) => <src_loop_block>`
|
||||
// let <src_pat> = value;
|
||||
let value_ident = token::gensym_ident("__value");
|
||||
// this is careful to use src_pat.span so that error
|
||||
// messages point exact at that.
|
||||
let local = @ast::Local {
|
||||
ty: fld.cx.ty_infer(src_pat.span),
|
||||
pat: src_pat,
|
||||
init: Some(fld.cx.expr_ident(src_pat.span, value_ident)),
|
||||
id: ast::DUMMY_NODE_ID,
|
||||
span: src_pat.span,
|
||||
source: ast::LocalFor
|
||||
};
|
||||
let local = codemap::respan(src_pat.span, ast::DeclLocal(local));
|
||||
let local = @codemap::respan(span, ast::StmtDecl(@local, ast::DUMMY_NODE_ID));
|
||||
|
||||
// { let ...; <src_loop_block> }
|
||||
let block = fld.cx.block(span, vec![local],
|
||||
Some(fld.cx.expr_block(src_loop_block)));
|
||||
|
||||
// `Some(mut value) => { ... }`
|
||||
// Note the _'s in the name will stop any unused mutability warnings.
|
||||
let value_pat = fld.cx.pat_ident_binding_mode(span, value_ident,
|
||||
ast::BindByValue(ast::MutMutable));
|
||||
let some_arm =
|
||||
fld.cx.arm(span,
|
||||
vec!(fld.cx.pat_enum(span, some_path, vec!(src_pat))),
|
||||
fld.cx.expr_block(src_loop_block));
|
||||
vec!(fld.cx.pat_enum(span, some_path, vec!(value_pat))),
|
||||
fld.cx.expr_block(block));
|
||||
|
||||
// `match i.next() { ... }`
|
||||
let match_expr = {
|
||||
@ -669,7 +697,8 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander)
|
||||
pat: pat,
|
||||
init: init,
|
||||
id: id,
|
||||
span: span
|
||||
span: span,
|
||||
source: source,
|
||||
} = **local;
|
||||
// expand the pat (it might contain exprs... #:(o)>
|
||||
let expanded_pat = fld.fold_pat(pat);
|
||||
@ -703,6 +732,7 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander)
|
||||
init: new_init_opt,
|
||||
id: id,
|
||||
span: span,
|
||||
source: source
|
||||
};
|
||||
SmallVector::one(@Spanned {
|
||||
node: StmtDecl(@Spanned {
|
||||
|
@ -288,6 +288,7 @@ pub trait Folder {
|
||||
pat: self.fold_pat(l.pat),
|
||||
init: l.init.map(|e| self.fold_expr(e)),
|
||||
span: self.new_span(l.span),
|
||||
source: l.source,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -34,7 +34,7 @@ use ast::{Ident, NormalFn, Inherited, Item, Item_, ItemStatic};
|
||||
use ast::{ItemEnum, ItemFn, ItemForeignMod, ItemImpl};
|
||||
use ast::{ItemMac, ItemMod, ItemStruct, ItemTrait, ItemTy, Lit, Lit_};
|
||||
use ast::{LitBool, LitFloat, LitFloatUnsuffixed, LitInt, LitChar};
|
||||
use ast::{LitIntUnsuffixed, LitNil, LitStr, LitUint, Local};
|
||||
use ast::{LitIntUnsuffixed, LitNil, LitStr, LitUint, Local, LocalLet};
|
||||
use ast::{MutImmutable, MutMutable, Mac_, MacInvocTT, Matcher, MatchNonterminal};
|
||||
use ast::{MatchSeq, MatchTok, Method, MutTy, BiMul, Mutability};
|
||||
use ast::{NamedField, UnNeg, NoReturn, UnNot, P, Pat, PatEnum};
|
||||
@ -3034,6 +3034,7 @@ impl<'a> Parser<'a> {
|
||||
init: init,
|
||||
id: ast::DUMMY_NODE_ID,
|
||||
span: mk_sp(lo, self.last_span.hi),
|
||||
source: LocalLet,
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -0,0 +1,16 @@
|
||||
// Copyright 2014 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.
|
||||
|
||||
|
||||
fn main() {
|
||||
for
|
||||
&1 //~ ERROR refutable pattern in `for` loop binding
|
||||
in [1].iter() {}
|
||||
}
|
32
src/test/compile-fail/precise-refutable-pattern-errors.rs
Normal file
32
src/test/compile-fail/precise-refutable-pattern-errors.rs
Normal file
@ -0,0 +1,32 @@
|
||||
// Copyright 2014 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.
|
||||
|
||||
|
||||
fn func(
|
||||
(
|
||||
1, //~ ERROR refutable pattern in function argument
|
||||
(
|
||||
Some( //~ ERROR refutable pattern in function argument
|
||||
1), // nested, so no warning.
|
||||
2..3 //~ ERROR refutable pattern in function argument
|
||||
)
|
||||
): (int, (Option<int>, int))
|
||||
) {}
|
||||
|
||||
fn main() {
|
||||
let (
|
||||
1, //~ ERROR refutable pattern in local binding
|
||||
(
|
||||
Some( //~ ERROR refutable pattern in local binding
|
||||
1), // nested, so no warning.
|
||||
2..3 //~ ERROR refutable pattern in local binding
|
||||
)
|
||||
) = (1, (None, 2));
|
||||
}
|
Loading…
Reference in New Issue
Block a user