From be5537e95fbe80d4564982a44d73c4053ee29ee9 Mon Sep 17 00:00:00 2001
From: Marijn Haverbeke <marijnh@gmail.com>
Date: Tue, 13 Sep 2011 15:50:03 +0200
Subject: [PATCH] Be more strict about what constitutes a block expression

Blocks (or statements involving blocks) that end in a semicolon are no
longer considered the block-expression of their outer block. This used
to be an expression block, but now is a statement block:

    { if foo { ret 1; } else { ret 10; } }

This helps clear up some ambiguities in our grammar.
---
 src/comp/syntax/parse/parser.rs          | 41 ++++++++++++++++++++----
 src/test/compile-fail/forgot-ret.rs      |  2 +-
 src/test/compile-fail/missing-return2.rs |  3 +-
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/comp/syntax/parse/parser.rs b/src/comp/syntax/parse/parser.rs
index be54b38a68a..5006277ec6b 100644
--- a/src/comp/syntax/parse/parser.rs
+++ b/src/comp/syntax/parse/parser.rs
@@ -1571,8 +1571,42 @@ fn parse_source_stmt(p: parser) -> @ast::stmt {
     }
 }
 
+fn stmt_is_expr(stmt: @ast::stmt) -> bool {
+    fn expr_is_expr(e: @ast::expr) -> bool {
+        alt e.node {
+          ast::expr_if(_, th, els) | ast::expr_if_check(_, th, els) {
+            if option::is_none(els) { false }
+            else { !option::is_none(th.node.expr) ||
+                       expr_is_expr(option::get(els)) }
+          }
+          ast::expr_alt(_, arms) {
+            let found_expr = false;
+            for arm in arms {
+                if !option::is_none(arm.body.node.expr) { found_expr = true; }
+            }
+            found_expr
+          }
+          ast::expr_block(blk) | ast::expr_while(_, blk) |
+          ast::expr_for(_, _, blk) | ast::expr_for_each(_, _, blk) |
+          ast::expr_do_while(blk, _) {
+            !option::is_none(blk.node.expr)
+          }
+          _ { true }
+        }
+    }
+
+    ret alt stmt.node {
+      ast::stmt_expr(e, _) { expr_is_expr(e) }
+      _ { false }
+    };
+}
+
 fn stmt_to_expr(stmt: @ast::stmt) -> option::t<@ast::expr> {
-    ret alt stmt.node { ast::stmt_expr(e, _) { some(e) } _ { none } };
+    ret if stmt_is_expr(stmt) {
+        alt stmt.node {
+          ast::stmt_expr(e, _) { some(e) }
+        }
+    } else { none };
 }
 
 fn stmt_ends_with_semi(stmt: ast::stmt) -> bool {
@@ -1627,10 +1661,6 @@ fn stmt_ends_with_semi(stmt: ast::stmt) -> bool {
             }
       }
 
-
-
-
-
       // We should not be calling this on a cdir.
       ast::stmt_crate_directive(cdir) {
         fail;
@@ -1681,7 +1711,6 @@ fn parse_block_tail(p: parser, lo: uint, s: ast::check_mode) -> ast::blk {
                 // Not an expression statement.
                 stmts += [stmt];
 
-
                 if p.get_file_type() == SOURCE_FILE &&
                        stmt_ends_with_semi(*stmt) {
                     expect(p, token::SEMI);
diff --git a/src/test/compile-fail/forgot-ret.rs b/src/test/compile-fail/forgot-ret.rs
index 65d7db8b30e..0f780f1b335 100644
--- a/src/test/compile-fail/forgot-ret.rs
+++ b/src/test/compile-fail/forgot-ret.rs
@@ -1,5 +1,5 @@
 // -*- rust -*-
-// error-pattern: mismatched types
+// error-pattern: not all control paths return a value
 
 fn god_exists(a: int) -> bool { be god_exists(a); }
 
diff --git a/src/test/compile-fail/missing-return2.rs b/src/test/compile-fail/missing-return2.rs
index 446dacdf4f9..a73db075196 100644
--- a/src/test/compile-fail/missing-return2.rs
+++ b/src/test/compile-fail/missing-return2.rs
@@ -1,7 +1,6 @@
-// error-pattern: mismatched types
+// error-pattern: not all control paths return a value
 
 fn f() -> int {
-
     // Make sure typestate doesn't interpret this alt expression
     // as the function result
     alt true { true { } }