Skip to content

Commit 347db06

Browse files
spernsteinerspernsteiner
committed
hir::print: fix parenthesization of exprs
1 parent 5b2151e commit 347db06

File tree

1 file changed

+180
-38
lines changed

1 file changed

+180
-38
lines changed

‎src/librustc/hir/print.rs

+180-38
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use syntax::print::pp::Breaks::{Consistent, Inconsistent};
2020
use syntax::print::pprust::PrintState;
2121
use syntax::ptr::P;
2222
use syntax::symbol::keywords;
23+
use syntax::util::parser::{self, AssocOp, Fixity};
2324
use syntax_pos::{self, BytePos};
2425

2526
use hir;
@@ -210,18 +211,6 @@ pub fn visibility_qualified(vis: &hir::Visibility, w: &str) -> String {
210211
})
211212
}
212213

213-
fn needs_parentheses(expr: &hir::Expr) -> bool {
214-
match expr.node {
215-
hir::ExprAssign(..) |
216-
hir::ExprBinary(..) |
217-
hir::ExprClosure(..) |
218-
hir::ExprAssignOp(..) |
219-
hir::ExprCast(..) |
220-
hir::ExprType(..) => true,
221-
_ => false,
222-
}
223-
}
224-
225214
impl<'a> State<'a> {
226215
pub fn cbox(&mut self, u: usize) -> io::Result<()> {
227216
self.boxes.push(pp::Breaks::Consistent);
@@ -1047,7 +1036,7 @@ impl<'a> State<'a> {
10471036
self.cbox(indent_unit - 1)?;
10481037
self.ibox(0)?;
10491038
self.s.word(" else if ")?;
1050-
self.print_expr(&i)?;
1039+
self.print_expr_as_cond(&i)?;
10511040
self.s.space()?;
10521041
self.print_expr(&then)?;
10531042
self.print_else(e.as_ref().map(|e| &**e))
@@ -1075,7 +1064,7 @@ impl<'a> State<'a> {
10751064
elseopt: Option<&hir::Expr>)
10761065
-> io::Result<()> {
10771066
self.head("if")?;
1078-
self.print_expr(test)?;
1067+
self.print_expr_as_cond(test)?;
10791068
self.s.space()?;
10801069
self.print_expr(blk)?;
10811070
self.print_else(elseopt)
@@ -1091,7 +1080,7 @@ impl<'a> State<'a> {
10911080
self.print_pat(pat)?;
10921081
self.s.space()?;
10931082
self.word_space("=")?;
1094-
self.print_expr(expr)?;
1083+
self.print_expr_as_cond(expr)?;
10951084
self.s.space()?;
10961085
self.print_block(blk)?;
10971086
self.print_else(elseopt)
@@ -1104,8 +1093,31 @@ impl<'a> State<'a> {
11041093
self.pclose()
11051094
}
11061095

1107-
pub fn print_expr_maybe_paren(&mut self, expr: &hir::Expr) -> io::Result<()> {
1108-
let needs_par = needs_parentheses(expr);
1096+
pub fn print_expr_maybe_paren(&mut self, expr: &hir::Expr, prec: i8) -> io::Result<()> {
1097+
let needs_par = expr_precedence(expr) < prec;
1098+
if needs_par {
1099+
self.popen()?;
1100+
}
1101+
self.print_expr(expr)?;
1102+
if needs_par {
1103+
self.pclose()?;
1104+
}
1105+
Ok(())
1106+
}
1107+
1108+
/// Print an expr using syntax that's acceptable in a condition position, such as the `cond` in
1109+
/// `if cond { ... }`.
1110+
pub fn print_expr_as_cond(&mut self, expr: &hir::Expr) -> io::Result<()> {
1111+
let needs_par = match expr.node {
1112+
// These cases need parens due to the parse error observed in #26461: `if return {}`
1113+
// parses as the erroneous construct `if (return {})`, not `if (return) {}`.
1114+
hir::ExprClosure(..) |
1115+
hir::ExprRet(..) |
1116+
hir::ExprBreak(..) => true,
1117+
1118+
_ => contains_exterior_struct_lit(expr),
1119+
};
1120+
11091121
if needs_par {
11101122
self.popen()?;
11111123
}
@@ -1182,7 +1194,14 @@ impl<'a> State<'a> {
11821194
}
11831195

11841196
fn print_expr_call(&mut self, func: &hir::Expr, args: &[hir::Expr]) -> io::Result<()> {
1185-
self.print_expr_maybe_paren(func)?;
1197+
let prec =
1198+
match func.node {
1199+
hir::ExprField(..) |
1200+
hir::ExprTupField(..) => parser::PREC_FORCE_PAREN,
1201+
_ => parser::PREC_POSTFIX,
1202+
};
1203+
1204+
self.print_expr_maybe_paren(func, prec)?;
11861205
self.print_call_post(args)
11871206
}
11881207

@@ -1191,7 +1210,7 @@ impl<'a> State<'a> {
11911210
args: &[hir::Expr])
11921211
-> io::Result<()> {
11931212
let base_args = &args[1..];
1194-
self.print_expr(&args[0])?;
1213+
self.print_expr_maybe_paren(&args[0], parser::PREC_POSTFIX)?;
11951214
self.s.word(".")?;
11961215
self.print_name(segment.name)?;
11971216
if !segment.parameters.lifetimes.is_empty() ||
@@ -1207,15 +1226,25 @@ impl<'a> State<'a> {
12071226
lhs: &hir::Expr,
12081227
rhs: &hir::Expr)
12091228
-> io::Result<()> {
1210-
self.print_expr(lhs)?;
1229+
let assoc_op = bin_op_to_assoc_op(op.node);
1230+
let prec = assoc_op.precedence() as i8;
1231+
let fixity = assoc_op.fixity();
1232+
1233+
let (left_prec, right_prec) = match fixity {
1234+
Fixity::Left => (prec, prec + 1),
1235+
Fixity::Right => (prec + 1, prec),
1236+
Fixity::None => (prec + 1, prec + 1),
1237+
};
1238+
1239+
self.print_expr_maybe_paren(lhs, left_prec)?;
12111240
self.s.space()?;
12121241
self.word_space(op.node.as_str())?;
1213-
self.print_expr(rhs)
1242+
self.print_expr_maybe_paren(rhs, right_prec)
12141243
}
12151244

12161245
fn print_expr_unary(&mut self, op: hir::UnOp, expr: &hir::Expr) -> io::Result<()> {
12171246
self.s.word(op.as_str())?;
1218-
self.print_expr_maybe_paren(expr)
1247+
self.print_expr_maybe_paren(expr, parser::PREC_PREFIX)
12191248
}
12201249

12211250
fn print_expr_addr_of(&mut self,
@@ -1224,7 +1253,7 @@ impl<'a> State<'a> {
12241253
-> io::Result<()> {
12251254
self.s.word("&")?;
12261255
self.print_mutability(mutability)?;
1227-
self.print_expr_maybe_paren(expr)
1256+
self.print_expr_maybe_paren(expr, parser::PREC_PREFIX)
12281257
}
12291258

12301259
pub fn print_expr(&mut self, expr: &hir::Expr) -> io::Result<()> {
@@ -1235,7 +1264,7 @@ impl<'a> State<'a> {
12351264
match expr.node {
12361265
hir::ExprBox(ref expr) => {
12371266
self.word_space("box")?;
1238-
self.print_expr(expr)?;
1267+
self.print_expr_maybe_paren(expr, parser::PREC_PREFIX)?;
12391268
}
12401269
hir::ExprArray(ref exprs) => {
12411270
self.print_expr_vec(exprs)?;
@@ -1268,13 +1297,15 @@ impl<'a> State<'a> {
12681297
self.print_literal(&lit)?;
12691298
}
12701299
hir::ExprCast(ref expr, ref ty) => {
1271-
self.print_expr(&expr)?;
1300+
let prec = AssocOp::As.precedence() as i8;
1301+
self.print_expr_maybe_paren(&expr, prec)?;
12721302
self.s.space()?;
12731303
self.word_space("as")?;
12741304
self.print_type(&ty)?;
12751305
}
12761306
hir::ExprType(ref expr, ref ty) => {
1277-
self.print_expr(&expr)?;
1307+
let prec = AssocOp::Colon.precedence() as i8;
1308+
self.print_expr_maybe_paren(&expr, prec)?;
12781309
self.word_space(":")?;
12791310
self.print_type(&ty)?;
12801311
}
@@ -1287,7 +1318,7 @@ impl<'a> State<'a> {
12871318
self.word_space(":")?;
12881319
}
12891320
self.head("while")?;
1290-
self.print_expr(&test)?;
1321+
self.print_expr_as_cond(&test)?;
12911322
self.s.space()?;
12921323
self.print_block(&blk)?;
12931324
}
@@ -1304,7 +1335,7 @@ impl<'a> State<'a> {
13041335
self.cbox(indent_unit)?;
13051336
self.ibox(4)?;
13061337
self.word_nbsp("match")?;
1307-
self.print_expr(&expr)?;
1338+
self.print_expr_as_cond(&expr)?;
13081339
self.s.space()?;
13091340
self.bopen()?;
13101341
for arm in arms {
@@ -1335,30 +1366,32 @@ impl<'a> State<'a> {
13351366
self.print_block(&blk)?;
13361367
}
13371368
hir::ExprAssign(ref lhs, ref rhs) => {
1338-
self.print_expr(&lhs)?;
1369+
let prec = AssocOp::Assign.precedence() as i8;
1370+
self.print_expr_maybe_paren(&lhs, prec + 1)?;
13391371
self.s.space()?;
13401372
self.word_space("=")?;
1341-
self.print_expr(&rhs)?;
1373+
self.print_expr_maybe_paren(&rhs, prec)?;
13421374
}
13431375
hir::ExprAssignOp(op, ref lhs, ref rhs) => {
1344-
self.print_expr(&lhs)?;
1376+
let prec = AssocOp::Assign.precedence() as i8;
1377+
self.print_expr_maybe_paren(&lhs, prec + 1)?;
13451378
self.s.space()?;
13461379
self.s.word(op.node.as_str())?;
13471380
self.word_space("=")?;
1348-
self.print_expr(&rhs)?;
1381+
self.print_expr_maybe_paren(&rhs, prec)?;
13491382
}
13501383
hir::ExprField(ref expr, name) => {
1351-
self.print_expr(&expr)?;
1384+
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX)?;
13521385
self.s.word(".")?;
13531386
self.print_name(name.node)?;
13541387
}
13551388
hir::ExprTupField(ref expr, id) => {
1356-
self.print_expr(&expr)?;
1389+
self.print_expr_maybe_paren(&expr, parser::PREC_POSTFIX)?;
13571390
self.s.word(".")?;
13581391
self.print_usize(id.node)?;
13591392
}
13601393
hir::ExprIndex(ref expr, ref index) => {
1361-
self.print_expr(&expr)?;
1394+
self.print_expr_maybe_paren(&expr, parser::PREC_POSTFIX)?;
13621395
self.s.word("[")?;
13631396
self.print_expr(&index)?;
13641397
self.s.word("]")?;
@@ -1374,7 +1407,7 @@ impl<'a> State<'a> {
13741407
self.s.space()?;
13751408
}
13761409
if let Some(ref expr) = *opt_expr {
1377-
self.print_expr(expr)?;
1410+
self.print_expr_maybe_paren(expr, parser::PREC_JUMP)?;
13781411
self.s.space()?;
13791412
}
13801413
}
@@ -1391,7 +1424,7 @@ impl<'a> State<'a> {
13911424
match *result {
13921425
Some(ref expr) => {
13931426
self.s.word(" ")?;
1394-
self.print_expr(&expr)?;
1427+
self.print_expr_maybe_paren(&expr, parser::PREC_JUMP)?;
13951428
}
13961429
_ => (),
13971430
}
@@ -1463,7 +1496,7 @@ impl<'a> State<'a> {
14631496
}
14641497
hir::ExprYield(ref expr) => {
14651498
self.s.word("yield")?;
1466-
self.print_expr(&expr)?;
1499+
self.print_expr_maybe_paren(&expr, parser::PREC_JUMP)?;
14671500
}
14681501
}
14691502
self.ann.post(self, NodeExpr(expr))?;
@@ -2246,3 +2279,112 @@ fn stmt_ends_with_semi(stmt: &hir::Stmt_) -> bool {
22462279
}
22472280
}
22482281
}
2282+
2283+
2284+
fn expr_precedence(expr: &hir::Expr) -> i8 {
2285+
use syntax::util::parser::*;
2286+
2287+
match expr.node {
2288+
hir::ExprClosure(..) => PREC_CLOSURE,
2289+
2290+
hir::ExprBreak(..) |
2291+
hir::ExprAgain(..) |
2292+
hir::ExprRet(..) |
2293+
hir::ExprYield(..) => PREC_JUMP,
2294+
2295+
hir::ExprIf(..) |
2296+
hir::ExprWhile(..) |
2297+
hir::ExprLoop(..) |
2298+
hir::ExprMatch(..) |
2299+
hir::ExprBlock(..) => PREC_BLOCK,
2300+
2301+
// Binop-like expr kinds, handled by `AssocOp`.
2302+
hir::ExprBinary(op, _, _) => bin_op_to_assoc_op(op.node).precedence() as i8,
2303+
2304+
hir::ExprCast(..) => AssocOp::As.precedence() as i8,
2305+
hir::ExprType(..) => AssocOp::Colon.precedence() as i8,
2306+
2307+
hir::ExprAssign(..) |
2308+
hir::ExprAssignOp(..) => AssocOp::Assign.precedence() as i8,
2309+
2310+
// Unary, prefix
2311+
hir::ExprBox(..) |
2312+
hir::ExprAddrOf(..) |
2313+
hir::ExprUnary(..) => PREC_PREFIX,
2314+
2315+
// Unary, postfix
2316+
hir::ExprCall(..) |
2317+
hir::ExprMethodCall(..) |
2318+
hir::ExprField(..) |
2319+
hir::ExprTupField(..) |
2320+
hir::ExprIndex(..) |
2321+
hir::ExprInlineAsm(..) => PREC_POSTFIX,
2322+
2323+
// Never need parens
2324+
hir::ExprArray(..) |
2325+
hir::ExprRepeat(..) |
2326+
hir::ExprTup(..) |
2327+
hir::ExprLit(..) |
2328+
hir::ExprPath(..) |
2329+
hir::ExprStruct(..) => PREC_PAREN,
2330+
}
2331+
}
2332+
2333+
fn bin_op_to_assoc_op(op: hir::BinOp_) -> AssocOp {
2334+
use hir::BinOp_::*;
2335+
match op {
2336+
BiAdd => AssocOp::Add,
2337+
BiSub => AssocOp::Subtract,
2338+
BiMul => AssocOp::Multiply,
2339+
BiDiv => AssocOp::Divide,
2340+
BiRem => AssocOp::Modulus,
2341+
2342+
BiAnd => AssocOp::LAnd,
2343+
BiOr => AssocOp::LOr,
2344+
2345+
BiBitXor => AssocOp::BitXor,
2346+
BiBitAnd => AssocOp::BitAnd,
2347+
BiBitOr => AssocOp::BitOr,
2348+
BiShl => AssocOp::ShiftLeft,
2349+
BiShr => AssocOp::ShiftRight,
2350+
2351+
BiEq => AssocOp::Equal,
2352+
BiLt => AssocOp::Less,
2353+
BiLe => AssocOp::LessEqual,
2354+
BiNe => AssocOp::NotEqual,
2355+
BiGe => AssocOp::GreaterEqual,
2356+
BiGt => AssocOp::Greater,
2357+
}
2358+
}
2359+
2360+
/// Expressions that syntactically contain an "exterior" struct literal i.e. not surrounded by any
2361+
/// parens or other delimiters, e.g. `X { y: 1 }`, `X { y: 1 }.method()`, `foo == X { y: 1 }` and
2362+
/// `X { y: 1 } == foo` all do, but `(X { y: 1 }) == foo` does not.
2363+
fn contains_exterior_struct_lit(value: &hir::Expr) -> bool {
2364+
match value.node {
2365+
hir::ExprStruct(..) => true,
2366+
2367+
hir::ExprAssign(ref lhs, ref rhs) |
2368+
hir::ExprAssignOp(_, ref lhs, ref rhs) |
2369+
hir::ExprBinary(_, ref lhs, ref rhs) => {
2370+
// X { y: 1 } + X { y: 2 }
2371+
contains_exterior_struct_lit(&lhs) || contains_exterior_struct_lit(&rhs)
2372+
}
2373+
hir::ExprUnary(_, ref x) |
2374+
hir::ExprCast(ref x, _) |
2375+
hir::ExprType(ref x, _) |
2376+
hir::ExprField(ref x, _) |
2377+
hir::ExprTupField(ref x, _) |
2378+
hir::ExprIndex(ref x, _) => {
2379+
// &X { y: 1 }, X { y: 1 }.y
2380+
contains_exterior_struct_lit(&x)
2381+
}
2382+
2383+
hir::ExprMethodCall(.., ref exprs) => {
2384+
// X { y: 1 }.bar(...)
2385+
contains_exterior_struct_lit(&exprs[0])
2386+
}
2387+
2388+
_ => false,
2389+
}
2390+
}

0 commit comments

Comments
 (0)