diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index fb797a027956f..9b751447504ef 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -863,9 +863,18 @@ fn default(cx: &MatchCheckCtxt, r: &[@Pat]) -> Option > { 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) { + 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)) - } - PatTup(ref elts) => { - elts.iter().any(|elt| is_refutable(cx, *elt)) + for f in fields.iter() { + find_refutable(cx, f.pat, spans); + } } - PatEnum(_, Some(ref args)) => { - args.iter().any(|a| is_refutable(cx, *a)) + PatTup(ref elts) | PatEnum(_, Some(ref elts))=> { + for elt in elts.iter() { + find_refutable(cx, *elt, spans) + } } - PatEnum(_,_) => { false } - PatVec(..) => { true } + PatEnum(_,_) => {} + PatVec(..) => { this_pattern!() } } } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index e77d1faf05d89..69a92a871855c 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -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 : = ;` @@ -427,6 +435,7 @@ pub struct Local { pub init: Option<@Expr>, pub id: NodeId, pub span: Span, + pub source: LocalSource, } pub type Decl = Spanned; diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 449feb3afbf96..bb7b73c5f81f2 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -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)) diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 6c6bf52010406..b8766200ccd00 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -147,11 +147,17 @@ pub fn expand_expr(e: @ast::Expr, fld: &mut MacroExpander) -> @ast::Expr { // [':] loop { // match i.next() { // None => break, - // Some() => + // Some(mut value) => { + // let = value; + // + // } // } // } // } // } + // + // (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() => ` + // let = 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 ...; } + 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 { diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index ae5cf550bb9bc..6a9f26ae83d68 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -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, } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ae5f16c2580e2..65687ce4d9447 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -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, } } diff --git a/src/test/compile-fail/for-loop-refutable-pattern-error-message.rs b/src/test/compile-fail/for-loop-refutable-pattern-error-message.rs new file mode 100644 index 0000000000000..8b00b61490993 --- /dev/null +++ b/src/test/compile-fail/for-loop-refutable-pattern-error-message.rs @@ -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 or the MIT license +// , 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() {} +} diff --git a/src/test/compile-fail/precise-refutable-pattern-errors.rs b/src/test/compile-fail/precise-refutable-pattern-errors.rs new file mode 100644 index 0000000000000..efa2dbad83fda --- /dev/null +++ b/src/test/compile-fail/precise-refutable-pattern-errors.rs @@ -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 or the MIT license +// , 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)) + ) {} + +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)); +}