Skip to content

Commit e66bd97

Browse files
Rollup merge of #133603 - dtolnay:precedence, r=lcnr
Eliminate magic numbers from expression precedence Context: see rust-lang/rust#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.
2 parents 5e799b2 + c52fe8b commit e66bd97

6 files changed

+24
-25
lines changed

clippy_lints/src/dereference.rs

+14-14
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use clippy_utils::{
77
peel_middle_ty_refs,
88
};
99
use core::mem;
10-
use rustc_ast::util::parser::{PREC_PREFIX, PREC_UNAMBIGUOUS};
10+
use rustc_ast::util::parser::ExprPrecedence;
1111
use rustc_data_structures::fx::FxIndexMap;
1212
use rustc_errors::Applicability;
1313
use rustc_hir::def_id::DefId;
@@ -963,7 +963,7 @@ fn report<'tcx>(
963963
// expr_str (the suggestion) is never shown if is_final_ufcs is true, since it's
964964
// `expr.kind == ExprKind::Call`. Therefore, this is, afaik, always unnecessary.
965965
/*
966-
expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence() < PREC_PREFIX {
966+
expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence() < ExprPrecedence::Prefix {
967967
Cow::Owned(format!("({expr_str})"))
968968
} else {
969969
expr_str
@@ -999,13 +999,13 @@ fn report<'tcx>(
999999
data.first_expr.span,
10001000
state.msg,
10011001
|diag| {
1002-
let (precedence, calls_field) = match cx.tcx.parent_hir_node(data.first_expr.hir_id) {
1002+
let needs_paren = match cx.tcx.parent_hir_node(data.first_expr.hir_id) {
10031003
Node::Expr(e) => match e.kind {
1004-
ExprKind::Call(callee, _) if callee.hir_id != data.first_expr.hir_id => (0, false),
1005-
ExprKind::Call(..) => (PREC_UNAMBIGUOUS, matches!(expr.kind, ExprKind::Field(..))),
1006-
_ => (e.precedence(), false),
1004+
ExprKind::Call(callee, _) if callee.hir_id != data.first_expr.hir_id => false,
1005+
ExprKind::Call(..) => expr.precedence() < ExprPrecedence::Unambiguous || matches!(expr.kind, ExprKind::Field(..)),
1006+
_ => expr.precedence() < e.precedence(),
10071007
},
1008-
_ => (0, false),
1008+
_ => false,
10091009
};
10101010
let is_in_tuple = matches!(
10111011
get_parent_expr(cx, data.first_expr),
@@ -1016,7 +1016,7 @@ fn report<'tcx>(
10161016
);
10171017

10181018
let sugg = if !snip_is_macro
1019-
&& (calls_field || expr.precedence() < precedence)
1019+
&& needs_paren
10201020
&& !has_enclosing_paren(&snip)
10211021
&& !is_in_tuple
10221022
{
@@ -1049,16 +1049,16 @@ fn report<'tcx>(
10491049
}
10501050
}
10511051

1052-
let (prefix, precedence) = match mutability {
1052+
let (prefix, needs_paren) = match mutability {
10531053
Some(mutability) if !ty.is_ref() => {
10541054
let prefix = match mutability {
10551055
Mutability::Not => "&",
10561056
Mutability::Mut => "&mut ",
10571057
};
1058-
(prefix, PREC_PREFIX)
1058+
(prefix, expr.precedence() < ExprPrecedence::Prefix)
10591059
},
1060-
None if !ty.is_ref() && data.adjusted_ty.is_ref() => ("&", 0),
1061-
_ => ("", 0),
1060+
None if !ty.is_ref() && data.adjusted_ty.is_ref() => ("&", false),
1061+
_ => ("", false),
10621062
};
10631063
span_lint_hir_and_then(
10641064
cx,
@@ -1070,7 +1070,7 @@ fn report<'tcx>(
10701070
let mut app = Applicability::MachineApplicable;
10711071
let (snip, snip_is_macro) =
10721072
snippet_with_context(cx, expr.span, data.first_expr.span.ctxt(), "..", &mut app);
1073-
let sugg = if !snip_is_macro && expr.precedence() < precedence && !has_enclosing_paren(&snip) {
1073+
let sugg = if !snip_is_macro && needs_paren && !has_enclosing_paren(&snip) {
10741074
format!("{prefix}({snip})")
10751075
} else {
10761076
format!("{prefix}{snip}")
@@ -1157,7 +1157,7 @@ impl<'tcx> Dereferencing<'tcx> {
11571157
},
11581158
Some(parent) if !parent.span.from_expansion() => {
11591159
// Double reference might be needed at this point.
1160-
if parent.precedence() == PREC_UNAMBIGUOUS {
1160+
if parent.precedence() == ExprPrecedence::Unambiguous {
11611161
// Parentheses would be needed here, don't lint.
11621162
*outer_pat = None;
11631163
} else {

clippy_lints/src/loops/single_element_loop.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::source::{indent_of, snippet, snippet_with_applicability};
44
use clippy_utils::visitors::contains_break_or_continue;
55
use rustc_ast::Mutability;
6-
use rustc_ast::util::parser::PREC_PREFIX;
6+
use rustc_ast::util::parser::ExprPrecedence;
77
use rustc_errors::Applicability;
88
use rustc_hir::{BorrowKind, Expr, ExprKind, Pat, PatKind, is_range_literal};
99
use rustc_lint::LateContext;
@@ -84,7 +84,7 @@ pub(super) fn check<'tcx>(
8484
if !prefix.is_empty()
8585
&& (
8686
// Precedence of internal expression is less than or equal to precedence of `&expr`.
87-
arg_expression.precedence() <= PREC_PREFIX || is_range_literal(arg_expression)
87+
arg_expression.precedence() <= ExprPrecedence::Prefix || is_range_literal(arg_expression)
8888
)
8989
{
9090
arg_snip = format!("({arg_snip})").into();

clippy_lints/src/matches/manual_utils.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use clippy_utils::{
77
CaptureKind, can_move_expr_to_closure, is_else_clause, is_lint_allowed, is_res_lang_ctor, path_res,
88
path_to_local_id, peel_blocks, peel_hir_expr_refs, peel_hir_expr_while,
99
};
10-
use rustc_ast::util::parser::PREC_UNAMBIGUOUS;
10+
use rustc_ast::util::parser::ExprPrecedence;
1111
use rustc_errors::Applicability;
1212
use rustc_hir::LangItem::{OptionNone, OptionSome};
1313
use rustc_hir::def::Res;
@@ -117,7 +117,7 @@ where
117117
// it's being passed by value.
118118
let scrutinee = peel_hir_expr_refs(scrutinee).0;
119119
let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app);
120-
let scrutinee_str = if scrutinee.span.eq_ctxt(expr.span) && scrutinee.precedence() < PREC_UNAMBIGUOUS {
120+
let scrutinee_str = if scrutinee.span.eq_ctxt(expr.span) && scrutinee.precedence() < ExprPrecedence::Unambiguous {
121121
format!("({scrutinee_str})")
122122
} else {
123123
scrutinee_str.into()

clippy_lints/src/neg_multiply.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::consts::{self, Constant};
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::source::snippet_with_context;
44
use clippy_utils::sugg::has_enclosing_paren;
5-
use rustc_ast::util::parser::PREC_PREFIX;
5+
use rustc_ast::util::parser::ExprPrecedence;
66
use rustc_errors::Applicability;
77
use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
88
use rustc_lint::{LateContext, LateLintPass};
@@ -58,7 +58,7 @@ fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) {
5858
{
5959
let mut applicability = Applicability::MachineApplicable;
6060
let (snip, from_macro) = snippet_with_context(cx, exp.span, span.ctxt(), "..", &mut applicability);
61-
let suggestion = if !from_macro && exp.precedence() < PREC_PREFIX && !has_enclosing_paren(&snip) {
61+
let suggestion = if !from_macro && exp.precedence() < ExprPrecedence::Prefix && !has_enclosing_paren(&snip) {
6262
format!("-({snip})")
6363
} else {
6464
format!("-{snip}")

clippy_lints/src/redundant_slicing.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::source::snippet_with_context;
33
use clippy_utils::ty::is_type_lang_item;
44
use clippy_utils::{get_parent_expr, peel_middle_ty_refs};
5-
use rustc_ast::util::parser::PREC_PREFIX;
5+
use rustc_ast::util::parser::ExprPrecedence;
66
use rustc_errors::Applicability;
77
use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability};
88
use rustc_lint::{LateContext, LateLintPass, Lint};
@@ -85,7 +85,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing {
8585
let (expr_ty, expr_ref_count) = peel_middle_ty_refs(cx.typeck_results().expr_ty(expr));
8686
let (indexed_ty, indexed_ref_count) = peel_middle_ty_refs(cx.typeck_results().expr_ty(indexed));
8787
let parent_expr = get_parent_expr(cx, expr);
88-
let needs_parens_for_prefix = parent_expr.is_some_and(|parent| parent.precedence() > PREC_PREFIX);
88+
let needs_parens_for_prefix = parent_expr.is_some_and(|parent| parent.precedence() > ExprPrecedence::Prefix);
8989

9090
if expr_ty == indexed_ty {
9191
if expr_ref_count > indexed_ref_count {

clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::sugg::Sugg;
4-
use rustc_ast::util::parser::AssocOp;
4+
use rustc_ast::util::parser::ExprPrecedence;
55
use rustc_errors::Applicability;
66
use rustc_hir::{Expr, Node};
77
use rustc_hir_typeck::cast::check_cast;
@@ -44,8 +44,7 @@ pub(super) fn check<'tcx>(
4444
};
4545

4646
if let Node::Expr(parent) = cx.tcx.parent_hir_node(e.hir_id)
47-
&& parent.precedence()
48-
> i8::try_from(AssocOp::As.precedence()).expect("AssocOp always returns a precedence < 128")
47+
&& parent.precedence() > ExprPrecedence::Cast
4948
{
5049
sugg = format!("({sugg})");
5150
}

0 commit comments

Comments
 (0)