Skip to content

Commit f512c54

Browse files
dtolnaydtolnay
committed
Eliminate magic numbers from expression precedence
1 parent f828b33 commit f512c54

File tree

13 files changed

+137
-114
lines changed

13 files changed

+137
-114
lines changed

Diff for: ‎compiler/rustc_ast/src/ast.rs

+10-12
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ pub use crate::format::*;
3939
use crate::ptr::P;
4040
use crate::token::{self, CommentKind, Delimiter};
4141
use crate::tokenstream::{DelimSpan, LazyAttrTokenStream, TokenStream};
42-
use crate::util::parser::{
43-
AssocOp, PREC_CLOSURE, PREC_JUMP, PREC_PREFIX, PREC_RANGE, PREC_UNAMBIGUOUS,
44-
};
42+
use crate::util::parser::{AssocOp, ExprPrecedence};
4543

4644
/// A "Label" is an identifier of some point in sources,
4745
/// e.g. in the following code:
@@ -1317,29 +1315,29 @@ impl Expr {
13171315
Some(P(Ty { kind, id: self.id, span: self.span, tokens: None }))
13181316
}
13191317

1320-
pub fn precedence(&self) -> i8 {
1318+
pub fn precedence(&self) -> ExprPrecedence {
13211319
match self.kind {
1322-
ExprKind::Closure(..) => PREC_CLOSURE,
1320+
ExprKind::Closure(..) => ExprPrecedence::Closure,
13231321

13241322
ExprKind::Break(..)
13251323
| ExprKind::Continue(..)
13261324
| ExprKind::Ret(..)
13271325
| ExprKind::Yield(..)
13281326
| ExprKind::Yeet(..)
1329-
| ExprKind::Become(..) => PREC_JUMP,
1327+
| ExprKind::Become(..) => ExprPrecedence::Jump,
13301328

13311329
// `Range` claims to have higher precedence than `Assign`, but `x .. x = x` fails to
13321330
// parse, instead of parsing as `(x .. x) = x`. Giving `Range` a lower precedence
13331331
// ensures that `pprust` will add parentheses in the right places to get the desired
13341332
// parse.
1335-
ExprKind::Range(..) => PREC_RANGE,
1333+
ExprKind::Range(..) => ExprPrecedence::Range,
13361334

13371335
// Binop-like expr kinds, handled by `AssocOp`.
1338-
ExprKind::Binary(op, ..) => AssocOp::from_ast_binop(op.node).precedence() as i8,
1339-
ExprKind::Cast(..) => AssocOp::As.precedence() as i8,
1336+
ExprKind::Binary(op, ..) => AssocOp::from_ast_binop(op.node).precedence(),
1337+
ExprKind::Cast(..) => ExprPrecedence::Cast,
13401338

13411339
ExprKind::Assign(..) |
1342-
ExprKind::AssignOp(..) => AssocOp::Assign.precedence() as i8,
1340+
ExprKind::AssignOp(..) => ExprPrecedence::Assign,
13431341

13441342
// Unary, prefix
13451343
ExprKind::AddrOf(..)
@@ -1348,7 +1346,7 @@ impl Expr {
13481346
// need parens sometimes. E.g. we can print `(let _ = a) && b` as `let _ = a && b`
13491347
// but we need to print `(let _ = a) < b` as-is with parens.
13501348
| ExprKind::Let(..)
1351-
| ExprKind::Unary(..) => PREC_PREFIX,
1349+
| ExprKind::Unary(..) => ExprPrecedence::Prefix,
13521350

13531351
// Never need parens
13541352
ExprKind::Array(_)
@@ -1381,7 +1379,7 @@ impl Expr {
13811379
| ExprKind::Underscore
13821380
| ExprKind::While(..)
13831381
| ExprKind::Err(_)
1384-
| ExprKind::Dummy => PREC_UNAMBIGUOUS,
1382+
| ExprKind::Dummy => ExprPrecedence::Unambiguous,
13851383
}
13861384
}
13871385

Diff for: ‎compiler/rustc_ast/src/util/parser.rs

+51-23
Original file line numberDiff line numberDiff line change
@@ -128,21 +128,21 @@ impl AssocOp {
128128
}
129129

130130
/// Gets the precedence of this operator
131-
pub fn precedence(&self) -> usize {
131+
pub fn precedence(&self) -> ExprPrecedence {
132132
use AssocOp::*;
133133
match *self {
134-
As => 14,
135-
Multiply | Divide | Modulus => 13,
136-
Add | Subtract => 12,
137-
ShiftLeft | ShiftRight => 11,
138-
BitAnd => 10,
139-
BitXor => 9,
140-
BitOr => 8,
141-
Less | Greater | LessEqual | GreaterEqual | Equal | NotEqual => 7,
142-
LAnd => 6,
143-
LOr => 5,
144-
DotDot | DotDotEq => 4,
145-
Assign | AssignOp(_) => 2,
134+
As => ExprPrecedence::Cast,
135+
Multiply | Divide | Modulus => ExprPrecedence::Product,
136+
Add | Subtract => ExprPrecedence::Sum,
137+
ShiftLeft | ShiftRight => ExprPrecedence::Shift,
138+
BitAnd => ExprPrecedence::BitAnd,
139+
BitXor => ExprPrecedence::BitXor,
140+
BitOr => ExprPrecedence::BitOr,
141+
Less | Greater | LessEqual | GreaterEqual | Equal | NotEqual => ExprPrecedence::Compare,
142+
LAnd => ExprPrecedence::LAnd,
143+
LOr => ExprPrecedence::LOr,
144+
DotDot | DotDotEq => ExprPrecedence::Range,
145+
Assign | AssignOp(_) => ExprPrecedence::Assign,
146146
}
147147
}
148148

@@ -229,25 +229,53 @@ impl AssocOp {
229229
}
230230
}
231231

232-
pub const PREC_CLOSURE: i8 = -40;
233-
pub const PREC_JUMP: i8 = -30;
234-
pub const PREC_RANGE: i8 = -10;
235-
// The range 2..=14 is reserved for AssocOp binary operator precedences.
236-
pub const PREC_PREFIX: i8 = 50;
237-
pub const PREC_UNAMBIGUOUS: i8 = 60;
232+
#[derive(Clone, Copy, PartialEq, PartialOrd)]
233+
pub enum ExprPrecedence {
234+
Closure,
235+
// return, break, yield
236+
Jump,
237+
// = += -= *= /= %= &= |= ^= <<= >>=
238+
Assign,
239+
// .. ..=
240+
Range,
241+
// ||
242+
LOr,
243+
// &&
244+
LAnd,
245+
// == != < > <= >=
246+
Compare,
247+
// |
248+
BitOr,
249+
// ^
250+
BitXor,
251+
// &
252+
BitAnd,
253+
// << >>
254+
Shift,
255+
// + -
256+
Sum,
257+
// * / %
258+
Product,
259+
// as
260+
Cast,
261+
// unary - * ! & &mut
262+
Prefix,
263+
// paths, loops, function calls, array indexing, field expressions, method calls
264+
Unambiguous,
265+
}
238266

239267
/// In `let p = e`, operators with precedence `<=` this one requires parentheses in `e`.
240-
pub fn prec_let_scrutinee_needs_par() -> usize {
241-
AssocOp::LAnd.precedence()
268+
pub fn prec_let_scrutinee_needs_par() -> ExprPrecedence {
269+
ExprPrecedence::LAnd
242270
}
243271

244272
/// Suppose we have `let _ = e` and the `order` of `e`.
245273
/// Is the `order` such that `e` in `let _ = e` needs parentheses when it is on the RHS?
246274
///
247275
/// Conversely, suppose that we have `(let _ = a) OP b` and `order` is that of `OP`.
248276
/// Can we print this as `let _ = a OP b`?
249-
pub fn needs_par_as_let_scrutinee(order: i8) -> bool {
250-
order <= prec_let_scrutinee_needs_par() as i8
277+
pub fn needs_par_as_let_scrutinee(order: ExprPrecedence) -> bool {
278+
order <= prec_let_scrutinee_needs_par()
251279
}
252280

253281
/// Expressions that syntactically contain an "exterior" struct literal i.e., not surrounded by any

Diff for: ‎compiler/rustc_ast_pretty/src/pprust/state/expr.rs

+22-25
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use itertools::{Itertools, Position};
55
use rustc_ast::ptr::P;
66
use rustc_ast::util::classify;
77
use rustc_ast::util::literal::escape_byte_str_symbol;
8-
use rustc_ast::util::parser::{self, AssocOp, Fixity};
8+
use rustc_ast::util::parser::{self, AssocOp, ExprPrecedence, Fixity};
99
use rustc_ast::{
1010
self as ast, BlockCheckMode, FormatAlignment, FormatArgPosition, FormatArgsPiece, FormatCount,
1111
FormatDebugHex, FormatSign, FormatTrait, token,
@@ -214,7 +214,7 @@ impl<'a> State<'a> {
214214
fn print_expr_call(&mut self, func: &ast::Expr, args: &[P<ast::Expr>], fixup: FixupContext) {
215215
let needs_paren = match func.kind {
216216
ast::ExprKind::Field(..) => true,
217-
_ => func.precedence() < parser::PREC_UNAMBIGUOUS,
217+
_ => func.precedence() < ExprPrecedence::Unambiguous,
218218
};
219219

220220
// Independent of parenthesization related to precedence, we must
@@ -256,7 +256,7 @@ impl<'a> State<'a> {
256256
// a statement containing an expression.
257257
self.print_expr_cond_paren(
258258
receiver,
259-
receiver.precedence() < parser::PREC_UNAMBIGUOUS,
259+
receiver.precedence() < ExprPrecedence::Unambiguous,
260260
fixup,
261261
);
262262

@@ -276,7 +276,7 @@ impl<'a> State<'a> {
276276
fixup: FixupContext,
277277
) {
278278
let assoc_op = AssocOp::from_ast_binop(op.node);
279-
let binop_prec = assoc_op.precedence() as i8;
279+
let binop_prec = assoc_op.precedence();
280280
let left_prec = lhs.precedence();
281281
let right_prec = rhs.precedence();
282282

@@ -317,7 +317,7 @@ impl<'a> State<'a> {
317317
self.word(op.as_str());
318318
self.print_expr_cond_paren(
319319
expr,
320-
expr.precedence() < parser::PREC_PREFIX,
320+
expr.precedence() < ExprPrecedence::Prefix,
321321
fixup.subsequent_subexpression(),
322322
);
323323
}
@@ -339,7 +339,7 @@ impl<'a> State<'a> {
339339
}
340340
self.print_expr_cond_paren(
341341
expr,
342-
expr.precedence() < parser::PREC_PREFIX,
342+
expr.precedence() < ExprPrecedence::Prefix,
343343
fixup.subsequent_subexpression(),
344344
);
345345
}
@@ -423,10 +423,9 @@ impl<'a> State<'a> {
423423
self.print_token_literal(lit, expr.span)
424424
}
425425
ast::ExprKind::Cast(expr, ty) => {
426-
let prec = AssocOp::As.precedence() as i8;
427426
self.print_expr_cond_paren(
428427
expr,
429-
expr.precedence() < prec,
428+
expr.precedence() < ExprPrecedence::Cast,
430429
fixup.leftmost_subexpression(),
431430
);
432431
self.space();
@@ -503,7 +502,7 @@ impl<'a> State<'a> {
503502
MatchKind::Postfix => {
504503
self.print_expr_cond_paren(
505504
expr,
506-
expr.precedence() < parser::PREC_UNAMBIGUOUS,
505+
expr.precedence() < ExprPrecedence::Unambiguous,
507506
fixup,
508507
);
509508
self.word_nbsp(".match");
@@ -567,46 +566,44 @@ impl<'a> State<'a> {
567566
ast::ExprKind::Await(expr, _) => {
568567
self.print_expr_cond_paren(
569568
expr,
570-
expr.precedence() < parser::PREC_UNAMBIGUOUS,
569+
expr.precedence() < ExprPrecedence::Unambiguous,
571570
fixup,
572571
);
573572
self.word(".await");
574573
}
575574
ast::ExprKind::Assign(lhs, rhs, _) => {
576-
let prec = AssocOp::Assign.precedence() as i8;
577575
self.print_expr_cond_paren(
578576
lhs,
579-
lhs.precedence() <= prec,
577+
lhs.precedence() <= ExprPrecedence::Assign,
580578
fixup.leftmost_subexpression(),
581579
);
582580
self.space();
583581
self.word_space("=");
584582
self.print_expr_cond_paren(
585583
rhs,
586-
rhs.precedence() < prec,
584+
rhs.precedence() < ExprPrecedence::Assign,
587585
fixup.subsequent_subexpression(),
588586
);
589587
}
590588
ast::ExprKind::AssignOp(op, lhs, rhs) => {
591-
let prec = AssocOp::Assign.precedence() as i8;
592589
self.print_expr_cond_paren(
593590
lhs,
594-
lhs.precedence() <= prec,
591+
lhs.precedence() <= ExprPrecedence::Assign,
595592
fixup.leftmost_subexpression(),
596593
);
597594
self.space();
598595
self.word(op.node.as_str());
599596
self.word_space("=");
600597
self.print_expr_cond_paren(
601598
rhs,
602-
rhs.precedence() < prec,
599+
rhs.precedence() < ExprPrecedence::Assign,
603600
fixup.subsequent_subexpression(),
604601
);
605602
}
606603
ast::ExprKind::Field(expr, ident) => {
607604
self.print_expr_cond_paren(
608605
expr,
609-
expr.precedence() < parser::PREC_UNAMBIGUOUS,
606+
expr.precedence() < ExprPrecedence::Unambiguous,
610607
fixup,
611608
);
612609
self.word(".");
@@ -615,7 +612,7 @@ impl<'a> State<'a> {
615612
ast::ExprKind::Index(expr, index, _) => {
616613
self.print_expr_cond_paren(
617614
expr,
618-
expr.precedence() < parser::PREC_UNAMBIGUOUS,
615+
expr.precedence() < ExprPrecedence::Unambiguous,
619616
fixup.leftmost_subexpression(),
620617
);
621618
self.word("[");
@@ -627,7 +624,7 @@ impl<'a> State<'a> {
627624
// than `Assign`, but `x .. x = x` gives a parse error instead of `x .. (x = x)`.
628625
// Here we use a fake precedence value so that any child with lower precedence than
629626
// a "normal" binop gets parenthesized. (`LOr` is the lowest-precedence binop.)
630-
let fake_prec = AssocOp::LOr.precedence() as i8;
627+
let fake_prec = ExprPrecedence::LOr;
631628
if let Some(e) = start {
632629
self.print_expr_cond_paren(
633630
e,
@@ -662,7 +659,7 @@ impl<'a> State<'a> {
662659
expr,
663660
// Parenthesize if required by precedence, or in the
664661
// case of `break 'inner: loop { break 'inner 1 } + 1`
665-
expr.precedence() < parser::PREC_JUMP
662+
expr.precedence() < ExprPrecedence::Jump
666663
|| (opt_label.is_none() && classify::leading_labeled_expr(expr)),
667664
fixup.subsequent_subexpression(),
668665
);
@@ -681,7 +678,7 @@ impl<'a> State<'a> {
681678
self.word(" ");
682679
self.print_expr_cond_paren(
683680
expr,
684-
expr.precedence() < parser::PREC_JUMP,
681+
expr.precedence() < ExprPrecedence::Jump,
685682
fixup.subsequent_subexpression(),
686683
);
687684
}
@@ -694,7 +691,7 @@ impl<'a> State<'a> {
694691
self.word(" ");
695692
self.print_expr_cond_paren(
696693
expr,
697-
expr.precedence() < parser::PREC_JUMP,
694+
expr.precedence() < ExprPrecedence::Jump,
698695
fixup.subsequent_subexpression(),
699696
);
700697
}
@@ -704,7 +701,7 @@ impl<'a> State<'a> {
704701
self.word(" ");
705702
self.print_expr_cond_paren(
706703
result,
707-
result.precedence() < parser::PREC_JUMP,
704+
result.precedence() < ExprPrecedence::Jump,
708705
fixup.subsequent_subexpression(),
709706
);
710707
}
@@ -758,13 +755,13 @@ impl<'a> State<'a> {
758755
self.space();
759756
self.print_expr_cond_paren(
760757
expr,
761-
expr.precedence() < parser::PREC_JUMP,
758+
expr.precedence() < ExprPrecedence::Jump,
762759
fixup.subsequent_subexpression(),
763760
);
764761
}
765762
}
766763
ast::ExprKind::Try(e) => {
767-
self.print_expr_cond_paren(e, e.precedence() < parser::PREC_UNAMBIGUOUS, fixup);
764+
self.print_expr_cond_paren(e, e.precedence() < ExprPrecedence::Unambiguous, fixup);
768765
self.word("?")
769766
}
770767
ast::ExprKind::TryBlock(blk) => {

Diff for: ‎compiler/rustc_errors/src/diagnostic_impls.rs

+7
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::path::{Path, PathBuf};
66
use std::process::ExitStatus;
77

88
use rustc_abi::TargetDataLayoutErrors;
9+
use rustc_ast::util::parser::ExprPrecedence;
910
use rustc_ast_pretty::pprust;
1011
use rustc_macros::Subdiagnostic;
1112
use rustc_span::Span;
@@ -298,6 +299,12 @@ impl IntoDiagArg for hir::def::Namespace {
298299
}
299300
}
300301

302+
impl IntoDiagArg for ExprPrecedence {
303+
fn into_diag_arg(self) -> DiagArgValue {
304+
DiagArgValue::Number(self as i32)
305+
}
306+
}
307+
301308
#[derive(Clone)]
302309
pub struct DiagSymbolList<S = Symbol>(Vec<S>);
303310

0 commit comments

Comments
 (0)