bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Rollup merge of #133603 - dtolnay:precedence, r=lcnr

Browse source 
Eliminate magic numbers from expression precedence

Context: see https://github.com/rust-lang/rust/pull/133140.

This PR continues on backporting Syn's expression precedence design into rustc. Rustc's design used mysterious integer quantities represented variously as `i8` or `usize` (e.g. `PREC_CLOSURE = -40i8`), a special significance around `0` that is never named, and an extra `PREC_FORCE_PAREN` precedence level that does not correspond to any expression. Syn's design uses a C-like enum with variants that clearly correspond to specific sets of expression kinds.

This PR is a refactoring that has no intended behavior change on its own, but it unblocks other precedence work that rustc's precedence design was poorly suited to accommodate.

- Asymmetrical precedence, so that a pretty-printer can tell `(return 1) + 1` needs parens but `1 + return 1` does not.

- Squashing the `Closure` and `Jump` cases into a single precedence level.

- Numerous remaining false positives and false negatives in rustc pretty-printer's parenthesization of macro metavariables, for example in `$e < rhs` where $e is `lhs as Thing<T>`.

FYI `@fmease` &mdash; you don't need to review if rustbot picks someone else, but you mentioned being interested in the followup PRs.
This commit is contained in:
Guillaume Gomez 2024-12-02 17:36:03 +01:00 • committed by GitHub
commit 7dd0c8314d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 220 additions and 196 deletions
Download patch file Download diff file Expand all files Collapse all files

3
compiler/rustc_parse/src/errors.rs
View file

@ -3,6 +3,7 @@
use std::borrow::Cow;
use rustc_ast::token::Token;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_ast::{Path, Visibility};
use rustc_errors::codes::*;
use rustc_errors::{
@ -2686,7 +2687,7 @@ pub(crate) struct UnexpectedExpressionInPattern {
/// Was a `RangePatternBound` expected?
pub is_bound: bool,
/// The unexpected expr's precedence (used in match arm guard suggestions).
pub expr_precedence: i8,
pub expr_precedence: ExprPrecedence,
}
#[derive(Subdiagnostic)]

35
compiler/rustc_parse/src/parser/expr.rs
View file

@ -1,7 +1,7 @@
// ignore-tidy-filelength
use core::mem;
use core::ops::ControlFlow;
use core::ops::{Bound, ControlFlow};
use ast::mut_visit::{self, MutVisitor};
use ast::token::IdentIsRaw;
@ -10,7 +10,7 @@ use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter, Token, TokenKind};
use rustc_ast::util::case::Case;
use rustc_ast::util::classify;
use rustc_ast::util::parser::{AssocOp, Fixity, prec_let_scrutinee_needs_par};
use rustc_ast::util::parser::{AssocOp, ExprPrecedence, Fixity, prec_let_scrutinee_needs_par};
use rustc_ast::visit::{Visitor, walk_expr};
use rustc_ast::{
self as ast, AnonConst, Arm, AttrStyle, AttrVec, BinOp, BinOpKind, BlockCheckMode, CaptureBy,
@ -120,7 +120,7 @@ impl<'a> Parser<'a> {
r: Restrictions,
attrs: AttrWrapper,
) -> PResult<'a, (P<Expr>, bool)> {
self.with_res(r, |this| this.parse_expr_assoc_with(0, attrs))
self.with_res(r, |this| this.parse_expr_assoc_with(Bound::Unbounded, attrs))
}
/// Parses an associative expression with operators of at least `min_prec` precedence.
@ -128,7 +128,7 @@ impl<'a> Parser<'a> {
/// followed by a subexpression (e.g. `1 + 2`).
pub(super) fn parse_expr_assoc_with(
&mut self,
min_prec: usize,
min_prec: Bound<ExprPrecedence>,
attrs: AttrWrapper,
) -> PResult<'a, (P<Expr>, bool)> {
let lhs = if self.token.is_range_separator() {
@ -144,7 +144,7 @@ impl<'a> Parser<'a> {
/// was actually parsed.
pub(super) fn parse_expr_assoc_rest_with(
&mut self,
min_prec: usize,
min_prec: Bound<ExprPrecedence>,
starts_stmt: bool,
mut lhs: P<Expr>,
) -> PResult<'a, (P<Expr>, bool)> {
@ -163,7 +163,11 @@ impl<'a> Parser<'a> {
self.restrictions
};
let prec = op.node.precedence();
if prec < min_prec {
if match min_prec {
Bound::Included(min_prec) => prec < min_prec,
Bound::Excluded(min_prec) => prec <= min_prec,
Bound::Unbounded => false,
} {
break;
}
// Check for deprecated `...` syntax
@ -276,16 +280,16 @@ impl<'a> Parser<'a> {
}
let fixity = op.fixity();
let prec_adjustment = match fixity {
Fixity::Right => 0,
Fixity::Left => 1,
let min_prec = match fixity {
Fixity::Right => Bound::Included(prec),
Fixity::Left => Bound::Excluded(prec),
// We currently have no non-associative operators that are not handled above by
// the special cases. The code is here only for future convenience.
Fixity::None => 1,
Fixity::None => Bound::Excluded(prec),
};
let (rhs, _) = self.with_res(restrictions - Restrictions::STMT_EXPR, |this| {
let attrs = this.parse_outer_attributes()?;
this.parse_expr_assoc_with(prec + prec_adjustment, attrs)
this.parse_expr_assoc_with(min_prec, attrs)
})?;
let span = self.mk_expr_sp(&lhs, lhs_span, rhs.span);
@ -451,7 +455,7 @@ impl<'a> Parser<'a> {
/// The other two variants are handled in `parse_prefix_range_expr` below.
fn parse_expr_range(
&mut self,
prec: usize,
prec: ExprPrecedence,
lhs: P<Expr>,
op: AssocOp,
cur_op_span: Span,
@ -460,7 +464,7 @@ impl<'a> Parser<'a> {
let maybe_lt = self.token.clone();
let attrs = self.parse_outer_attributes()?;
Some(
self.parse_expr_assoc_with(prec + 1, attrs)
self.parse_expr_assoc_with(Bound::Excluded(prec), attrs)
.map_err(|err| self.maybe_err_dotdotlt_syntax(maybe_lt, err))?
.0,
)
@ -518,7 +522,7 @@ impl<'a> Parser<'a> {
let (span, opt_end) = if this.is_at_start_of_range_notation_rhs() {
// RHS must be parsed with more associativity than the dots.
let attrs = this.parse_outer_attributes()?;
this.parse_expr_assoc_with(op.unwrap().precedence() + 1, attrs)
this.parse_expr_assoc_with(Bound::Excluded(op.unwrap().precedence()), attrs)
.map(|(x, _)| (lo.to(x.span), Some(x)))
.map_err(|err| this.maybe_err_dotdotlt_syntax(maybe_lt, err))?
} else {
@ -2643,7 +2647,8 @@ impl<'a> Parser<'a> {
self.expect(&token::Eq)?;
}
let attrs = self.parse_outer_attributes()?;
let (expr, _) = self.parse_expr_assoc_with(1 + prec_let_scrutinee_needs_par(), attrs)?;
let (expr, _) =
self.parse_expr_assoc_with(Bound::Excluded(prec_let_scrutinee_needs_par()), attrs)?;
let span = lo.to(expr.span);
Ok(self.mk_expr(span, ExprKind::Let(pat, expr, span, recovered)))
}

23
compiler/rustc_parse/src/parser/pat.rs
View file

@ -1,12 +1,13 @@
use std::ops::Bound;
use rustc_ast::mut_visit::{self, MutVisitor};
use rustc_ast::ptr::P;
use rustc_ast::token::{self, BinOpToken, Delimiter, IdentIsRaw, Token};
use rustc_ast::util::parser::AssocOp;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_ast::visit::{self, Visitor};
use rustc_ast::{
self as ast, Arm, AttrVec, BinOpKind, BindingMode, ByRef, Expr, ExprKind, LocalKind, MacCall,
Mutability, Pat, PatField, PatFieldsRest, PatKind, Path, QSelf, RangeEnd, RangeSyntax, Stmt,
StmtKind,
self as ast, Arm, AttrVec, BindingMode, ByRef, Expr, ExprKind, LocalKind, MacCall, Mutability,
Pat, PatField, PatFieldsRest, PatKind, Path, QSelf, RangeEnd, RangeSyntax, Stmt, StmtKind,
};
use rustc_ast_pretty::pprust;
use rustc_errors::{Applicability, Diag, DiagArgValue, PResult, StashKey};
@ -435,8 +436,9 @@ impl<'a> Parser<'a> {
// Parse an associative expression such as `+ expr`, `% expr`, ...
// Assignments, ranges and `|` are disabled by [`Restrictions::IS_PAT`].
let Ok((expr, _)) =
snapshot.parse_expr_assoc_rest_with(0, false, expr).map_err(|err| err.cancel())
let Ok((expr, _)) = snapshot
.parse_expr_assoc_rest_with(Bound::Unbounded, false, expr)
.map_err(|err| err.cancel())
else {
// We got a trailing method/operator, but that wasn't an expression.
return None;
@ -545,10 +547,7 @@ impl<'a> Parser<'a> {
// HACK: a neater way would be preferable.
let expr = match &err.args["expr_precedence"] {
DiagArgValue::Number(expr_precedence) => {
if *expr_precedence
<= AssocOp::from_ast_binop(BinOpKind::Eq).precedence()
as i32
{
if *expr_precedence <= ExprPrecedence::Compare as i32 {
format!("({expr})")
} else {
format!("{expr}")
@ -570,9 +569,7 @@ impl<'a> Parser<'a> {
}
Some(guard) => {
// Are parentheses required around the old guard?
let wrap_guard = guard.precedence()
<= AssocOp::from_ast_binop(BinOpKind::And).precedence()
as i8;
let wrap_guard = guard.precedence() <= ExprPrecedence::LAnd;
err.subdiagnostic(
UnexpectedExpressionInPatternSugg::UpdateGuard {

5
compiler/rustc_parse/src/parser/stmt.rs
View file

@ -1,5 +1,6 @@
use std::borrow::Cow;
use std::mem;
use std::ops::Bound;
use ast::Label;
use rustc_ast as ast;
@ -207,7 +208,7 @@ impl<'a> Parser<'a> {
// Perform this outside of the `collect_tokens` closure, since our
// outer attributes do not apply to this part of the expression.
let (expr, _) = self.with_res(Restrictions::STMT_EXPR, |this| {
this.parse_expr_assoc_rest_with(0, true, expr)
this.parse_expr_assoc_rest_with(Bound::Unbounded, true, expr)
})?;
Ok(self.mk_stmt(lo.to(self.prev_token.span), StmtKind::Expr(expr)))
} else {
@ -240,7 +241,7 @@ impl<'a> Parser<'a> {
let e = self.mk_expr(lo.to(hi), ExprKind::MacCall(mac));
let e = self.maybe_recover_from_bad_qpath(e)?;
let e = self.parse_expr_dot_or_call_with(attrs, e, lo)?;
let (e, _) = self.parse_expr_assoc_rest_with(0, false, e)?;
let (e, _) = self.parse_expr_assoc_rest_with(Bound::Unbounded, false, e)?;
StmtKind::Expr(e)
};
Ok(self.mk_stmt(lo.to(hi), kind))