make parser handle parenthesized block-sugar exprs properly

2 files changed, 114 insertions, 81 deletions

diff --git a/src/comp/syntax/parse/parser.rs b/src/comp/syntax/parse/parser.rs
index d9edeefd44d..59aa5ee516a 100644
--- a/src/comp/syntax/parse/parser.rs
+++ b/src/comp/syntax/parse/parser.rs
@@ -736,7 +736,29 @@ fn mk_lit_u32(p: parser, i: u32) -> @ast::expr {
     ret @{id: p.get_id(), node: ast::expr_lit(lv_lit), span: span};
 }
 
-fn parse_bottom_expr(p: parser) -> @ast::expr {
+// We don't allow single-entry tuples in the true AST; that indicates a
+// parenthesized expression.  However, we preserve them temporarily while
+// parsing because `(while{...})+3` parses differently from `while{...}+3`.
+//
+// To reflect the fact that the @ast::expr is not a true expr that should be
+// part of the AST, we wrap such expressions in the pexpr tag.  They
+// can then be converted to true expressions by a call to `to_expr()`.
+tag pexpr {
+    pexpr(@ast::expr);
+}
+
+fn mk_pexpr(p: parser, lo: uint, hi: uint, node: ast::expr_) -> pexpr {
+    ret pexpr(mk_expr(p, lo, hi, node));
+}
+
+fn to_expr(e: pexpr) -> @ast::expr {
+    alt e.node {
+      ast::expr_tup(es) when vec::len(es) == 1u { es[0u] }
+      _ { *e }
+    }
+}
+
+fn parse_bottom_expr(p: parser) -> pexpr {
     let lo = p.get_lo_pos();
     let hi = p.get_hi_pos();
 
@@ -747,15 +769,19 @@ fn parse_bottom_expr(p: parser) -> @ast::expr {
             hi = p.get_hi_pos();
             p.bump();
             let lit = @spanned(lo, hi, ast::lit_nil);
-            ret mk_expr(p, lo, hi, ast::expr_lit(lit));
+            ret mk_pexpr(p, lo, hi, ast::expr_lit(lit));
         }
         let es = [parse_expr(p)];
         while p.peek() == token::COMMA { p.bump(); es += [parse_expr(p)]; }
         hi = p.get_hi_pos();
         expect(p, token::RPAREN);
-        if vec::len(es) == 1u {
-            ret mk_expr(p, lo, hi, es[0].node);
-        } else { ret mk_expr(p, lo, hi, ast::expr_tup(es)); }
+
+        // Note: we retain the expr_tup() even for simple
+        // parenthesized expressions, but only for a "little while".
+        // This is so that wrappers around parse_bottom_expr()
+        // can tell whether the expression was parenthesized or not,
+        // which affects expr_is_complete().
+        ret mk_pexpr(p, lo, hi, ast::expr_tup(es));
     } else if p.peek() == token::LBRACE {
         p.bump();
         if is_word(p, "mutable") ||
@@ -775,34 +801,34 @@ fn parse_bottom_expr(p: parser) -> @ast::expr {
             expect(p, token::RBRACE);
             ex = ast::expr_rec(fields, base);
         } else if is_bar(p.peek()) {
-            ret parse_fn_block_expr(p);
+            ret pexpr(parse_fn_block_expr(p));
         } else {
             let blk = parse_block_tail(p, lo, ast::default_blk);
-            ret mk_expr(p, blk.span.lo, blk.span.hi, ast::expr_block(blk));
+            ret mk_pexpr(p, blk.span.lo, blk.span.hi, ast::expr_block(blk));
         }
     } else if eat_word(p, "if") {
-        ret parse_if_expr(p);
+        ret pexpr(parse_if_expr(p));
     } else if eat_word(p, "for") {
-        ret parse_for_expr(p);
+        ret pexpr(parse_for_expr(p));
     } else if eat_word(p, "while") {
-        ret parse_while_expr(p);
+        ret pexpr(parse_while_expr(p));
     } else if eat_word(p, "do") {
-        ret parse_do_while_expr(p);
+        ret pexpr(parse_do_while_expr(p));
     } else if eat_word(p, "alt") {
-        ret parse_alt_expr(p);
+        ret pexpr(parse_alt_expr(p));
     } else if eat_word(p, "fn") {
         let proto = parse_fn_ty_proto(p);
-        ret parse_fn_expr(p, proto);
+        ret pexpr(parse_fn_expr(p, proto));
     } else if eat_word(p, "block") {
-        ret parse_fn_expr(p, ast::proto_block);
+        ret pexpr(parse_fn_expr(p, ast::proto_block));
     } else if eat_word(p, "lambda") {
-        ret parse_fn_expr(p, ast::proto_shared(ast::sugar_sexy));
+        ret pexpr(parse_fn_expr(p, ast::proto_shared(ast::sugar_sexy)));
     } else if eat_word(p, "sendfn") {
-        ret parse_fn_expr(p, ast::proto_send);
+        ret pexpr(parse_fn_expr(p, ast::proto_send));
     } else if eat_word(p, "unchecked") {
-        ret parse_block_expr(p, lo, ast::unchecked_blk);
+        ret pexpr(parse_block_expr(p, lo, ast::unchecked_blk));
     } else if eat_word(p, "unsafe") {
-        ret parse_block_expr(p, lo, ast::unsafe_blk);
+        ret pexpr(parse_block_expr(p, lo, ast::unsafe_blk));
     } else if p.peek() == token::LBRACKET {
         p.bump();
         let mut = parse_mutability(p);
@@ -816,15 +842,16 @@ fn parse_bottom_expr(p: parser) -> @ast::expr {
         expect(p, token::GT);
 
         /* hack: early return to take advantage of specialized function */
-        ret mk_mac_expr(p, lo, p.get_hi_pos(), ast::mac_embed_type(ty));
+        ret pexpr(mk_mac_expr(p, lo, p.get_hi_pos(),
+                              ast::mac_embed_type(ty)));
     } else if p.peek() == token::POUND_LBRACE {
         p.bump();
         let blk = ast::mac_embed_block(
             parse_block_tail(p, lo, ast::default_blk));
-        ret mk_mac_expr(p, lo, p.get_hi_pos(), blk);
+        ret pexpr(mk_mac_expr(p, lo, p.get_hi_pos(), blk));
     } else if p.peek() == token::ELLIPSIS {
         p.bump();
-        ret mk_mac_expr(p, lo, p.get_hi_pos(), ast::mac_ellipsis);
+        ret pexpr(mk_mac_expr(p, lo, p.get_hi_pos(), ast::mac_ellipsis));
     } else if eat_word(p, "obj") {
         // Anonymous object
 
@@ -941,7 +968,7 @@ fn parse_bottom_expr(p: parser) -> @ast::expr {
         hi = lit.span.hi;
         ex = ast::expr_lit(@lit);
     }
-    ret mk_expr(p, lo, hi, ex);
+    ret mk_pexpr(p, lo, hi, ex);
 }
 
 fn parse_block_expr(p: parser,
@@ -977,7 +1004,7 @@ fn parse_syntax_ext_naked(p: parser, lo: uint) -> @ast::expr {
     ret mk_mac_expr(p, lo, hi, ast::mac_invoc(pth, e, none));
 }
 
-fn parse_dot_or_call_expr(p: parser) -> @ast::expr {
+fn parse_dot_or_call_expr(p: parser) -> pexpr {
     let b = parse_bottom_expr(p);
     parse_dot_or_call_expr_with(p, b)
 }
@@ -986,10 +1013,10 @@ fn permits_call(p: parser) -> bool {
     ret p.get_restriction() != RESTRICT_NO_CALL_EXPRS;
 }
 
-fn parse_dot_or_call_expr_with(p: parser, e: @ast::expr) -> @ast::expr {
+fn parse_dot_or_call_expr_with(p: parser, e0: pexpr) -> pexpr {
+    let e = e0;
     let lo = e.span.lo;
     let hi = e.span.hi;
-    let e = e;
     while !expr_is_complete(p, e) {
         alt p.peek() {
           // expr(...)
@@ -997,22 +1024,22 @@ fn parse_dot_or_call_expr_with(p: parser, e: @ast::expr) -> @ast::expr {
             let es = parse_seq(token::LPAREN, token::RPAREN,
                                seq_sep(token::COMMA), parse_expr, p);
             hi = es.span.hi;
-            let nd = ast::expr_call(e, es.node, false);
-            e = mk_expr(p, lo, hi, nd);
+            let nd = ast::expr_call(to_expr(e), es.node, false);
+            e = mk_pexpr(p, lo, hi, nd);
           }
 
-          // expr { || ... }
+          // expr {|| ... }
           token::LBRACE. when is_bar(p.look_ahead(1u)) && permits_call(p) {
             p.bump();
             let blk = parse_fn_block_expr(p);
             alt e.node {
               ast::expr_call(f, args, false) {
-                e = @{node: ast::expr_call(f, args + [blk], true)
-                      with *e};
+                e = pexpr(@{node: ast::expr_call(f, args + [blk], true)
+                            with *to_expr(e)});
               }
               _ {
-                e = mk_expr(p, lo, p.get_last_hi_pos(),
-                            ast::expr_call(e, [blk], true));
+                e = mk_pexpr(p, lo, p.get_last_hi_pos(),
+                            ast::expr_call(to_expr(e), [blk], true));
               }
             }
           }
@@ -1023,7 +1050,7 @@ fn parse_dot_or_call_expr_with(p: parser, e: @ast::expr) -> @ast::expr {
             let ix = parse_expr(p);
             hi = ix.span.hi;
             expect(p, token::RBRACKET);
-            e = mk_expr(p, lo, hi, ast::expr_index(e, ix));
+            e = mk_pexpr(p, lo, hi, ast::expr_index(to_expr(e), ix));
           }
 
           // expr.f
@@ -1038,7 +1065,10 @@ fn parse_dot_or_call_expr_with(p: parser, e: @ast::expr) -> @ast::expr {
                     parse_seq_to_gt(some(token::COMMA),
                                     {|p| parse_ty(p, false)}, p)
                 } else { [] };
-                e = mk_expr(p, lo, hi, ast::expr_field(e, p.get_str(i), tys));
+                e = mk_pexpr(p, lo, hi,
+                             ast::expr_field(to_expr(e),
+                                             p.get_str(i),
+                                             tys));
               }
               t { unexpected(p, t); }
             }
@@ -1050,7 +1080,7 @@ fn parse_dot_or_call_expr_with(p: parser, e: @ast::expr) -> @ast::expr {
     ret e;
 }
 
-fn parse_prefix_expr(p: parser) -> @ast::expr {
+fn parse_prefix_expr(p: parser) -> pexpr {
     let lo = p.get_lo_pos();
     let hi = p.get_hi_pos();
 
@@ -1058,7 +1088,7 @@ fn parse_prefix_expr(p: parser) -> @ast::expr {
     alt p.peek() {
       token::NOT. {
         p.bump();
-        let e = parse_prefix_expr(p);
+        let e = to_expr(parse_prefix_expr(p));
         hi = e.span.hi;
         ex = ast::expr_unary(ast::not, e);
       }
@@ -1066,13 +1096,13 @@ fn parse_prefix_expr(p: parser) -> @ast::expr {
         alt b {
           token::MINUS. {
             p.bump();
-            let e = parse_prefix_expr(p);
+            let e = to_expr(parse_prefix_expr(p));
             hi = e.span.hi;
             ex = ast::expr_unary(ast::neg, e);
           }
           token::STAR. {
             p.bump();
-            let e = parse_prefix_expr(p);
+            let e = to_expr(parse_prefix_expr(p));
             hi = e.span.hi;
             ex = ast::expr_unary(ast::deref, e);
           }
@@ -1082,20 +1112,20 @@ fn parse_prefix_expr(p: parser) -> @ast::expr {
       token::AT. {
         p.bump();
         let m = parse_mutability(p);
-        let e = parse_prefix_expr(p);
+        let e = to_expr(parse_prefix_expr(p));
         hi = e.span.hi;
         ex = ast::expr_unary(ast::box(m), e);
       }
       token::TILDE. {
         p.bump();
         let m = parse_mutability(p);
-        let e = parse_prefix_expr(p);
+        let e = to_expr(parse_prefix_expr(p));
         hi = e.span.hi;
         ex = ast::expr_unary(ast::uniq(m), e);
       }
       _ { ret parse_dot_or_call_expr(p); }
     }
-    ret mk_expr(p, lo, hi, ex);
+    ret mk_pexpr(p, lo, hi, ex);
 }
 
 fn parse_ternary(p: parser) -> @ast::expr {
@@ -1146,9 +1176,10 @@ const unop_prec: int = 100;
 const as_prec: int = 5;
 const ternary_prec: int = 0;
 
-fn parse_more_binops(p: parser, lhs: @ast::expr, min_prec: int) ->
+fn parse_more_binops(p: parser, plhs: pexpr, min_prec: int) ->
    @ast::expr {
-    if expr_is_complete(p, lhs) { ret lhs; }
+    let lhs = to_expr(plhs);
+    if expr_is_complete(p, plhs) { ret lhs; }
     let peeked = p.peek();
     if peeked == token::BINOP(token::OR) &&
        p.get_restriction() == RESTRICT_NO_BAR_OP { ret lhs; }
@@ -1157,7 +1188,7 @@ fn parse_more_binops(p: parser, lhs: @ast::expr, min_prec: int) ->
             p.bump();
             let expr = parse_prefix_expr(p);
             let rhs = parse_more_binops(p, expr, cur.prec);
-            let bin = mk_expr(p, lhs.span.lo, rhs.span.hi,
+            let bin = mk_pexpr(p, lhs.span.lo, rhs.span.hi,
                               ast::expr_binary(cur.op, lhs, rhs));
             ret parse_more_binops(p, bin, min_prec);
         }
@@ -1165,7 +1196,7 @@ fn parse_more_binops(p: parser, lhs: @ast::expr, min_prec: int) ->
     if as_prec > min_prec && eat_word(p, "as") {
         let rhs = parse_ty(p, true);
         let _as =
-            mk_expr(p, lhs.span.lo, rhs.span.hi, ast::expr_cast(lhs, rhs));
+            mk_pexpr(p, lhs.span.lo, rhs.span.hi, ast::expr_cast(lhs, rhs));
         ret parse_more_binops(p, _as, min_prec);
     }
     ret lhs;
@@ -1587,12 +1618,12 @@ fn parse_stmt(p: parser) -> @ast::stmt {
     }
 }
 
-fn expr_is_complete(p: parser, e: @ast::expr) -> bool {
+fn expr_is_complete(p: parser, e: pexpr) -> bool {
     log(debug, ("expr_is_complete", p.get_restriction(),
-                print::pprust::expr_to_str(e),
-                expr_requires_semi_to_be_stmt(e)));
+                print::pprust::expr_to_str(*e),
+                expr_requires_semi_to_be_stmt(*e)));
     ret p.get_restriction() == RESTRICT_STMT_EXPR &&
-        !expr_requires_semi_to_be_stmt(e);
+        !expr_requires_semi_to_be_stmt(*e);
 }
 
 fn expr_requires_semi_to_be_stmt(e: @ast::expr) -> bool {
diff --git a/src/comp/syntax/print/pprust.rs b/src/comp/syntax/print/pprust.rs
index da592b6d3c1..efcb5af30ae 100644
--- a/src/comp/syntax/print/pprust.rs
+++ b/src/comp/syntax/print/pprust.rs
@@ -558,22 +558,6 @@ fn print_attribute(s: ps, attr: ast::attribute) {
     word(s.s, "]");
 }
 
-// An expression that begins with a dual-form statement/expression like `{
-// ... }-10` would be parsed as `{ ... };-10` unless parentheses are used (ie,
-// `({...}-10)`).  These parentheses are not, however, preserved by the
-// parser. This function specifies whether parentheses must be inserted.
-fn stmt_expr_requires_parens(ex: @ast::expr) -> bool {
-    fn helper(ex: @ast::expr, inner: bool) -> bool {
-        alt ex.node {
-          ast::expr_call(subex, _, _) | ast::expr_binary(_, subex, _) {
-            be helper(subex, true);
-          }
-          _ when !inner { ret false; }
-          _ { ret !parse::parser::expr_requires_semi_to_be_stmt(ex); }
-        }
-    }
-    ret helper(ex, false);
-}
 
 fn print_stmt(s: ps, st: ast::stmt) {
     maybe_print_comment(s, st.span.lo);
@@ -583,13 +567,7 @@ fn print_stmt(s: ps, st: ast::stmt) {
       }
       ast::stmt_expr(expr, _) {
         space_if_not_bol(s);
-        if stmt_expr_requires_parens(expr) {
-            popen(s);
-            print_expr(s, expr);
-            pclose(s);
-        } else {
-            print_expr(s, expr);
-        }
+        print_tl_expr(s, expr);
       }
     }
     if parse::parser::stmt_ends_with_semi(st) { word(s.s, ";"); }
@@ -626,7 +604,7 @@ fn print_possibly_embedded_block(s: ps, blk: ast::blk, embedded: embed_type,
     alt blk.node.expr {
       some(expr) {
         space_if_not_bol(s);
-        print_expr(s, expr);
+        print_tl_expr(s, expr);
         maybe_print_trailing_comment(s, expr.span, some(blk.span.hi));
       }
       _ { }
@@ -714,6 +692,37 @@ fn print_mac(s: ps, m: ast::mac) {
     }
 }
 
+// An expression that begins with a dual-form statement/expression like `{
+// ... }-10` would be parsed as `{ ... };-10` unless parentheses are used (ie,
+// `({...}-10)`).  These parentheses are not, however, preserved by the
+// parser. This function specifies whether parentheses must be inserted.
+fn print_tl_expr(s: ps, &&expr: @ast::expr) {
+    fn stmt_expr_requires_parens(ex: @ast::expr) -> bool {
+        fn helper(ex: @ast::expr, inner: bool) -> bool {
+            log(debug, ("helper", ex, inner));
+            if inner && !parse::parser::expr_requires_semi_to_be_stmt(ex) {
+                ret true;
+            }
+            alt ex.node {
+              ast::expr_call(subex, _, _) | ast::expr_binary(_, subex, _) {
+                be helper(subex, true);
+              }
+              _ { ret false; }
+            }
+        }
+        ret helper(ex, false);
+    }
+
+    #debug("print_tl_expr %s", expr_to_str(expr));
+    if stmt_expr_requires_parens(expr) {
+        popen(s);
+        print_expr(s, expr);
+        pclose(s);
+    } else {
+        print_expr(s, expr);
+    }
+}
+
 fn print_expr(s: ps, &&expr: @ast::expr) {
     maybe_print_comment(s, expr.span.lo);
     ibox(s, indent_unit);
@@ -1380,15 +1389,8 @@ fn need_parens(expr: @ast::expr, outer_prec: int) -> bool {
       ast::expr_binary(op, _, _) { operator_prec(op) < outer_prec }
       ast::expr_cast(_, _) { parse::parser::as_prec < outer_prec }
       ast::expr_ternary(_, _, _) { parse::parser::ternary_prec < outer_prec }
-
-
-
-
-
       // This may be too conservative in some cases
-      ast::expr_assign(_, _) {
-        true
-      }
+      ast::expr_assign(_, _) { true }
       ast::expr_move(_, _) { true }
       ast::expr_swap(_, _) { true }
       ast::expr_assign_op(_, _, _) { true }