From f2a137829e07505aaaa2be4ae97d6ecfb6ef6823 Mon Sep 17 00:00:00 2001
From: Huon Wilson <dbau.pp+github@gmail.com>
Date: Mon, 26 May 2014 22:01:09 +1000
Subject: [PATCH] syntax: desugar a `for` loop to a let binding to get better
 error messages when the pattern is refutable.

This means the compiler points directly to the pattern and said that the
problem is the pattern being refutable (rather than just saying that
some value isn't covered in the `match` as it did previously).

Fixes #14390.
---
 src/librustc/middle/check_match.rs            |  7 +++-
 src/libsyntax/ext/expand.rs                   | 36 ++++++++++++++++---
 ...or-loop-refutable-pattern-error-message.rs | 16 +++++++++
 3 files changed, 54 insertions(+), 5 deletions(-)
 create mode 100644 src/test/compile-fail/for-loop-refutable-pattern-error-message.rs

diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs
index fb797a027956..cc319b70e396 100644
--- a/src/librustc/middle/check_match.rs
+++ b/src/librustc/middle/check_match.rs
@@ -864,8 +864,13 @@ 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) {
+        let name = match loc.source {
+            LocalLet => "local",
+            LocalFor => "`for` loop"
+        };
+
         cx.tcx.sess.span_err(loc.pat.span,
-                             "refutable pattern in local binding");
+                             format!("refutable pattern in {} binding", name).as_slice());
     }
 
     // Check legality of move bindings.
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 762363f3abea..b8766200ccd0 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 {
             //       ['<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 = {
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 000000000000..8b00b6149099
--- /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 <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() {}
+}