Try harder to disambig blocks followed by various exprs in pprust. Closes #840

As it turns out, it's not just unops we need to worry about. Also tuples and
anything that requires parens.

src/comp/syntax/print/pprust.rs

@@ -7,6 +7,7 @@ import std::uint;
 import std::option;
 import parse::lexer;
 import syntax::codemap::codemap;
+import syntax::visit;
 import ast;
 import option::some;
 import option::none;
@@ -596,13 +597,13 @@ fn print_possibly_embedded_block(s: &ps, blk: &ast::blk, embedded: embed_type,
 
     let last_stmt = option::none;
     for st: @ast::stmt in blk.node.stmts {
-        maybe_protect_unop(s, last_stmt, stmt_(st));
+        maybe_protect_block(s, last_stmt, stmt_(st));
         print_stmt(s, *st);
         last_stmt = option::some(st);
     }
     alt blk.node.expr {
       some(expr) {
-        maybe_protect_unop(s, last_stmt, expr_(expr));
+        maybe_protect_block(s, last_stmt, expr_(expr));
         space_if_not_bol(s);
         print_expr(s, expr);
         maybe_print_trailing_comment(s, expr.span, some(blk.span.hi));
@@ -615,11 +616,10 @@ fn print_possibly_embedded_block(s: &ps, blk: &ast::blk, embedded: embed_type,
     tag expr_or_stmt { stmt_(@ast::stmt); expr_(@ast::expr); }
 
     // The Rust syntax has an ambiguity when an if, alt, or block statement is
-    // followed by a unary op statement. In those cases we have to add an
+    // followed by a unary op or paren. In those cases we have to add an
-    // extra semi to make sure the unop is not parsed as a binop with the
+    // extra semi to make sure the output retains the same meaning.
-    // if/alt/block expression.
+    fn maybe_protect_block(s: &ps, last: &option::t<@ast::stmt>,
-    fn maybe_protect_unop(s: &ps, last: &option::t<@ast::stmt>,
+                           next: &expr_or_stmt) {
-                          next: &expr_or_stmt) {
         let last_expr_is_block = alt last {
           option::some(@{node: ast::stmt_expr(e, _), _}) {
             alt e.node {
@@ -632,20 +632,74 @@ fn print_possibly_embedded_block(s: &ps, blk: &ast::blk, embedded: embed_type,
           }
           _ { false }
         };
-        let next_expr_is_unnop = alt next {
+
-          expr_(@{node: ast::expr_unary(_, _), _}) { true }
+        if !last_expr_is_block { ret; }
+
+        let next_expr_is_ambig = alt next {
+          expr_(e) { expr_is_ambig(e) }
           stmt_(@{node: ast::stmt_expr(e, _), _}) {
-            alt e.node {
+            expr_is_ambig(e)
-              ast::expr_unary(_, _) { true }
-              _ { false }
-            }
           }
           _ { false }
         };
 
-        if last_expr_is_block && next_expr_is_unnop {
+        if last_expr_is_block && next_expr_is_ambig {
             word(s.s, ";");
         }
+
+        fn expr_is_ambig(ex: @ast::expr) -> bool {
+          // We're going to walk the expression to the 'left' looking for
+          // various properties that might indicate ambiguity
+
+          type env = @mutable bool;
+          let visitor = visit::mk_vt(@{
+              visit_expr: visit_expr
+                  with *visit::default_visitor()
+          });
+          let env = @mutable false;
+          visit_expr(ex, env, visitor);
+          ret *env;
+
+          fn visit_expr(ex: &@ast::expr, e: &env, v: &visit::vt<env>) {
+              assert *e == false;
+
+              if expr_is_ambig(ex) {
+                  *e = true;
+                  ret;
+              }
+
+              alt ex.node {
+                ast::expr_assign(x, _) { v.visit_expr(x, e, v); }
+                ast::expr_assign_op(_, x, _) { visit_expr(x, e, v); }
+                ast::expr_move(x, _) { v.visit_expr(x, e, v); }
+                ast::expr_field(x, _) { v.visit_expr(x, e, v); }
+                ast::expr_index(x, _) { v.visit_expr(x, e, v); }
+                ast::expr_binary(op, x, _) {
+                  if need_parens(x, operator_prec(op)) {
+                      *e = true;
+                      ret;
+                  }
+                  v.visit_expr(x, e, v);
+                }
+                ast::expr_cast(x, _) {
+                  if need_parens(x, parse::parser::as_prec) {
+                      *e = true;
+                      ret;
+                  }
+                }
+                ast::expr_ternary(x, _, _) { v.visit_expr(x, e, v); }
+                _ { }
+              }
+          }
+
+          fn expr_is_ambig(ex: @ast::expr) -> bool {
+              alt ex.node {
+                ast::expr_unary(_, _) { true }
+                ast::expr_tup(_) { true }
+                _ { false }
+              }
+          }
+        }
     }
 }
 
@@ -1307,7 +1361,7 @@ fn operator_prec(op: ast::binop) -> int {
     fail;
 }
 
-fn print_maybe_parens(s: &ps, expr: &@ast::expr, outer_prec: int) {
+fn need_parens(expr: &@ast::expr, outer_prec: int) -> bool {
     let add_them;
     alt expr.node {
       ast::expr_binary(op, _, _) {
@@ -1319,6 +1373,11 @@ fn print_maybe_parens(s: &ps, expr: &@ast::expr, outer_prec: int) {
       }
       _ { add_them = false; }
     }
+    ret add_them;
+}
+
+fn print_maybe_parens(s: &ps, expr: &@ast::expr, outer_prec: int) {
+    let add_them = need_parens(expr, outer_prec);
     if add_them { popen(s); }
     print_expr(s, expr);
     if add_them { pclose(s); }

src/test/pretty/block-disambig.rs

@@ -0,0 +1,65 @@
+// Tests that the pretty printer correctly disambiguates various scenarios
+// involving block statements by ending them with a semi-colon
+fn test1() {
+    let val = @0;
+    {};
+    *val;
+}
+
+fn test2() -> int {
+    let val = @0;
+    {};
+    *val
+}
+
+fn test3() {
+    let regs = @{mutable eax: 0};
+    alt true {
+      true { }
+    };
+    (*regs).eax = 1;
+}
+
+fn test4() -> bool {
+    let regs = @true;
+    if true { };
+    *regs || false
+}
+
+fn test5() -> (int, int) {
+    {};
+    (0, 1)
+}
+
+fn test6() -> bool {
+    {};
+    (true || false) && true
+}
+
+fn test7() -> uint {
+    let regs = @0;
+    alt true { true { } };
+    (*regs < 2) as uint
+}
+
+fn test8() -> int {
+    let val = @0;
+    alt true { true { } };
+    *val < 1 ? 0 : 1
+}
+
+fn test9() {
+    let regs = @mutable 0;
+    alt true {
+      true { }
+    };
+    *regs += 1;
+}
+
+fn test10() -> int {
+    let regs = @mutable [0];
+    alt true {
+      true { }
+    };
+    (*regs).(0)
+}