From afe8d13a66e2f823847b39e8be7e038a67eaa337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 10 Jan 2018 11:40:16 -0800 Subject: [PATCH] Use single source of truth for expr precedence Introduce a new unified type that holds the expression precedence for both AST and HIR nodes. --- src/librustc/hir/mod.rs | 64 +++++++++- src/librustc/hir/print.rs | 51 +------- src/librustc_typeck/check/demand.rs | 4 +- src/libsyntax/ast.rs | 180 ++++++++++++++++++++++++++++ src/libsyntax/print/pprust.rs | 2 +- src/libsyntax/util/parser.rs | 62 +--------- 6 files changed, 248 insertions(+), 115 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 8d43b9b4aa7..0ee719d1157 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -34,13 +34,14 @@ use util::nodemap::{NodeMap, FxHashSet}; use syntax_pos::{Span, DUMMY_SP}; use syntax::codemap::{self, Spanned}; use syntax::abi::Abi; -use syntax::ast::{Ident, Name, NodeId, DUMMY_NODE_ID, AsmDialect}; +use syntax::ast::{self, Ident, Name, NodeId, DUMMY_NODE_ID, AsmDialect}; use syntax::ast::{Attribute, Lit, StrStyle, FloatTy, IntTy, UintTy, MetaItem}; use syntax::ext::hygiene::SyntaxContext; use syntax::ptr::P; use syntax::symbol::{Symbol, keywords}; use syntax::tokenstream::TokenStream; use syntax::util::ThinVec; +use syntax::ast::ExprPrecedence; use ty::AdtKind; use rustc_data_structures::indexed_vec; @@ -958,6 +959,31 @@ impl BinOp_ { } } +impl Into for BinOp_ { + fn into(self) -> ast::BinOpKind { + match self { + BiAdd => ast::BinOpKind::Add, + BiSub => ast::BinOpKind::Sub, + BiMul => ast::BinOpKind::Mul, + BiDiv => ast::BinOpKind::Div, + BiRem => ast::BinOpKind::Rem, + BiAnd => ast::BinOpKind::And, + BiOr => ast::BinOpKind::Or, + BiBitXor => ast::BinOpKind::BitXor, + BiBitAnd => ast::BinOpKind::BitAnd, + BiBitOr => ast::BinOpKind::BitOr, + BiShl => ast::BinOpKind::Shl, + BiShr => ast::BinOpKind::Shr, + BiEq => ast::BinOpKind::Eq, + BiLt => ast::BinOpKind::Lt, + BiLe => ast::BinOpKind::Le, + BiNe => ast::BinOpKind::Ne, + BiGe => ast::BinOpKind::Ge, + BiGt => ast::BinOpKind::Gt, + } + } +} + pub type BinOp = Spanned; #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] @@ -1166,6 +1192,42 @@ pub struct Expr { pub hir_id: HirId, } +impl Expr { + pub fn precedence(&self) -> ExprPrecedence { + match self.node { + ExprBox(_) => ExprPrecedence::Box, + ExprArray(_) => ExprPrecedence::Array, + ExprCall(..) => ExprPrecedence::Call, + ExprMethodCall(..) => ExprPrecedence::MethodCall, + ExprTup(_) => ExprPrecedence::Tup, + ExprBinary(op, ..) => ExprPrecedence::Binary(op.node.into()), + ExprUnary(..) => ExprPrecedence::Unary, + ExprLit(_) => ExprPrecedence::Lit, + ExprType(..) | ExprCast(..) => ExprPrecedence::Cast, + ExprIf(..) => ExprPrecedence::If, + ExprWhile(..) => ExprPrecedence::While, + ExprLoop(..) => ExprPrecedence::Loop, + ExprMatch(..) => ExprPrecedence::Match, + ExprClosure(..) => ExprPrecedence::Closure, + ExprBlock(..) => ExprPrecedence::Block, + ExprAssign(..) => ExprPrecedence::Assign, + ExprAssignOp(..) => ExprPrecedence::AssignOp, + ExprField(..) => ExprPrecedence::Field, + ExprTupField(..) => ExprPrecedence::TupField, + ExprIndex(..) => ExprPrecedence::Index, + ExprPath(..) => ExprPrecedence::Path, + ExprAddrOf(..) => ExprPrecedence::AddrOf, + ExprBreak(..) => ExprPrecedence::Break, + ExprAgain(..) => ExprPrecedence::Continue, + ExprRet(..) => ExprPrecedence::Ret, + ExprInlineAsm(..) => ExprPrecedence::InlineAsm, + ExprStruct(..) => ExprPrecedence::Struct, + ExprRepeat(..) => ExprPrecedence::Repeat, + ExprYield(..) => ExprPrecedence::Yield, + } + } +} + impl fmt::Debug for Expr { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "expr({}: {})", self.id, diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index a8e55674ae5..4cfa7a470a4 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -1104,7 +1104,7 @@ impl<'a> State<'a> { } pub fn print_expr_maybe_paren(&mut self, expr: &hir::Expr, prec: i8) -> io::Result<()> { - let needs_par = expr_precedence(expr) < prec; + let needs_par = expr.precedence().order() < prec; if needs_par { self.popen()?; } @@ -2318,55 +2318,6 @@ 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, - - // 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::ExprIf(..) | - hir::ExprWhile(..) | - hir::ExprLoop(..) | - hir::ExprMatch(..) | - hir::ExprBlock(..) | - hir::ExprStruct(..) => PREC_PAREN, - } -} - fn bin_op_to_assoc_op(op: hir::BinOp_) -> AssocOp { use hir::BinOp_::*; match op { diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index d43d681c925..b6b863cfea6 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -15,7 +15,7 @@ use rustc::infer::InferOk; use rustc::traits::ObligationCause; use syntax::ast; -use syntax::util::parser::{expr_precedence, AssocOp}; +use syntax::util::parser::AssocOp; use syntax_pos::{self, Span}; use rustc::hir; use rustc::hir::print; @@ -336,7 +336,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // For now, don't suggest casting with `as`. let can_cast = false; - let needs_paren = expr_precedence(expr) < (AssocOp::As.precedence() as i8); + let needs_paren = expr.precedence().order() < (AssocOp::As.precedence() as i8); if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(expr.span) { let msg = format!("you can cast an `{}` to `{}`", checked_ty, expected_ty); diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 9c0622e7bef..b45de7787a1 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -15,6 +15,17 @@ pub use self::UnsafeSource::*; pub use self::PathParameters::*; pub use symbol::{Ident, Symbol as Name}; pub use util::ThinVec; +pub use util::parser::{ + AssocOp, + PREC_RESET, + PREC_CLOSURE, + PREC_JUMP, + PREC_RANGE, + PREC_PREFIX, + PREC_POSTFIX, + PREC_PAREN, + PREC_FORCE_PAREN, +}; use syntax_pos::{Span, DUMMY_SP}; use codemap::{respan, Spanned}; @@ -28,6 +39,7 @@ use tokenstream::{ThinTokenStream, TokenStream}; use serialize::{self, Encoder, Decoder}; use std::collections::HashSet; +use std::cmp::Ordering; use std::fmt; use std::rc::Rc; use std::u32; @@ -730,6 +742,7 @@ impl BinOpKind { _ => false } } + pub fn is_comparison(&self) -> bool { use self::BinOpKind::*; match *self { @@ -740,6 +753,7 @@ impl BinOpKind { false, } } + /// Returns `true` if the binary operator takes its arguments by value pub fn is_by_value(&self) -> bool { !self.is_comparison() @@ -903,6 +917,129 @@ pub struct Expr { pub attrs: ThinVec } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExprPrecedence { + Closure, + Break, + Continue, + Ret, + Yield, + + Range, + + Binary(BinOpKind), + + InPlace, + Cast, + Type, + + Assign, + AssignOp, + + Box, + AddrOf, + Unary, + + Call, + MethodCall, + Field, + TupField, + Index, + Try, + InlineAsm, + Mac, + + Array, + Repeat, + Tup, + Lit, + Path, + Paren, + If, + IfLet, + While, + WhileLet, + ForLoop, + Loop, + Match, + Block, + Catch, + Struct, +} + +impl PartialOrd for ExprPrecedence { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.order().cmp(&other.order())) + } +} + +impl Ord for ExprPrecedence { + fn cmp(&self, other: &Self) -> Ordering { + self.order().cmp(&other.order()) + } +} + +impl ExprPrecedence { + pub fn order(self) -> i8 { + match self { + ExprPrecedence::Closure => PREC_CLOSURE, + + ExprPrecedence::Break | + ExprPrecedence::Continue | + ExprPrecedence::Ret | + ExprPrecedence::Yield => PREC_JUMP, + + // `Range` claims to have higher precedence than `Assign`, but `x .. x = x` fails to + // parse, instead of parsing as `(x .. x) = x`. Giving `Range` a lower precedence + // ensures that `pprust` will add parentheses in the right places to get the desired + // parse. + ExprPrecedence::Range => PREC_RANGE, + + // Binop-like expr kinds, handled by `AssocOp`. + ExprPrecedence::Binary(op) => AssocOp::from_ast_binop(op).precedence() as i8, + ExprPrecedence::InPlace => AssocOp::Inplace.precedence() as i8, + ExprPrecedence::Cast => AssocOp::As.precedence() as i8, + ExprPrecedence::Type => AssocOp::Colon.precedence() as i8, + + ExprPrecedence::Assign | + ExprPrecedence::AssignOp => AssocOp::Assign.precedence() as i8, + + // Unary, prefix + ExprPrecedence::Box | + ExprPrecedence::AddrOf | + ExprPrecedence::Unary => PREC_PREFIX, + + // Unary, postfix + ExprPrecedence::Call | + ExprPrecedence::MethodCall | + ExprPrecedence::Field | + ExprPrecedence::TupField | + ExprPrecedence::Index | + ExprPrecedence::Try | + ExprPrecedence::InlineAsm | + ExprPrecedence::Mac => PREC_POSTFIX, + + // Never need parens + ExprPrecedence::Array | + ExprPrecedence::Repeat | + ExprPrecedence::Tup | + ExprPrecedence::Lit | + ExprPrecedence::Path | + ExprPrecedence::Paren | + ExprPrecedence::If | + ExprPrecedence::IfLet | + ExprPrecedence::While | + ExprPrecedence::WhileLet | + ExprPrecedence::ForLoop | + ExprPrecedence::Loop | + ExprPrecedence::Match | + ExprPrecedence::Block | + ExprPrecedence::Catch | + ExprPrecedence::Struct => PREC_PAREN, + } + } +} + impl Expr { /// Wether this expression would be valid somewhere that expects a value, for example, an `if` /// condition. @@ -966,6 +1103,49 @@ impl Expr { Some(P(Ty { node, id: self.id, span: self.span })) } + + pub fn precedence(&self) -> ExprPrecedence { + match self.node { + ExprKind::Box(_) => ExprPrecedence::Box, + ExprKind::InPlace(..) => ExprPrecedence::InPlace, + ExprKind::Array(_) => ExprPrecedence::Array, + ExprKind::Call(..) => ExprPrecedence::Call, + ExprKind::MethodCall(..) => ExprPrecedence::MethodCall, + ExprKind::Tup(_) => ExprPrecedence::Tup, + ExprKind::Binary(op, ..) => ExprPrecedence::Binary(op.node), + ExprKind::Unary(..) => ExprPrecedence::Unary, + ExprKind::Lit(_) => ExprPrecedence::Lit, + ExprKind::Type(..) | ExprKind::Cast(..) => ExprPrecedence::Cast, + ExprKind::If(..) => ExprPrecedence::If, + ExprKind::IfLet(..) => ExprPrecedence::IfLet, + ExprKind::While(..) => ExprPrecedence::While, + ExprKind::WhileLet(..) => ExprPrecedence::WhileLet, + ExprKind::ForLoop(..) => ExprPrecedence::ForLoop, + ExprKind::Loop(..) => ExprPrecedence::Loop, + ExprKind::Match(..) => ExprPrecedence::Match, + ExprKind::Closure(..) => ExprPrecedence::Closure, + ExprKind::Block(..) => ExprPrecedence::Block, + ExprKind::Catch(..) => ExprPrecedence::Catch, + ExprKind::Assign(..) => ExprPrecedence::Assign, + ExprKind::AssignOp(..) => ExprPrecedence::AssignOp, + ExprKind::Field(..) => ExprPrecedence::Field, + ExprKind::TupField(..) => ExprPrecedence::TupField, + ExprKind::Index(..) => ExprPrecedence::Index, + ExprKind::Range(..) => ExprPrecedence::Range, + ExprKind::Path(..) => ExprPrecedence::Path, + ExprKind::AddrOf(..) => ExprPrecedence::AddrOf, + ExprKind::Break(..) => ExprPrecedence::Break, + ExprKind::Continue(..) => ExprPrecedence::Continue, + ExprKind::Ret(..) => ExprPrecedence::Ret, + ExprKind::InlineAsm(..) => ExprPrecedence::InlineAsm, + ExprKind::Mac(..) => ExprPrecedence::Mac, + ExprKind::Struct(..) => ExprPrecedence::Struct, + ExprKind::Repeat(..) => ExprPrecedence::Repeat, + ExprKind::Paren(..) => ExprPrecedence::Paren, + ExprKind::Try(..) => ExprPrecedence::Try, + ExprKind::Yield(..) => ExprPrecedence::Yield, + } + } } impl fmt::Debug for Expr { diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 5374bf180f4..ff065b57b8d 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1839,7 +1839,7 @@ impl<'a> State<'a> { } pub fn print_expr_maybe_paren(&mut self, expr: &ast::Expr, prec: i8) -> io::Result<()> { - let needs_par = parser::expr_precedence(expr) < prec; + let needs_par = expr.precedence().order() < prec; if needs_par { self.popen()?; } diff --git a/src/libsyntax/util/parser.rs b/src/libsyntax/util/parser.rs index 6014ec5aa92..a95ef17a1d5 100644 --- a/src/libsyntax/util/parser.rs +++ b/src/libsyntax/util/parser.rs @@ -9,7 +9,7 @@ // except according to those terms. use parse::token::{Token, BinOpToken}; use symbol::keywords; -use ast::{self, BinOpKind, ExprKind}; +use ast::{self, BinOpKind}; /// Associative operator with precedence. /// @@ -228,66 +228,6 @@ pub const PREC_POSTFIX: i8 = 60; pub const PREC_PAREN: i8 = 99; pub const PREC_FORCE_PAREN: i8 = 100; -pub fn expr_precedence(expr: &ast::Expr) -> i8 { - match expr.node { - ExprKind::Closure(..) => PREC_CLOSURE, - - ExprKind::Break(..) | - ExprKind::Continue(..) | - ExprKind::Ret(..) | - ExprKind::Yield(..) => PREC_JUMP, - - // `Range` claims to have higher precedence than `Assign`, but `x .. x = x` fails to parse, - // instead of parsing as `(x .. x) = x`. Giving `Range` a lower precedence ensures that - // `pprust` will add parentheses in the right places to get the desired parse. - ExprKind::Range(..) => PREC_RANGE, - - // Binop-like expr kinds, handled by `AssocOp`. - ExprKind::Binary(op, _, _) => - AssocOp::from_ast_binop(op.node).precedence() as i8, - - ExprKind::InPlace(..) => AssocOp::Inplace.precedence() as i8, - ExprKind::Cast(..) => AssocOp::As.precedence() as i8, - ExprKind::Type(..) => AssocOp::Colon.precedence() as i8, - - ExprKind::Assign(..) | - ExprKind::AssignOp(..) => AssocOp::Assign.precedence() as i8, - - // Unary, prefix - ExprKind::Box(..) | - ExprKind::AddrOf(..) | - ExprKind::Unary(..) => PREC_PREFIX, - - // Unary, postfix - ExprKind::Call(..) | - ExprKind::MethodCall(..) | - ExprKind::Field(..) | - ExprKind::TupField(..) | - ExprKind::Index(..) | - ExprKind::Try(..) | - ExprKind::InlineAsm(..) | - ExprKind::Mac(..) => PREC_POSTFIX, - - // Never need parens - ExprKind::Array(..) | - ExprKind::Repeat(..) | - ExprKind::Tup(..) | - ExprKind::Lit(..) | - ExprKind::Path(..) | - ExprKind::Paren(..) | - ExprKind::If(..) | - ExprKind::IfLet(..) | - ExprKind::While(..) | - ExprKind::WhileLet(..) | - ExprKind::ForLoop(..) | - ExprKind::Loop(..) | - ExprKind::Match(..) | - ExprKind::Block(..) | - ExprKind::Catch(..) | - ExprKind::Struct(..) => PREC_PAREN, - } -} - /// 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.