From 347db068a5786d3cac6e2204d7e14b635ece558d Mon Sep 17 00:00:00 2001 From: Stuart Pernsteiner Date: Mon, 14 Aug 2017 18:27:20 -0400 Subject: [PATCH] hir::print: fix parenthesization of exprs --- src/librustc/hir/print.rs | 218 +++++++++++++++++++++++++++++++------- 1 file changed, 180 insertions(+), 38 deletions(-) diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index dce824bd513a7..dce7dd33db423 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -20,6 +20,7 @@ use syntax::print::pp::Breaks::{Consistent, Inconsistent}; use syntax::print::pprust::PrintState; use syntax::ptr::P; use syntax::symbol::keywords; +use syntax::util::parser::{self, AssocOp, Fixity}; use syntax_pos::{self, BytePos}; use hir; @@ -210,18 +211,6 @@ pub fn visibility_qualified(vis: &hir::Visibility, w: &str) -> String { }) } -fn needs_parentheses(expr: &hir::Expr) -> bool { - match expr.node { - hir::ExprAssign(..) | - hir::ExprBinary(..) | - hir::ExprClosure(..) | - hir::ExprAssignOp(..) | - hir::ExprCast(..) | - hir::ExprType(..) => true, - _ => false, - } -} - impl<'a> State<'a> { pub fn cbox(&mut self, u: usize) -> io::Result<()> { self.boxes.push(pp::Breaks::Consistent); @@ -1047,7 +1036,7 @@ impl<'a> State<'a> { self.cbox(indent_unit - 1)?; self.ibox(0)?; self.s.word(" else if ")?; - self.print_expr(&i)?; + self.print_expr_as_cond(&i)?; self.s.space()?; self.print_expr(&then)?; self.print_else(e.as_ref().map(|e| &**e)) @@ -1075,7 +1064,7 @@ impl<'a> State<'a> { elseopt: Option<&hir::Expr>) -> io::Result<()> { self.head("if")?; - self.print_expr(test)?; + self.print_expr_as_cond(test)?; self.s.space()?; self.print_expr(blk)?; self.print_else(elseopt) @@ -1091,7 +1080,7 @@ impl<'a> State<'a> { self.print_pat(pat)?; self.s.space()?; self.word_space("=")?; - self.print_expr(expr)?; + self.print_expr_as_cond(expr)?; self.s.space()?; self.print_block(blk)?; self.print_else(elseopt) @@ -1104,8 +1093,31 @@ impl<'a> State<'a> { self.pclose() } - pub fn print_expr_maybe_paren(&mut self, expr: &hir::Expr) -> io::Result<()> { - let needs_par = needs_parentheses(expr); + pub fn print_expr_maybe_paren(&mut self, expr: &hir::Expr, prec: i8) -> io::Result<()> { + let needs_par = expr_precedence(expr) < prec; + if needs_par { + self.popen()?; + } + self.print_expr(expr)?; + if needs_par { + self.pclose()?; + } + Ok(()) + } + + /// Print an expr using syntax that's acceptable in a condition position, such as the `cond` in + /// `if cond { ... }`. + pub fn print_expr_as_cond(&mut self, expr: &hir::Expr) -> io::Result<()> { + let needs_par = match expr.node { + // These cases need parens due to the parse error observed in #26461: `if return {}` + // parses as the erroneous construct `if (return {})`, not `if (return) {}`. + hir::ExprClosure(..) | + hir::ExprRet(..) | + hir::ExprBreak(..) => true, + + _ => contains_exterior_struct_lit(expr), + }; + if needs_par { self.popen()?; } @@ -1182,7 +1194,14 @@ impl<'a> State<'a> { } fn print_expr_call(&mut self, func: &hir::Expr, args: &[hir::Expr]) -> io::Result<()> { - self.print_expr_maybe_paren(func)?; + let prec = + match func.node { + hir::ExprField(..) | + hir::ExprTupField(..) => parser::PREC_FORCE_PAREN, + _ => parser::PREC_POSTFIX, + }; + + self.print_expr_maybe_paren(func, prec)?; self.print_call_post(args) } @@ -1191,7 +1210,7 @@ impl<'a> State<'a> { args: &[hir::Expr]) -> io::Result<()> { let base_args = &args[1..]; - self.print_expr(&args[0])?; + self.print_expr_maybe_paren(&args[0], parser::PREC_POSTFIX)?; self.s.word(".")?; self.print_name(segment.name)?; if !segment.parameters.lifetimes.is_empty() || @@ -1207,15 +1226,25 @@ impl<'a> State<'a> { lhs: &hir::Expr, rhs: &hir::Expr) -> io::Result<()> { - self.print_expr(lhs)?; + let assoc_op = bin_op_to_assoc_op(op.node); + let prec = assoc_op.precedence() as i8; + let fixity = assoc_op.fixity(); + + let (left_prec, right_prec) = match fixity { + Fixity::Left => (prec, prec + 1), + Fixity::Right => (prec + 1, prec), + Fixity::None => (prec + 1, prec + 1), + }; + + self.print_expr_maybe_paren(lhs, left_prec)?; self.s.space()?; self.word_space(op.node.as_str())?; - self.print_expr(rhs) + self.print_expr_maybe_paren(rhs, right_prec) } fn print_expr_unary(&mut self, op: hir::UnOp, expr: &hir::Expr) -> io::Result<()> { self.s.word(op.as_str())?; - self.print_expr_maybe_paren(expr) + self.print_expr_maybe_paren(expr, parser::PREC_PREFIX) } fn print_expr_addr_of(&mut self, @@ -1224,7 +1253,7 @@ impl<'a> State<'a> { -> io::Result<()> { self.s.word("&")?; self.print_mutability(mutability)?; - self.print_expr_maybe_paren(expr) + self.print_expr_maybe_paren(expr, parser::PREC_PREFIX) } pub fn print_expr(&mut self, expr: &hir::Expr) -> io::Result<()> { @@ -1235,7 +1264,7 @@ impl<'a> State<'a> { match expr.node { hir::ExprBox(ref expr) => { self.word_space("box")?; - self.print_expr(expr)?; + self.print_expr_maybe_paren(expr, parser::PREC_PREFIX)?; } hir::ExprArray(ref exprs) => { self.print_expr_vec(exprs)?; @@ -1268,13 +1297,15 @@ impl<'a> State<'a> { self.print_literal(&lit)?; } hir::ExprCast(ref expr, ref ty) => { - self.print_expr(&expr)?; + let prec = AssocOp::As.precedence() as i8; + self.print_expr_maybe_paren(&expr, prec)?; self.s.space()?; self.word_space("as")?; self.print_type(&ty)?; } hir::ExprType(ref expr, ref ty) => { - self.print_expr(&expr)?; + let prec = AssocOp::Colon.precedence() as i8; + self.print_expr_maybe_paren(&expr, prec)?; self.word_space(":")?; self.print_type(&ty)?; } @@ -1287,7 +1318,7 @@ impl<'a> State<'a> { self.word_space(":")?; } self.head("while")?; - self.print_expr(&test)?; + self.print_expr_as_cond(&test)?; self.s.space()?; self.print_block(&blk)?; } @@ -1304,7 +1335,7 @@ impl<'a> State<'a> { self.cbox(indent_unit)?; self.ibox(4)?; self.word_nbsp("match")?; - self.print_expr(&expr)?; + self.print_expr_as_cond(&expr)?; self.s.space()?; self.bopen()?; for arm in arms { @@ -1335,30 +1366,32 @@ impl<'a> State<'a> { self.print_block(&blk)?; } hir::ExprAssign(ref lhs, ref rhs) => { - self.print_expr(&lhs)?; + let prec = AssocOp::Assign.precedence() as i8; + self.print_expr_maybe_paren(&lhs, prec + 1)?; self.s.space()?; self.word_space("=")?; - self.print_expr(&rhs)?; + self.print_expr_maybe_paren(&rhs, prec)?; } hir::ExprAssignOp(op, ref lhs, ref rhs) => { - self.print_expr(&lhs)?; + let prec = AssocOp::Assign.precedence() as i8; + self.print_expr_maybe_paren(&lhs, prec + 1)?; self.s.space()?; self.s.word(op.node.as_str())?; self.word_space("=")?; - self.print_expr(&rhs)?; + self.print_expr_maybe_paren(&rhs, prec)?; } hir::ExprField(ref expr, name) => { - self.print_expr(&expr)?; + self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX)?; self.s.word(".")?; self.print_name(name.node)?; } hir::ExprTupField(ref expr, id) => { - self.print_expr(&expr)?; + self.print_expr_maybe_paren(&expr, parser::PREC_POSTFIX)?; self.s.word(".")?; self.print_usize(id.node)?; } hir::ExprIndex(ref expr, ref index) => { - self.print_expr(&expr)?; + self.print_expr_maybe_paren(&expr, parser::PREC_POSTFIX)?; self.s.word("[")?; self.print_expr(&index)?; self.s.word("]")?; @@ -1374,7 +1407,7 @@ impl<'a> State<'a> { self.s.space()?; } if let Some(ref expr) = *opt_expr { - self.print_expr(expr)?; + self.print_expr_maybe_paren(expr, parser::PREC_JUMP)?; self.s.space()?; } } @@ -1391,7 +1424,7 @@ impl<'a> State<'a> { match *result { Some(ref expr) => { self.s.word(" ")?; - self.print_expr(&expr)?; + self.print_expr_maybe_paren(&expr, parser::PREC_JUMP)?; } _ => (), } @@ -1463,7 +1496,7 @@ impl<'a> State<'a> { } hir::ExprYield(ref expr) => { self.s.word("yield")?; - self.print_expr(&expr)?; + self.print_expr_maybe_paren(&expr, parser::PREC_JUMP)?; } } self.ann.post(self, NodeExpr(expr))?; @@ -2246,3 +2279,112 @@ fn stmt_ends_with_semi(stmt: &hir::Stmt_) -> bool { } } } + + +fn expr_precedence(expr: &hir::Expr) -> i8 { + use syntax::util::parser::*; + + match expr.node { + hir::ExprClosure(..) => PREC_CLOSURE, + + hir::ExprBreak(..) | + hir::ExprAgain(..) | + hir::ExprRet(..) | + hir::ExprYield(..) => PREC_JUMP, + + hir::ExprIf(..) | + hir::ExprWhile(..) | + hir::ExprLoop(..) | + hir::ExprMatch(..) | + hir::ExprBlock(..) => PREC_BLOCK, + + // Binop-like expr kinds, handled by `AssocOp`. + hir::ExprBinary(op, _, _) => bin_op_to_assoc_op(op.node).precedence() as i8, + + hir::ExprCast(..) => AssocOp::As.precedence() as i8, + hir::ExprType(..) => AssocOp::Colon.precedence() as i8, + + hir::ExprAssign(..) | + hir::ExprAssignOp(..) => AssocOp::Assign.precedence() as i8, + + // Unary, prefix + hir::ExprBox(..) | + hir::ExprAddrOf(..) | + hir::ExprUnary(..) => PREC_PREFIX, + + // Unary, postfix + hir::ExprCall(..) | + hir::ExprMethodCall(..) | + hir::ExprField(..) | + hir::ExprTupField(..) | + hir::ExprIndex(..) | + hir::ExprInlineAsm(..) => PREC_POSTFIX, + + // Never need parens + hir::ExprArray(..) | + hir::ExprRepeat(..) | + hir::ExprTup(..) | + hir::ExprLit(..) | + hir::ExprPath(..) | + hir::ExprStruct(..) => PREC_PAREN, + } +} + +fn bin_op_to_assoc_op(op: hir::BinOp_) -> AssocOp { + use hir::BinOp_::*; + match op { + BiAdd => AssocOp::Add, + BiSub => AssocOp::Subtract, + BiMul => AssocOp::Multiply, + BiDiv => AssocOp::Divide, + BiRem => AssocOp::Modulus, + + BiAnd => AssocOp::LAnd, + BiOr => AssocOp::LOr, + + BiBitXor => AssocOp::BitXor, + BiBitAnd => AssocOp::BitAnd, + BiBitOr => AssocOp::BitOr, + BiShl => AssocOp::ShiftLeft, + BiShr => AssocOp::ShiftRight, + + BiEq => AssocOp::Equal, + BiLt => AssocOp::Less, + BiLe => AssocOp::LessEqual, + BiNe => AssocOp::NotEqual, + BiGe => AssocOp::GreaterEqual, + BiGt => AssocOp::Greater, + } +} + +/// Expressions that syntactically contain an "exterior" struct literal i.e. not surrounded by any +/// parens or other delimiters, e.g. `X { y: 1 }`, `X { y: 1 }.method()`, `foo == X { y: 1 }` and +/// `X { y: 1 } == foo` all do, but `(X { y: 1 }) == foo` does not. +fn contains_exterior_struct_lit(value: &hir::Expr) -> bool { + match value.node { + hir::ExprStruct(..) => true, + + hir::ExprAssign(ref lhs, ref rhs) | + hir::ExprAssignOp(_, ref lhs, ref rhs) | + hir::ExprBinary(_, ref lhs, ref rhs) => { + // X { y: 1 } + X { y: 2 } + contains_exterior_struct_lit(&lhs) || contains_exterior_struct_lit(&rhs) + } + hir::ExprUnary(_, ref x) | + hir::ExprCast(ref x, _) | + hir::ExprType(ref x, _) | + hir::ExprField(ref x, _) | + hir::ExprTupField(ref x, _) | + hir::ExprIndex(ref x, _) => { + // &X { y: 1 }, X { y: 1 }.y + contains_exterior_struct_lit(&x) + } + + hir::ExprMethodCall(.., ref exprs) => { + // X { y: 1 }.bar(...) + contains_exterior_struct_lit(&exprs[0]) + } + + _ => false, + } +}