From 5d8cfd53b513d999ffff22f17e3066a30a8ed949 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 24 Aug 2014 18:04:29 -0700 Subject: [PATCH 1/9] Teach libsyntax about `if let` --- src/libsyntax/ast.rs | 1 + src/libsyntax/fold.rs | 6 ++++++ src/libsyntax/parse/classify.rs | 1 + src/libsyntax/parse/parser.rs | 35 ++++++++++++++++++++++++--------- src/libsyntax/print/pprust.rs | 33 ++++++++++++++++++++++++++++--- src/libsyntax/visit.rs | 8 +++++++- 6 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 0fee3ff321850..02922fe3655ba 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -521,6 +521,7 @@ pub enum Expr_ { ExprLit(P), ExprCast(P, P), ExprIf(P, P, Option>), + ExprIfLet(P, P, P, Option>), // FIXME #6993: change to Option ... or not, if these are hygienic. ExprWhile(P, P, Option), // FIXME #6993: change to Option ... or not, if these are hygienic. diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 53be7f2c20c4e..8bdc64a926df8 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1205,6 +1205,12 @@ pub fn noop_fold_expr(Expr {id, node, span}: Expr, folder: &mut T) -> folder.fold_block(tr), fl.map(|x| folder.fold_expr(x))) } + ExprIfLet(pat, expr, tr, fl) => { + ExprIfLet(folder.fold_pat(pat), + folder.fold_expr(expr), + folder.fold_block(tr), + fl.map(|x| folder.fold_expr(x))) + } ExprWhile(cond, body, opt_ident) => { ExprWhile(folder.fold_expr(cond), folder.fold_block(body), diff --git a/src/libsyntax/parse/classify.rs b/src/libsyntax/parse/classify.rs index cdd221aca7cf0..cb57318445e41 100644 --- a/src/libsyntax/parse/classify.rs +++ b/src/libsyntax/parse/classify.rs @@ -24,6 +24,7 @@ use ast; pub fn expr_requires_semi_to_be_stmt(e: &ast::Expr) -> bool { match e.node { ast::ExprIf(..) + | ast::ExprIfLet(..) | ast::ExprMatch(..) | ast::ExprBlock(_) | ast::ExprWhile(..) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 415ff6a4097ac..d4ba81c737bce 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -23,7 +23,7 @@ use ast::{DeclLocal, DefaultBlock, UnDeref, BiDiv, EMPTY_CTXT, EnumDef, Explicit use ast::{Expr, Expr_, ExprAddrOf, ExprMatch, ExprAgain}; use ast::{ExprAssign, ExprAssignOp, ExprBinary, ExprBlock, ExprBox}; use ast::{ExprBreak, ExprCall, ExprCast}; -use ast::{ExprField, ExprTupField, ExprFnBlock, ExprIf, ExprIndex, ExprSlice}; +use ast::{ExprField, ExprTupField, ExprFnBlock, ExprIf, ExprIfLet, ExprIndex, ExprSlice}; use ast::{ExprLit, ExprLoop, ExprMac}; use ast::{ExprMethodCall, ExprParen, ExprPath, ExprProc}; use ast::{ExprRepeat, ExprRet, ExprStruct, ExprTup, ExprUnary, ExprUnboxedFn}; @@ -576,13 +576,10 @@ impl<'a> Parser<'a> { /// If the next token is the given keyword, eat it and return /// true. Otherwise, return false. pub fn eat_keyword(&mut self, kw: keywords::Keyword) -> bool { - match self.token { - token::IDENT(sid, false) if kw.to_name() == sid.name => { - self.bump(); - true - } - _ => false - } + if self.is_keyword(kw) { + self.bump(); + true + } else { false } } /// If the given word is not a keyword, signal an error. @@ -2860,8 +2857,11 @@ impl<'a> Parser<'a> { } } - /// Parse an 'if' expression ('if' token already eaten) + /// Parse an 'if' or 'if let' expression ('if' token already eaten) pub fn parse_if_expr(&mut self) -> P { + if self.is_keyword(keywords::Let) { + return self.parse_if_let_expr(); + } let lo = self.last_span.lo; let cond = self.parse_expr_res(RestrictionNoStructLiteral); let thn = self.parse_block(); @@ -2875,6 +2875,23 @@ impl<'a> Parser<'a> { self.mk_expr(lo, hi, ExprIf(cond, thn, els)) } + /// Parse an 'if let' expression ('if' token already eaten) + pub fn parse_if_let_expr(&mut self) -> P { + let lo = self.last_span.lo; + self.expect_keyword(keywords::Let); + let pat = self.parse_pat(); + self.expect(&token::EQ); + let expr = self.parse_expr_res(RestrictionNoStructLiteral); + let thn = self.parse_block(); + let (hi, els) = if self.eat_keyword(keywords::Else) { + let expr = self.parse_else_expr(); + (expr.span.hi, Some(expr)) + } else { + (thn.span.hi, None) + }; + self.mk_expr(lo, hi, ExprIfLet(pat, expr, thn, els)) + } + // `|args| expr` pub fn parse_lambda_expr(&mut self, capture_clause: CaptureClause) -> P { diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index ae4ba611bab53..1e5810fb311d9 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1307,6 +1307,19 @@ impl<'a> State<'a> { try!(self.print_block(&**then)); self.print_else(e.as_ref().map(|e| &**e)) } + // "another else-if-let" + ast::ExprIfLet(ref pat, ref expr, ref then, ref e) => { + try!(self.cbox(indent_unit - 1u)); + try!(self.ibox(0u)); + try!(word(&mut self.s, " else if let ")); + try!(self.print_pat(&**pat)); + try!(space(&mut self.s)); + try!(self.word_space("=")); + try!(self.print_expr(&**expr)); + try!(space(&mut self.s)); + try!(self.print_block(&**then)); + self.print_else(e.as_ref().map(|e| &**e)) + } // "final else" ast::ExprBlock(ref b) => { try!(self.cbox(indent_unit - 1u)); @@ -1325,15 +1338,26 @@ impl<'a> State<'a> { } pub fn print_if(&mut self, test: &ast::Expr, blk: &ast::Block, - elseopt: Option<&ast::Expr>, chk: bool) -> IoResult<()> { + elseopt: Option<&ast::Expr>) -> IoResult<()> { try!(self.head("if")); - if chk { try!(self.word_nbsp("check")); } try!(self.print_expr(test)); try!(space(&mut self.s)); try!(self.print_block(blk)); self.print_else(elseopt) } + pub fn print_if_let(&mut self, pat: &ast::Pat, expr: &ast::Expr, blk: &ast::Block, + elseopt: Option<&ast::Expr>) -> IoResult<()> { + try!(self.head("if let")); + try!(self.print_pat(pat)); + try!(space(&mut self.s)); + try!(self.word_space("=")); + try!(self.print_expr(expr)); + try!(space(&mut self.s)); + try!(self.print_block(blk)); + self.print_else(elseopt) + } + pub fn print_mac(&mut self, m: &ast::Mac) -> IoResult<()> { match m.node { // I think it's reasonable to hide the ctxt here: @@ -1474,7 +1498,10 @@ impl<'a> State<'a> { try!(self.print_type(&**ty)); } ast::ExprIf(ref test, ref blk, ref elseopt) => { - try!(self.print_if(&**test, &**blk, elseopt.as_ref().map(|e| &**e), false)); + try!(self.print_if(&**test, &**blk, elseopt.as_ref().map(|e| &**e))); + } + ast::ExprIfLet(ref pat, ref expr, ref blk, ref elseopt) => { + try!(self.print_if_let(&**pat, &**expr, &** blk, elseopt.as_ref().map(|e| &**e))); } ast::ExprWhile(ref test, ref blk, opt_ident) => { for ident in opt_ident.iter() { diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index 3b2ed30b76d89..6fc79e2c42ab9 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -730,13 +730,19 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) { visitor.visit_expr(&**subexpression); visitor.visit_block(&**block) } + ExprIfLet(ref pattern, ref subexpression, ref if_block, ref optional_else) => { + visitor.visit_pat(&**pattern); + visitor.visit_expr(&**subexpression); + visitor.visit_block(&**if_block); + walk_expr_opt(visitor, optional_else); + } ExprForLoop(ref pattern, ref subexpression, ref block, _) => { visitor.visit_pat(&**pattern); visitor.visit_expr(&**subexpression); visitor.visit_block(&**block) } ExprLoop(ref block, _) => visitor.visit_block(&**block), - ExprMatch(ref subexpression, ref arms) => { + ExprMatch(ref subexpression, ref arms, _) => { visitor.visit_expr(&**subexpression); for arm in arms.iter() { visitor.visit_arm(arm) From 0e6ff432dc83e5675163b1c29ce73dbc5e18ba24 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 24 Aug 2014 19:08:48 -0700 Subject: [PATCH 2/9] Desugar 'if let' into the appropriate 'match' --- src/librustc/middle/cfg/construct.rs | 2 + src/librustc/middle/expr_use_visitor.rs | 2 + src/librustc/middle/liveness.rs | 4 + src/librustc/middle/mem_categorization.rs | 2 + src/librustc/middle/trans/debuginfo.rs | 5 ++ src/librustc/middle/ty.rs | 1 + src/librustc/middle/typeck/check/mod.rs | 1 + src/librustc_back/svh.rs | 1 + src/libsyntax/ext/expand.rs | 91 ++++++++++++++++++++++- 9 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/librustc/middle/cfg/construct.rs b/src/librustc/middle/cfg/construct.rs index b268c2a7a518b..1f0171de351ee 100644 --- a/src/librustc/middle/cfg/construct.rs +++ b/src/librustc/middle/cfg/construct.rs @@ -222,6 +222,8 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { self.add_node(expr.id, [then_exit, else_exit]) // 4, 5 } + ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), + ast::ExprWhile(ref cond, ref body, _) => { // // [pred] diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 81994ee64a8cf..875416feb2e4e 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -374,6 +374,8 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,TYPER> { } } + ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), + ast::ExprMatch(ref discr, ref arms) => { let discr_cmt = return_if_err!(self.mc.cat_expr(&**discr)); self.borrow_expr(&**discr, ty::ReEmpty, ty::ImmBorrow, MatchDiscriminant); diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index b6893a6a3b419..061f110afa906 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -481,6 +481,7 @@ fn visit_expr(ir: &mut IrMaps, expr: &Expr) { ir.add_live_node_for_node(expr.id, ExprNode(expr.span)); visit::walk_expr(ir, expr); } + ExprIfLet(..) => fail!("non-desugared ExprIfLet"), ExprForLoop(ref pat, _, _, _) => { pat_util::pat_bindings(&ir.tcx.def_map, &**pat, |bm, p_id, sp, path1| { debug!("adding local variable {} from for loop with bm {:?}", @@ -1011,6 +1012,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { self.propagate_through_expr(&**cond, ln) } + ExprIfLet(..) => fail!("non-desugared ExprIfLet"), + ExprWhile(ref cond, ref blk, _) => { self.propagate_through_loop(expr, WhileLoop(&**cond), &**blk, succ) } @@ -1470,6 +1473,7 @@ fn check_expr(this: &mut Liveness, expr: &Expr) { ExprPath(..) | ExprBox(..) => { visit::walk_expr(this, expr); } + ExprIfLet(..) => fail!("non-desugared ExprIfLet") } } diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 3b831dd6847a1..d5ada0c5411a3 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -505,6 +505,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { ast::ExprForLoop(..) => { Ok(self.cat_rvalue_node(expr.id(), expr.span(), expr_ty)) } + + ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet") } } diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 7a0e5aea7fffc..5221faa598c66 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -3576,6 +3576,11 @@ fn populate_scope_map(cx: &CrateContext, } } + ast::ExprIfLet(..) => { + cx.sess().span_bug(exp.span, "debuginfo::populate_scope_map() - \ + Found unexpanded if-let."); + } + ast::ExprWhile(ref cond_exp, ref loop_body, _) => { walk_expr(cx, &**cond_exp, scope_stack, scope_map); diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 875a79373a69d..b55c3039ac8e3 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -3634,6 +3634,7 @@ pub fn expr_kind(tcx: &ctxt, expr: &ast::Expr) -> ExprKind { ast::ExprLit(ref lit) if lit_is_str(&**lit) => { RvalueDpsExpr } + ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), ast::ExprCast(..) => { match tcx.node_types.borrow().find(&(expr.id as uint)) { diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 0bf22d97345cf..566203834080b 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -4106,6 +4106,7 @@ fn check_expr_with_unifier(fcx: &FnCtxt, check_then_else(fcx, &**cond, &**then_blk, opt_else_expr.as_ref().map(|e| &**e), id, expr.span, expected); } + ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), ast::ExprWhile(ref cond, ref body, _) => { check_expr_has_type(fcx, &**cond, ty::mk_bool()); check_block_no_value(fcx, &**body); diff --git a/src/librustc_back/svh.rs b/src/librustc_back/svh.rs index f98a2dac0841c..394833372a62a 100644 --- a/src/librustc_back/svh.rs +++ b/src/librustc_back/svh.rs @@ -293,6 +293,7 @@ mod svh_visitor { ExprForLoop(..) => SawExprForLoop, // just syntactic artifacts, expanded away by time of SVH. + ExprIfLet(..) => unreachable!(), ExprMac(..) => unreachable!(), } } diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 9f3df1a762398..341be15b739fa 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -39,7 +39,7 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { e.and_then(|ast::Expr {id, node, span}| match node { // expr_mac should really be expr_ext or something; it's the // entry-point for all syntax extensions. - ExprMac(mac) => { + ast::ExprMac(mac) => { let expanded_expr = match expand_mac_invoc(mac, span, |r| r.make_expr(), mark_expr, fld) { @@ -67,6 +67,95 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { fld.cx.expr(span, ast::ExprWhile(cond, body, opt_ident)) } + // Desugar ExprIfLet + // From: `if let = []` + ast::ExprIfLet(pat, expr, body, mut elseopt) => { + let span = e.span; + + // to: + // + // match { + // => , + // [_ if => ,] + // _ => [ | ()] + // } + + // ` => ` + let pat_arm = { + let body_expr = fld.cx.expr_block(body); + fld.cx.arm(pat.span, vec![pat], body_expr) + }; + + // `[_ if => ,]` + let else_if_arms = { + let mut arms = vec![]; + loop { + // NOTE: replace with 'if let' after snapshot + match elseopt { + Some(els) => match els.node { + // else if + ast::ExprIf(cond, then, elseopt_) => { + let pat_under = fld.cx.pat_wild(span); + elseopt = elseopt_; + arms.push(ast::Arm { + attrs: vec![], + pats: vec![pat_under], + guard: Some(cond), + body: fld.cx.expr_block(then) + }); + } + _ => break + }, + None => break + } + } + arms + }; + + // `_ => [ | ()]` + let else_arm = { + let pat_under = fld.cx.pat_wild(span); + let else_expr = match elseopt { + Some(els) => els, + None => fld.cx.expr_lit(span, ast::LitNil) + }; + fld.cx.arm(span, vec![pat_under], else_expr) + }; + + let mut arms = Vec::with_capacity(else_if_arms.len() + 2); + arms.push(pat_arm); + arms.push_all_move(else_if_arms); + arms.push(else_arm); + + let match_expr = fld.cx.expr_match(span, expr, arms); + fld.fold_expr(match_expr) + } + + // Desugar support for ExprIfLet in the ExprIf else position + ast::ExprIf(cond, blk, mut elseopt) => { + // NOTE: replace with 'if let' after snapshot + match elseopt { + Some(els) => match els.node { + ast::ExprIfLet(..) => { + // wrap the if-let expr in a block + let blk = P(ast::Block { + view_items: vec![], + stmts: vec![], + expr: Some(els), + id: ast::DUMMY_NODE_ID, + rules: ast::DefaultBlock, + span: els.span + }); + elseopt = Some(fld.cx.expr_block(blk)); + } + _ => () + }, + None => () + }; + let if_expr = fld.cx.expr(e.span, ast::ExprIf(cond, blk, elseopt)); + noop_fold_expr(if_expr, fld) + } + ast::ExprLoop(loop_block, opt_ident) => { let (loop_block, opt_ident) = expand_loop_block(loop_block, opt_ident, fld); fld.cx.expr(span, ast::ExprLoop(loop_block, opt_ident)) From 1bc407fb8413b6e7f05a9d38b0d0a770e67dde5a Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 24 Aug 2014 22:08:13 -0700 Subject: [PATCH 3/9] Add tests for `if let` --- src/test/run-pass/if-let.rs | 67 +++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 src/test/run-pass/if-let.rs diff --git a/src/test/run-pass/if-let.rs b/src/test/run-pass/if-let.rs new file mode 100644 index 0000000000000..a6886bf9850e2 --- /dev/null +++ b/src/test/run-pass/if-let.rs @@ -0,0 +1,67 @@ +// 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. + +pub fn main() { + let x = Some(3i); + if let Some(y) = x { + assert_eq!(y, 3i); + } else { + fail!("if-let failed"); + } + let mut worked = false; + if let Some(_) = x { + worked = true; + } + assert!(worked); + let clause: uint; + if let None = Some("test") { + clause = 1; + } else if 4u > 5 { + clause = 2; + } else if let Ok(()) = Err::<(),&'static str>("test") { + clause = 3; + } else { + clause = 4; + } + assert_eq!(clause, 4u); + + if 3i > 4 { + fail!("bad math"); + } else if let 1 = 2i { + fail!("bad pattern match"); + } + + enum Foo { + One, + Two(uint), + Three(String, int) + } + + let foo = Three("three".to_string(), 42i); + if let One = foo { + fail!("bad pattern match"); + } else if let Two(_x) = foo { + fail!("bad pattern match"); + } else if let Three(s, _) = foo { + assert_eq!(s.as_slice(), "three"); + } else { + fail!("bad else"); + } + + if false { + fail!("wat"); + } else if let a@Two(_) = Two(42u) { + if let Two(b) = a { + assert_eq!(b, 42u); + } else { + fail!("fail in nested if-let"); + } + } +} From 976438f78fdce8092430f4c81ca272293c48f1a0 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Mon, 25 Aug 2014 14:55:00 -0700 Subject: [PATCH 4/9] Produce a better error for irrefutable `if let` patterns Modify ast::ExprMatch to include a new value of type ast::MatchSource, making it easy to tell whether the match was written literally or produced via desugaring. This allows us to customize error messages appropriately. --- src/librustc/diagnostics.rs | 5 ++ src/librustc/lint/builtin.rs | 7 ++- src/librustc/middle/cfg/construct.rs | 2 +- src/librustc/middle/check_match.rs | 26 +++++++-- src/librustc/middle/expr_use_visitor.rs | 2 +- src/librustc/middle/liveness.rs | 2 +- src/librustc/middle/trans/debuginfo.rs | 2 +- src/librustc/middle/trans/expr.rs | 2 +- src/librustc/middle/typeck/check/mod.rs | 2 +- src/librustc/middle/typeck/check/regionck.rs | 2 +- src/librustc/util/ppaux.rs | 1 + src/libsyntax/ast.rs | 8 ++- src/libsyntax/config.rs | 4 +- src/libsyntax/ext/build.rs | 2 +- src/libsyntax/ext/expand.rs | 2 +- src/libsyntax/fold.rs | 5 +- src/libsyntax/parse/parser.rs | 4 +- src/libsyntax/print/pprust.rs | 2 +- src/test/compile-fail/if-let.rs | 57 +++++++++++++++++++ .../compile-fail/lint-unnecessary-parens.rs | 1 + 20 files changed, 115 insertions(+), 23 deletions(-) create mode 100644 src/test/compile-fail/if-let.rs diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 36ba28dfc2a80..10fff4db8d61b 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -19,6 +19,11 @@ register_diagnostic!(E0001, r##" one is too specific or the ordering is incorrect. "##) +register_diagnostic!(E0162, r##" + This error is produced by an `if let` expression where the pattern is irrefutable. + An `if let` that can never fail is considered an error. +"##) + register_diagnostics!( E0002, E0003, diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index b812073c3a82a..b1cd431818cb8 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -1091,7 +1091,10 @@ impl LintPass for UnnecessaryParens { let (value, msg, struct_lit_needs_parens) = match e.node { ast::ExprIf(ref cond, _, _) => (cond, "`if` condition", true), ast::ExprWhile(ref cond, _, _) => (cond, "`while` condition", true), - ast::ExprMatch(ref head, _) => (head, "`match` head expression", true), + ast::ExprMatch(ref head, _, source) => match source { + ast::MatchNormal => (head, "`match` head expression", true), + ast::MatchIfLetDesugar => (head, "`if let` head expression", true) + }, ast::ExprRet(Some(ref value)) => (value, "`return` value", false), ast::ExprAssign(_, ref value) => (value, "assigned value", false), ast::ExprAssignOp(_, _, ref value) => (value, "assigned value", false), @@ -1241,7 +1244,7 @@ impl LintPass for UnusedMut { fn check_expr(&mut self, cx: &Context, e: &ast::Expr) { match e.node { - ast::ExprMatch(_, ref arms) => { + ast::ExprMatch(_, ref arms, _) => { for a in arms.iter() { self.check_unused_mut_pat(cx, a.pats.as_slice()) } diff --git a/src/librustc/middle/cfg/construct.rs b/src/librustc/middle/cfg/construct.rs index 1f0171de351ee..f370de31fdca7 100644 --- a/src/librustc/middle/cfg/construct.rs +++ b/src/librustc/middle/cfg/construct.rs @@ -324,7 +324,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { expr_exit } - ast::ExprMatch(ref discr, ref arms) => { + ast::ExprMatch(ref discr, ref arms, _) => { // // [pred] // | diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index d3321e555a40a..600d449ffe5e8 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -147,7 +147,7 @@ pub fn check_crate(tcx: &ty::ctxt) { fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) { visit::walk_expr(cx, ex); match ex.node { - ExprMatch(ref scrut, ref arms) => { + ExprMatch(ref scrut, ref arms, source) => { // First, check legality of move bindings. for arm in arms.iter() { check_legality_of_move_bindings(cx, @@ -184,7 +184,7 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) { } // Fourth, check for unreachable arms. - check_arms(cx, inlined_arms.as_slice()); + check_arms(cx, inlined_arms.as_slice(), source); // Finally, check if the whole match expression is exhaustive. // Check for empty enum, because is_useful only works on inhabited types. @@ -252,13 +252,31 @@ fn check_for_static_nan(cx: &MatchCheckCtxt, pats: &[P]) { } // Check for unreachable patterns -fn check_arms(cx: &MatchCheckCtxt, arms: &[(Vec>, Option<&Expr>)]) { +fn check_arms(cx: &MatchCheckCtxt, arms: &[(Vec>, Option<&Expr>)], source: MatchSource) { let mut seen = Matrix(vec![]); + let mut printed_if_let_err = false; for &(ref pats, guard) in arms.iter() { for pat in pats.iter() { let v = vec![&**pat]; + match is_useful(cx, &seen, v.as_slice(), LeaveOutWitness) { - NotUseful => span_err!(cx.tcx.sess, pat.span, E0001, "unreachable pattern"), + NotUseful => { + if source == MatchIfLetDesugar { + if printed_if_let_err { + // we already printed an irrefutable if-let pattern error. + // We don't want two, that's just confusing. + } else { + // find the first arm pattern so we can use its span + let &(ref first_arm_pats, _) = &arms[0]; // we know there's at least 1 arm + let first_pat = first_arm_pats.get(0); // and it's safe to assume 1 pat + let span = first_pat.span; + span_err!(cx.tcx.sess, span, E0162, "irrefutable if-let pattern"); + printed_if_let_err = true; + } + } else { + span_err!(cx.tcx.sess, pat.span, E0001, "unreachable pattern"); + } + } Useful => (), UsefulWithWitness(_) => unreachable!() } diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 875416feb2e4e..4423b83c74591 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -376,7 +376,7 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,TYPER> { ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), - ast::ExprMatch(ref discr, ref arms) => { + ast::ExprMatch(ref discr, ref arms, _) => { let discr_cmt = return_if_err!(self.mc.cat_expr(&**discr)); self.borrow_expr(&**discr, ty::ReEmpty, ty::ImmBorrow, MatchDiscriminant); diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 061f110afa906..00e65f34cf347 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -1029,7 +1029,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { self.propagate_through_loop(expr, LoopLoop, &**blk, succ) } - ExprMatch(ref e, ref arms) => { + ExprMatch(ref e, ref arms, _) => { // // (e) // | diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 5221faa598c66..aada90d260e8f 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -3659,7 +3659,7 @@ fn populate_scope_map(cx: &CrateContext, } } - ast::ExprMatch(ref discriminant_exp, ref arms) => { + ast::ExprMatch(ref discriminant_exp, ref arms, _) => { walk_expr(cx, &**discriminant_exp, scope_stack, scope_map); // For each arm we have to first walk the pattern as these might diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs index 120e8404f2c67..eeea10e8eb0a5 100644 --- a/src/librustc/middle/trans/expr.rs +++ b/src/librustc/middle/trans/expr.rs @@ -1013,7 +1013,7 @@ fn trans_rvalue_dps_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, ast::ExprIf(ref cond, ref thn, ref els) => { controlflow::trans_if(bcx, expr.id, &**cond, &**thn, els.as_ref().map(|e| &**e), dest) } - ast::ExprMatch(ref discr, ref arms) => { + ast::ExprMatch(ref discr, ref arms, _) => { _match::trans_match(bcx, expr, &**discr, arms.as_slice(), dest) } ast::ExprBlock(ref blk) => { diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 566203834080b..2c866b9ee383d 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -4144,7 +4144,7 @@ fn check_expr_with_unifier(fcx: &FnCtxt, fcx.write_nil(id); } } - ast::ExprMatch(ref discrim, ref arms) => { + ast::ExprMatch(ref discrim, ref arms, _) => { _match::check_match(fcx, expr, &**discrim, arms.as_slice()); } ast::ExprFnBlock(_, ref decl, ref body) => { diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index b853616991fed..9e20028569bf4 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -725,7 +725,7 @@ fn visit_expr(rcx: &mut Rcx, expr: &ast::Expr) { visit::walk_expr(rcx, expr); } - ast::ExprMatch(ref discr, ref arms) => { + ast::ExprMatch(ref discr, ref arms, _) => { link_match(rcx, &**discr, arms.as_slice()); visit::walk_expr(rcx, expr); diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index fcb613426fcb5..638aea10e3795 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -92,6 +92,7 @@ pub fn explain_region_and_span(cx: &ctxt, region: ty::Region) ast::ExprMethodCall(..) => { explain_span(cx, "method call", expr.span) }, + ast::ExprMatch(_, _, ast::MatchIfLetDesugar) => explain_span(cx, "if let", expr.span), ast::ExprMatch(..) => explain_span(cx, "match", expr.span), _ => explain_span(cx, "expression", expr.span) } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 02922fe3655ba..59c824d0eae77 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -529,7 +529,7 @@ pub enum Expr_ { // Conditionless loop (can be exited with break, cont, or ret) // FIXME #6993: change to Option ... or not, if these are hygienic. ExprLoop(P, Option), - ExprMatch(P, Vec), + ExprMatch(P, Vec, MatchSource), ExprFnBlock(CaptureClause, P, P), ExprProc(P, P), ExprUnboxedFn(CaptureClause, UnboxedClosureKind, P, P), @@ -577,6 +577,12 @@ pub struct QPath { pub item_name: Ident, } +#[deriving(Clone, PartialEq, Eq, Encodable, Decodable, Hash, Show)] +pub enum MatchSource { + MatchNormal, + MatchIfLetDesugar +} + #[deriving(Clone, PartialEq, Eq, Encodable, Decodable, Hash, Show)] pub enum CaptureClause { CaptureByValue, diff --git a/src/libsyntax/config.rs b/src/libsyntax/config.rs index 5b17f6f004a08..41b7218da0c52 100644 --- a/src/libsyntax/config.rs +++ b/src/libsyntax/config.rs @@ -210,10 +210,10 @@ fn fold_expr(cx: &mut Context, expr: P) -> P { fold::noop_fold_expr(ast::Expr { id: id, node: match node { - ast::ExprMatch(m, arms) => { + ast::ExprMatch(m, arms, source) => { ast::ExprMatch(m, arms.into_iter() .filter(|a| (cx.in_cfg)(a.attrs.as_slice())) - .collect()) + .collect(), source) } _ => node }, diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 0586868eb4584..f2b806f43ccbb 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -845,7 +845,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> { } fn expr_match(&self, span: Span, arg: P, arms: Vec) -> P { - self.expr(span, ast::ExprMatch(arg, arms)) + self.expr(span, ast::ExprMatch(arg, arms, ast::MatchNormal)) } fn expr_if(&self, span: Span, cond: P, diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 341be15b739fa..de6a8675e133b 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -127,7 +127,7 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { arms.push_all_move(else_if_arms); arms.push(else_arm); - let match_expr = fld.cx.expr_match(span, expr, arms); + let match_expr = fld.cx.expr(span, ast::ExprMatch(expr, arms, ast::MatchIfLetDesugar)); fld.fold_expr(match_expr) } diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 8bdc64a926df8..31bec58a4daa6 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1226,9 +1226,10 @@ pub fn noop_fold_expr(Expr {id, node, span}: Expr, folder: &mut T) -> ExprLoop(folder.fold_block(body), opt_ident.map(|i| folder.fold_ident(i))) } - ExprMatch(expr, arms) => { + ExprMatch(expr, arms, source) => { ExprMatch(folder.fold_expr(expr), - arms.move_map(|x| folder.fold_arm(x))) + arms.move_map(|x| folder.fold_arm(x)), + source) } ExprFnBlock(capture_clause, decl, body) => { ExprFnBlock(capture_clause, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index d4ba81c737bce..ccb398bf2fb3a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -38,7 +38,7 @@ use ast::{ItemMac, ItemMod, ItemStruct, ItemTrait, ItemTy}; use ast::{LifetimeDef, Lit, Lit_}; use ast::{LitBool, LitChar, LitByte, LitBinary}; use ast::{LitNil, LitStr, LitInt, Local, LocalLet}; -use ast::{MutImmutable, MutMutable, Mac_, MacInvocTT, Matcher, MatchNonterminal}; +use ast::{MutImmutable, MutMutable, Mac_, MacInvocTT, Matcher, MatchNonterminal, MatchNormal}; use ast::{MatchSeq, MatchTok, Method, MutTy, BiMul, Mutability}; use ast::{MethodImplItem, NamedField, UnNeg, NoReturn, UnNot}; use ast::{Pat, PatEnum, PatIdent, PatLit, PatRange, PatRegion, PatStruct}; @@ -2973,7 +2973,7 @@ impl<'a> Parser<'a> { } let hi = self.span.hi; self.bump(); - return self.mk_expr(lo, hi, ExprMatch(discriminant, arms)); + return self.mk_expr(lo, hi, ExprMatch(discriminant, arms, MatchNormal)); } pub fn parse_arm(&mut self) -> Arm { diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 1e5810fb311d9..a8e99b4e85f2b 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1535,7 +1535,7 @@ impl<'a> State<'a> { try!(space(&mut self.s)); try!(self.print_block(&**blk)); } - ast::ExprMatch(ref expr, ref arms) => { + ast::ExprMatch(ref expr, ref arms, _) => { try!(self.cbox(indent_unit)); try!(self.ibox(4)); try!(self.word_nbsp("match")); diff --git a/src/test/compile-fail/if-let.rs b/src/test/compile-fail/if-let.rs new file mode 100644 index 0000000000000..88b6854bb1d2c --- /dev/null +++ b/src/test/compile-fail/if-let.rs @@ -0,0 +1,57 @@ +// 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. + +#![feature(macro_rules)] + +fn macros() { + macro_rules! foo{ + ($p:pat, $e:expr, $b:block) => {{ + if let $p = $e $b + }} + } + macro_rules! bar{ + ($p:pat, $e:expr, $b:block) => {{ + foo!($p, $e, $b) + }} + } + + foo!(a, 1i, { //~ ERROR irrefutable if-let + println!("irrefutable pattern"); + }); + bar!(a, 1i, { //~ ERROR irrefutable if-let + println!("irrefutable pattern"); + }); +} + +pub fn main() { + if let a = 1i { //~ ERROR irrefutable if-let + println!("irrefutable pattern"); + } + + if let a = 1i { //~ ERROR irrefutable if-let + println!("irrefutable pattern"); + } else if true { + println!("else-if in irrefutable if-let"); + } else { + println!("else in irrefutable if-let"); + } + + if let 1i = 2i { + println!("refutable pattern"); + } else if let a = 1i { //~ ERROR irrefutable if-let + println!("irrefutable pattern"); + } + + if true { + println!("if"); + } else if let a = 1i { //~ ERROR irrefutable if-let + println!("irrefutable pattern"); + } +} diff --git a/src/test/compile-fail/lint-unnecessary-parens.rs b/src/test/compile-fail/lint-unnecessary-parens.rs index d51d5b4af87a9..82fb42571af74 100644 --- a/src/test/compile-fail/lint-unnecessary-parens.rs +++ b/src/test/compile-fail/lint-unnecessary-parens.rs @@ -32,6 +32,7 @@ fn main() { match (true) { //~ ERROR unnecessary parentheses around `match` head expression _ => {} } + if let 1i = (1i) {} //~ ERROR unnecessary parentheses around `if let` head expression let v = X { y: false }; // struct lits needs parens, so these shouldn't warn. if (v == X { y: true }) {} From 8a609521007c0c0c37d8d2396085631c08ad5232 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Tue, 26 Aug 2014 20:00:41 -0700 Subject: [PATCH 5/9] Move `if let` behind a feature gate --- src/doc/reference.md | 2 ++ src/doc/rust.md | 2 +- src/libsyntax/feature_gate.rs | 6 ++++++ src/test/compile-fail/if-let.rs | 2 +- src/test/compile-fail/lint-unnecessary-parens.rs | 1 + src/test/run-pass/if-let.rs | 2 ++ 6 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/doc/reference.md b/src/doc/reference.md index 21da810a30098..04ebcf9a3fb5e 100644 --- a/src/doc/reference.md +++ b/src/doc/reference.md @@ -2441,6 +2441,8 @@ The currently implemented features of the reference compiler are: * `default_type_params` - Allows use of default type parameters. The future of this feature is uncertain. +* `if_let` - Allows use of the `if let` desugaring syntax. + * `intrinsics` - Allows use of the "rust-intrinsics" ABI. Compiler intrinsics are inherently unstable and no promise about them is made. diff --git a/src/doc/rust.md b/src/doc/rust.md index 7f02260cd2cd4..c5dd95902fc6d 100644 --- a/src/doc/rust.md +++ b/src/doc/rust.md @@ -1,3 +1,3 @@ % The Rust Reference Manual -The manual has moved, and is now called [the reference](reference.html). +The manual has moved, and is now called [the reference](reference.html). \ No newline at end of file diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 1c6ee8acc94a5..fac4244228af4 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -71,6 +71,8 @@ static KNOWN_FEATURES: &'static [(&'static str, Status)] = &[ ("associated_types", Active), ("visible_private_types", Active), + ("if_let", Active), + // if you change this list without updating src/doc/rust.md, cmr will be sad // A temporary feature gate used to enable parser extensions needed @@ -356,6 +358,10 @@ impl<'a, 'v> Visitor<'v> for Context<'a> { e.span, "tuple indexing is experimental"); } + ast::ExprIfLet(..) => { + self.gate_feature("if_let", e.span, + "`if let` desugaring is experimental"); + } _ => {} } visit::walk_expr(self, e); diff --git a/src/test/compile-fail/if-let.rs b/src/test/compile-fail/if-let.rs index 88b6854bb1d2c..b82fb7a94c95f 100644 --- a/src/test/compile-fail/if-let.rs +++ b/src/test/compile-fail/if-let.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(macro_rules)] +#![feature(macro_rules,if_let)] fn macros() { macro_rules! foo{ diff --git a/src/test/compile-fail/lint-unnecessary-parens.rs b/src/test/compile-fail/lint-unnecessary-parens.rs index 82fb42571af74..3c962ebf21670 100644 --- a/src/test/compile-fail/lint-unnecessary-parens.rs +++ b/src/test/compile-fail/lint-unnecessary-parens.rs @@ -9,6 +9,7 @@ // except according to those terms. #![deny(unnecessary_parens)] +#![feature(if_let)] #[deriving(Eq, PartialEq)] struct X { y: bool } diff --git a/src/test/run-pass/if-let.rs b/src/test/run-pass/if-let.rs index a6886bf9850e2..4bf3a85677c72 100644 --- a/src/test/run-pass/if-let.rs +++ b/src/test/run-pass/if-let.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![feature(if_let)] + pub fn main() { let x = Some(3i); if let Some(y) = x { From 13e00e4a3d18802ca6407e59935be9f2c33ec061 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 27 Aug 2014 21:34:03 -0700 Subject: [PATCH 6/9] Update based on PR feedback --- src/doc/reference.md | 2 +- src/doc/rust.md | 2 +- src/librustc/middle/cfg/construct.rs | 4 ++- src/librustc/middle/expr_use_visitor.rs | 4 ++- src/librustc/middle/liveness.rs | 12 ++++++-- src/librustc/middle/mem_categorization.rs | 4 ++- src/librustc/middle/ty.rs | 5 +++- src/librustc/middle/typeck/check/mod.rs | 4 ++- src/libsyntax/ext/expand.rs | 36 ++++++++++------------- src/libsyntax/feature_gate.rs | 2 +- src/libsyntax/parse/parser.rs | 4 ++- 11 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/doc/reference.md b/src/doc/reference.md index 04ebcf9a3fb5e..0500088bbdc0b 100644 --- a/src/doc/reference.md +++ b/src/doc/reference.md @@ -2441,7 +2441,7 @@ The currently implemented features of the reference compiler are: * `default_type_params` - Allows use of default type parameters. The future of this feature is uncertain. -* `if_let` - Allows use of the `if let` desugaring syntax. +* `if_let` - Allows use of the `if let` syntax. * `intrinsics` - Allows use of the "rust-intrinsics" ABI. Compiler intrinsics are inherently unstable and no promise about them is made. diff --git a/src/doc/rust.md b/src/doc/rust.md index c5dd95902fc6d..7f02260cd2cd4 100644 --- a/src/doc/rust.md +++ b/src/doc/rust.md @@ -1,3 +1,3 @@ % The Rust Reference Manual -The manual has moved, and is now called [the reference](reference.html). \ No newline at end of file +The manual has moved, and is now called [the reference](reference.html). diff --git a/src/librustc/middle/cfg/construct.rs b/src/librustc/middle/cfg/construct.rs index f370de31fdca7..a9a3981ab5fc8 100644 --- a/src/librustc/middle/cfg/construct.rs +++ b/src/librustc/middle/cfg/construct.rs @@ -222,7 +222,9 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { self.add_node(expr.id, [then_exit, else_exit]) // 4, 5 } - ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), + ast::ExprIfLet(..) => { + self.tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet"); + } ast::ExprWhile(ref cond, ref body, _) => { // diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 4423b83c74591..51cdbfcf2514c 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -374,7 +374,9 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,TYPER> { } } - ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), + ast::ExprIfLet(..) => { + self.tcx().sess.span_bug(expr.span, "non-desugared ExprIfLet"); + } ast::ExprMatch(ref discr, ref arms, _) => { let discr_cmt = return_if_err!(self.mc.cat_expr(&**discr)); diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 00e65f34cf347..2176cd5658943 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -481,7 +481,9 @@ fn visit_expr(ir: &mut IrMaps, expr: &Expr) { ir.add_live_node_for_node(expr.id, ExprNode(expr.span)); visit::walk_expr(ir, expr); } - ExprIfLet(..) => fail!("non-desugared ExprIfLet"), + ExprIfLet(..) => { + ir.tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet"); + } ExprForLoop(ref pat, _, _, _) => { pat_util::pat_bindings(&ir.tcx.def_map, &**pat, |bm, p_id, sp, path1| { debug!("adding local variable {} from for loop with bm {:?}", @@ -1012,7 +1014,9 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { self.propagate_through_expr(&**cond, ln) } - ExprIfLet(..) => fail!("non-desugared ExprIfLet"), + ExprIfLet(..) => { + self.ir.tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet"); + } ExprWhile(ref cond, ref blk, _) => { self.propagate_through_loop(expr, WhileLoop(&**cond), &**blk, succ) @@ -1473,7 +1477,9 @@ fn check_expr(this: &mut Liveness, expr: &Expr) { ExprPath(..) | ExprBox(..) => { visit::walk_expr(this, expr); } - ExprIfLet(..) => fail!("non-desugared ExprIfLet") + ExprIfLet(..) => { + this.ir.tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet"); + } } } diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index d5ada0c5411a3..115432873ff9a 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -506,7 +506,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { Ok(self.cat_rvalue_node(expr.id(), expr.span(), expr_ty)) } - ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet") + ast::ExprIfLet(..) => { + self.tcx().sess.span_bug(expr.span, "non-desugared ExprIfLet"); + } } } diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index b55c3039ac8e3..5ef058fad350e 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -3631,10 +3631,13 @@ pub fn expr_kind(tcx: &ctxt, expr: &ast::Expr) -> ExprKind { RvalueDpsExpr } + ast::ExprIfLet(..) => { + tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet"); + } + ast::ExprLit(ref lit) if lit_is_str(&**lit) => { RvalueDpsExpr } - ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), ast::ExprCast(..) => { match tcx.node_types.borrow().find(&(expr.id as uint)) { diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 2c866b9ee383d..dd7b8a7e62413 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -4106,7 +4106,9 @@ fn check_expr_with_unifier(fcx: &FnCtxt, check_then_else(fcx, &**cond, &**then_blk, opt_else_expr.as_ref().map(|e| &**e), id, expr.span, expected); } - ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"), + ast::ExprIfLet(..) => { + tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet"); + } ast::ExprWhile(ref cond, ref body, _) => { check_expr_has_type(fcx, &**cond, ty::mk_bool()); check_block_no_value(fcx, &**body); diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index de6a8675e133b..76ca33cc2b846 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -132,26 +132,22 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { } // Desugar support for ExprIfLet in the ExprIf else position - ast::ExprIf(cond, blk, mut elseopt) => { - // NOTE: replace with 'if let' after snapshot - match elseopt { - Some(els) => match els.node { - ast::ExprIfLet(..) => { - // wrap the if-let expr in a block - let blk = P(ast::Block { - view_items: vec![], - stmts: vec![], - expr: Some(els), - id: ast::DUMMY_NODE_ID, - rules: ast::DefaultBlock, - span: els.span - }); - elseopt = Some(fld.cx.expr_block(blk)); - } - _ => () - }, - None => () - }; + ast::ExprIf(cond, blk, elseopt) => { + let elseopt = elseopt.map(|els| match els.node { + ast::ExprIfLet(..) => { + // wrap the if-let expr in a block + let blk = P(ast::Block { + view_items: vec![], + stmts: vec![], + expr: Some(els), + id: ast::DUMMY_NODE_ID, + rules: ast::DefaultBlock, + span: els.span + }); + fld.cx.expr_block(blk) + } + _ => els + }); let if_expr = fld.cx.expr(e.span, ast::ExprIf(cond, blk, elseopt)); noop_fold_expr(if_expr, fld) } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index fac4244228af4..ca6d488772c61 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -360,7 +360,7 @@ impl<'a, 'v> Visitor<'v> for Context<'a> { } ast::ExprIfLet(..) => { self.gate_feature("if_let", e.span, - "`if let` desugaring is experimental"); + "`if let` syntax is experimental"); } _ => {} } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ccb398bf2fb3a..0780e68a06294 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -579,7 +579,9 @@ impl<'a> Parser<'a> { if self.is_keyword(kw) { self.bump(); true - } else { false } + } else { + false + } } /// If the given word is not a keyword, signal an error. From e53f4a6b94623076a912df0c770c591d710c5de0 Mon Sep 17 00:00:00 2001 From: Jakub Wieczorek Date: Mon, 29 Sep 2014 22:40:26 +0200 Subject: [PATCH 7/9] Add `if let` to the reference --- src/doc/reference.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/doc/reference.md b/src/doc/reference.md index 0500088bbdc0b..14fc1d8d73d68 100644 --- a/src/doc/reference.md +++ b/src/doc/reference.md @@ -3231,7 +3231,7 @@ for i in range(0u, 256) { if_expr : "if" no_struct_literal_expr '{' block '}' else_tail ? ; -else_tail : "else" [ if_expr +else_tail : "else" [ if_expr | if_let_expr | '{' block '}' ] ; ``` @@ -3436,6 +3436,19 @@ let message = match maybe_digit { }; ``` +### If let expressions + +```{.ebnf .gram} +if_let_expr : "if" "let" pat '=' expr '{' block '}' + else_tail ? ; +else_tail : "else" [ if_expr | if_let_expr | '{' block '}' ] ; +``` + +An `if let` expression is semantically identical to an `if` expression but in place +of a condition expression it expects a refutable let statement. If the value of the +expression on the right hand side of the let statement matches the pattern, the corresponding +block will execute, otherwise flow proceeds to the first `else` block that follows. + ### Return expressions ```{.ebnf .gram} From 5254ccc7c16f232ddd8788b8a3e7c89ffa82532a Mon Sep 17 00:00:00 2001 From: Jakub Wieczorek Date: Mon, 29 Sep 2014 22:45:26 +0200 Subject: [PATCH 8/9] Update after the fall out from the syntax::ptr changes --- src/librustc/middle/check_match.rs | 4 +- src/libsyntax/ext/expand.rs | 66 ++++++++++++++++-------------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index 600d449ffe5e8..de9125ec44918 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -267,8 +267,8 @@ fn check_arms(cx: &MatchCheckCtxt, arms: &[(Vec>, Option<&Expr>)], source // We don't want two, that's just confusing. } else { // find the first arm pattern so we can use its span - let &(ref first_arm_pats, _) = &arms[0]; // we know there's at least 1 arm - let first_pat = first_arm_pats.get(0); // and it's safe to assume 1 pat + let &(ref first_arm_pats, _) = &arms[0]; + let first_pat = first_arm_pats.get(0); let span = first_pat.span; span_err!(cx.tcx.sess, span, E0162, "irrefutable if-let pattern"); printed_if_let_err = true; diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 76ca33cc2b846..fa3ccc8cf326e 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -70,8 +70,6 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { // Desugar ExprIfLet // From: `if let = []` ast::ExprIfLet(pat, expr, body, mut elseopt) => { - let span = e.span; - // to: // // match { @@ -90,23 +88,33 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { let else_if_arms = { let mut arms = vec![]; loop { - // NOTE: replace with 'if let' after snapshot - match elseopt { - Some(els) => match els.node { - // else if - ast::ExprIf(cond, then, elseopt_) => { - let pat_under = fld.cx.pat_wild(span); - elseopt = elseopt_; - arms.push(ast::Arm { - attrs: vec![], - pats: vec![pat_under], - guard: Some(cond), - body: fld.cx.expr_block(then) - }); - } - _ => break - }, - None => break + let elseopt_continue = elseopt + .and_then(|els| els.and_then(|els| match els.node { + // else if + ast::ExprIf(cond, then, elseopt) => { + let pat_under = fld.cx.pat_wild(span); + arms.push(ast::Arm { + attrs: vec![], + pats: vec![pat_under], + guard: Some(cond), + body: fld.cx.expr_block(then) + }); + elseopt.map(|elseopt| (elseopt, true)) + } + _ => Some((P(els), false)) + })); + match elseopt_continue { + Some((e, true)) => { + elseopt = Some(e); + } + Some((e, false)) => { + elseopt = Some(e); + break; + } + None => { + elseopt = None; + break; + } } } arms @@ -115,10 +123,7 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { // `_ => [ | ()]` let else_arm = { let pat_under = fld.cx.pat_wild(span); - let else_expr = match elseopt { - Some(els) => els, - None => fld.cx.expr_lit(span, ast::LitNil) - }; + let else_expr = elseopt.unwrap_or_else(|| fld.cx.expr_lit(span, ast::LitNil)); fld.cx.arm(span, vec![pat_under], else_expr) }; @@ -133,23 +138,24 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { // Desugar support for ExprIfLet in the ExprIf else position ast::ExprIf(cond, blk, elseopt) => { - let elseopt = elseopt.map(|els| match els.node { + let elseopt = elseopt.map(|els| els.and_then(|els| match els.node { ast::ExprIfLet(..) => { // wrap the if-let expr in a block + let span = els.span; let blk = P(ast::Block { view_items: vec![], stmts: vec![], - expr: Some(els), + expr: Some(P(els)), id: ast::DUMMY_NODE_ID, rules: ast::DefaultBlock, - span: els.span + span: span }); fld.cx.expr_block(blk) } - _ => els - }); - let if_expr = fld.cx.expr(e.span, ast::ExprIf(cond, blk, elseopt)); - noop_fold_expr(if_expr, fld) + _ => P(els) + })); + let if_expr = fld.cx.expr(span, ast::ExprIf(cond, blk, elseopt)); + if_expr.map(|e| noop_fold_expr(e, fld)) } ast::ExprLoop(loop_block, opt_ident) => { From e723051a2eb840a710f2b79afc1b42b0b707d0e4 Mon Sep 17 00:00:00 2001 From: Jakub Wieczorek Date: Tue, 30 Sep 2014 18:52:40 +0200 Subject: [PATCH 9/9] Temporarily remove the description for the diagnostic E0162 It turns out that adding new diagnostics is causing link failures in runpass-full-deps tests. Further investigation pending. --- src/librustc/diagnostics.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 10fff4db8d61b..0e85d7cc0752d 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -19,11 +19,6 @@ register_diagnostic!(E0001, r##" one is too specific or the ordering is incorrect. "##) -register_diagnostic!(E0162, r##" - This error is produced by an `if let` expression where the pattern is irrefutable. - An `if let` that can never fail is considered an error. -"##) - register_diagnostics!( E0002, E0003, @@ -156,5 +151,6 @@ register_diagnostics!( E0157, E0158, E0159, - E0161 + E0161, + E0162 )