Skip to content

Implement if let #16741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/doc/rust.md
Original file line number Diff line number Diff line change
Expand Up @@ -2457,6 +2457,8 @@ The currently implemented features of the reference compiler are:
whether this will continue as a feature or not. For these reasons,
the glob import statement has been hidden behind this feature flag.

* `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.

Expand Down
5 changes: 5 additions & 0 deletions src/librustc/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ register_diagnostic!(E0001, r##"
one is too specific or the ordering is incorrect.
"##)

register_diagnostic!(E0159, 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,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/front/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,15 @@ fn fold_block(cx: &mut Context, b: ast::P<ast::Block>) -> ast::P<ast::Block> {

fn fold_expr(cx: &mut Context, expr: Gc<ast::Expr>) -> Gc<ast::Expr> {
let expr = match expr.node {
ast::ExprMatch(ref m, ref arms) => {
ast::ExprMatch(ref m, ref arms, source) => {
let arms = arms.iter()
.filter(|a| (cx.in_cfg)(a.attrs.as_slice()))
.map(|a| a.clone())
.collect();
box(GC) ast::Expr {
id: expr.id,
span: expr.span.clone(),
node: ast::ExprMatch(m.clone(), arms),
node: ast::ExprMatch(m.clone(), arms, source),
}
}
_ => expr.clone()
Expand Down
10 changes: 10 additions & 0 deletions src/librustc/front/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ static KNOWN_FEATURES: &'static [(&'static str, Status)] = &[
("unboxed_closures", Active),
("import_shadowing", 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
Expand Down Expand Up @@ -339,6 +341,14 @@ impl<'a> Visitor<()> for Context<'a> {
"unboxed closures are a work-in-progress \
feature with known bugs");
}
ast::ExprIfLet(..) |
ast::ExprMatch(_, _, ast::MatchIfLetDesugar) => {
// note: at the point where we check feature-gates, the AST desugaring
// should not have happened, but the ExprMatch check is included just
// in case.
self.gate_feature("if_let", e.span,
"`if let` syntax is experimental");
}
_ => {}
}
visit::walk_expr(self, e, ());
Expand Down
7 changes: 5 additions & 2 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1080,7 +1080,10 @@ impl LintPass for UnnecessaryParens {
let (value, msg, struct_lit_needs_parens) = match e.node {
ast::ExprIf(cond, _, _) => (cond, "`if` condition", true),
ast::ExprWhile(cond, _) => (cond, "`while` condition", true),
ast::ExprMatch(head, _) => (head, "`match` head expression", true),
ast::ExprMatch(head, _, source) => match source {
ast::MatchNormal => (head, "`match` head expression", true),
ast::MatchIfLetDesugar => (head, "`if let` head expression", true)
},
ast::ExprRet(Some(value)) => (value, "`return` value", false),
ast::ExprAssign(_, value) => (value, "assigned value", false),
ast::ExprAssignOp(_, _, value) => (value, "assigned value", false),
Expand Down Expand Up @@ -1192,7 +1195,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())
}
Expand Down
6 changes: 5 additions & 1 deletion src/librustc/middle/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ impl<'a> CFGBuilder<'a> {
self.add_node(expr.id, [then_exit, else_exit]) // 4, 5
}

ast::ExprIfLet(..) => {
self.tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet");
}

ast::ExprWhile(ref cond, ref body) => {
//
// [pred]
Expand Down Expand Up @@ -325,7 +329,7 @@ impl<'a> CFGBuilder<'a> {
expr_exit
}

ast::ExprMatch(ref discr, ref arms) => {
ast::ExprMatch(ref discr, ref arms, _) => {
//
// [pred]
// |
Expand Down
25 changes: 21 additions & 4 deletions src/librustc/middle/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub fn check_crate(tcx: &ty::ctxt, krate: &Crate) {
fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) {
visit::walk_expr(cx, ex, ());
match ex.node {
ExprMatch(scrut, ref arms) => {
ExprMatch(scrut, ref arms, source) => {
// First, check legality of move bindings.
for arm in arms.iter() {
check_legality_of_move_bindings(cx,
Expand Down Expand Up @@ -178,7 +178,7 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) {
check_for_static_nan(cx, inlined_arms.as_slice());

// 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.
Expand Down Expand Up @@ -251,13 +251,30 @@ fn check_for_static_nan(cx: &MatchCheckCtxt, arms: &[Arm]) {
}

// Check for unreachable patterns
fn check_arms(cx: &MatchCheckCtxt, arms: &[Arm]) {
fn check_arms(cx: &MatchCheckCtxt, arms: &[Arm], source: MatchSource) {
let mut seen = Matrix(vec!());
let mut printed_if_let_err = false;
for arm in arms.iter() {
for &pat in arm.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 first_arm = &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, E0159, "irrefutable if-let pattern");
printed_if_let_err = true;
}
} else {
span_err!(cx.tcx.sess, pat.span, E0001, "unreachable pattern");
}
}
Useful => (),
UsefulWithWitness(_) => unreachable!()
}
Expand Down
6 changes: 5 additions & 1 deletion src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,11 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
}
}

ast::ExprMatch(ref discr, ref arms) => {
ast::ExprIfLet(..) => {
self.tcx().sess.span_bug(expr.span, "non-desugared ExprIfLet");
}

ast::ExprMatch(ref discr, ref arms, _) => {
// treatment of the discriminant is handled while
// walking the arms:
self.walk_expr(&**discr);
Expand Down
12 changes: 11 additions & 1 deletion src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,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(..) => {
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 {:?}",
Expand Down Expand Up @@ -1017,6 +1020,10 @@ impl<'a> Liveness<'a> {
self.propagate_through_expr(&**cond, ln)
}

ExprIfLet(..) => {
self.ir.tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet");
}

ExprWhile(ref cond, ref blk) => {
self.propagate_through_loop(expr,
WhileLoop(cond.clone()),
Expand All @@ -1035,7 +1042,7 @@ impl<'a> Liveness<'a> {
self.propagate_through_loop(expr, LoopLoop, &**blk, succ)
}

ExprMatch(ref e, ref arms) => {
ExprMatch(ref e, ref arms, _) => {
//
// (e)
// |
Expand Down Expand Up @@ -1458,6 +1465,9 @@ fn check_expr(this: &mut Liveness, expr: &Expr) {
ExprPath(..) | ExprBox(..) | ExprForLoop(..) => {
visit::walk_expr(this, expr, ());
}
ExprIfLet(..) => {
this.ir.tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet");
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,10 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> {
ast::ExprForLoop(..) => {
Ok(self.cat_rvalue_node(expr.id(), expr.span(), expr_ty))
}

ast::ExprIfLet(..) => {
self.tcx().sess.span_bug(expr.span, "non-desugared ExprIfLet");
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/librustc/middle/trans/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3603,6 +3603,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);

Expand Down Expand Up @@ -3681,7 +3686,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
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ fn trans_rvalue_dps_unadjusted<'a>(bcx: &'a Block<'a>,
ast::ExprIf(ref cond, ref thn, els) => {
controlflow::trans_if(bcx, expr.id, &**cond, thn.clone(), els, 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) => {
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3429,6 +3429,10 @@ pub fn expr_kind(tcx: &ctxt, expr: &ast::Expr) -> ExprKind {
RvalueDpsExpr
}

ast::ExprIfLet(..) => {
tcx.sess.span_bug(expr.span, "non-desugared ExprIfLet");
}

ast::ExprLit(lit) if lit_is_str(lit) => {
RvalueDpsExpr
}
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/middle/typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3430,6 +3430,9 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
check_then_else(fcx, &**cond, &**then_blk, opt_else_expr.clone(),
id, expr.span, expected);
}
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);
Expand Down Expand Up @@ -3468,7 +3471,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) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,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, ());
Expand Down
1 change: 1 addition & 0 deletions src/librustc/util/ppaux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a ExprIfLet arm here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually don't know what this bit of code is doing, so I don't know how to test it, but I assume it's only ever called on the post-desugared AST. The old for loop desugaring didn't touch this code, which is why I didn't add anything for ExprIfLet here.

}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_back/svh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ mod svh_visitor {
ExprForLoop(..) => SawExprForLoop,

// just syntactic artifacts, expanded away by time of SVH.
ExprIfLet(..) => unreachable!(),
ExprMac(..) => unreachable!(),
}
}
Expand Down
9 changes: 8 additions & 1 deletion src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,13 +534,14 @@ pub enum Expr_ {
ExprLit(Gc<Lit>),
ExprCast(Gc<Expr>, P<Ty>),
ExprIf(Gc<Expr>, P<Block>, Option<Gc<Expr>>),
ExprIfLet(P<Pat>, P<Expr>, P<Block>, Option<P<Expr>>),
ExprWhile(Gc<Expr>, P<Block>),
// FIXME #6993: change to Option<Name> ... or not, if these are hygienic.
ExprForLoop(Gc<Pat>, Gc<Expr>, P<Block>, Option<Ident>),
// Conditionless loop (can be exited with break, cont, or ret)
// FIXME #6993: change to Option<Name> ... or not, if these are hygienic.
ExprLoop(P<Block>, Option<Ident>),
ExprMatch(Gc<Expr>, Vec<Arm>),
ExprMatch(Gc<Expr>, Vec<Arm>, MatchSource),
ExprFnBlock(CaptureClause, P<FnDecl>, P<Block>),
ExprProc(P<FnDecl>, P<Block>),
ExprUnboxedFn(CaptureClause, UnboxedClosureKind, P<FnDecl>, P<Block>),
Expand Down Expand Up @@ -574,6 +575,12 @@ pub enum Expr_ {
ExprParen(Gc<Expr>)
}

#[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,
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {

fn expr_match(&self, span: Span, arg: Gc<ast::Expr>,
arms: Vec<ast::Arm>) -> Gc<Expr> {
self.expr(span, ast::ExprMatch(arg, arms))
self.expr(span, ast::ExprMatch(arg, arms, ast::MatchNormal))
}

fn expr_if(&self, span: Span,
Expand Down
Loading