Skip to content

Commit

Permalink
hir::print: fix parenthesization of exprs
Browse files Browse the repository at this point in the history
  • Loading branch information
@spernsteiner
spernsteiner committed Sep 6, 2017
1 parent 5b2151e commit 347db06
Showing 1 changed file with 180 additions and 38 deletions.
218 changes: 180 additions & 38 deletions src/librustc/hir/print.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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()?;
}
Expand Down Expand Up @@ -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)
}

Expand All @@ -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() ||
Expand All @@ -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,
Expand All @@ -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<()> {
Expand All @@ -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)?;
Expand Down Expand Up @@ -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)?;
}
Expand All @@ -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)?;
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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("]")?;
Expand All @@ -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()?;
}
}
Expand All @@ -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)?;
}
_ => (),
}
Expand Down Expand Up @@ -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))?;
Expand Down Expand Up @@ -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,
}
}

0 comments on commit 347db06

Please sign in to comment.