user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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 e66bd976c6
6 changed files with 24 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

28
clippy_lints/src/dereference.rs
View file

@ -7,7 +7,7 @@ use clippy_utils::{
peel_middle_ty_refs,
};
use core::mem;
use rustc_ast::util::parser::{PREC_PREFIX, PREC_UNAMBIGUOUS};
use rustc_ast::util::parser::ExprPrecedence;
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
@ -963,7 +963,7 @@ fn report<'tcx>(
// expr_str (the suggestion) is never shown if is_final_ufcs is true, since it's
// `expr.kind == ExprKind::Call`. Therefore, this is, afaik, always unnecessary.
/*
expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence() < PREC_PREFIX {
expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence() < ExprPrecedence::Prefix {
Cow::Owned(format!("({expr_str})"))
} else {
expr_str
@ -999,13 +999,13 @@ fn report<'tcx>(
data.first_expr.span,
state.msg,
|diag| {
let (precedence, calls_field) = match cx.tcx.parent_hir_node(data.first_expr.hir_id) {
let needs_paren = match cx.tcx.parent_hir_node(data.first_expr.hir_id) {
Node::Expr(e) => match e.kind {
ExprKind::Call(callee, _) if callee.hir_id != data.first_expr.hir_id => (0, false),
ExprKind::Call(..) => (PREC_UNAMBIGUOUS, matches!(expr.kind, ExprKind::Field(..))),
_ => (e.precedence(), false),
ExprKind::Call(callee, _) if callee.hir_id != data.first_expr.hir_id => false,
ExprKind::Call(..) => expr.precedence() < ExprPrecedence::Unambiguous || matches!(expr.kind, ExprKind::Field(..)),
_ => expr.precedence() < e.precedence(),
},
_ => (0, false),
_ => false,
};
let is_in_tuple = matches!(
get_parent_expr(cx, data.first_expr),
@ -1016,7 +1016,7 @@ fn report<'tcx>(
);
let sugg = if !snip_is_macro
&& (calls_field || expr.precedence() < precedence)
&& needs_paren
&& !has_enclosing_paren(&snip)
&& !is_in_tuple
{
@ -1049,16 +1049,16 @@ fn report<'tcx>(
}
}
let (prefix, precedence) = match mutability {
let (prefix, needs_paren) = match mutability {
Some(mutability) if !ty.is_ref() => {
let prefix = match mutability {
Mutability::Not => "&",
Mutability::Mut => "&mut ",
};
(prefix, PREC_PREFIX)
(prefix, expr.precedence() < ExprPrecedence::Prefix)
},
None if !ty.is_ref() && data.adjusted_ty.is_ref() => ("&", 0),
_ => ("", 0),
None if !ty.is_ref() && data.adjusted_ty.is_ref() => ("&", false),
_ => ("", false),
};
span_lint_hir_and_then(
cx,
@ -1070,7 +1070,7 @@ fn report<'tcx>(
let mut app = Applicability::MachineApplicable;
let (snip, snip_is_macro) =
snippet_with_context(cx, expr.span, data.first_expr.span.ctxt(), "..", &mut app);
let sugg = if !snip_is_macro && expr.precedence() < precedence && !has_enclosing_paren(&snip) {
let sugg = if !snip_is_macro && needs_paren && !has_enclosing_paren(&snip) {
format!("{prefix}({snip})")
} else {
format!("{prefix}{snip}")
@ -1157,7 +1157,7 @@ impl<'tcx> Dereferencing<'tcx> {
},
Some(parent) if !parent.span.from_expansion() => {
// Double reference might be needed at this point.
if parent.precedence() == PREC_UNAMBIGUOUS {
if parent.precedence() == ExprPrecedence::Unambiguous {
// Parentheses would be needed here, don't lint.
*outer_pat = None;
} else {

4
clippy_lints/src/loops/single_element_loop.rs
View file

@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{indent_of, snippet, snippet_with_applicability};
use clippy_utils::visitors::contains_break_or_continue;
use rustc_ast::Mutability;
use rustc_ast::util::parser::PREC_PREFIX;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, Pat, PatKind, is_range_literal};
use rustc_lint::LateContext;
@ -84,7 +84,7 @@ pub(super) fn check<'tcx>(
if !prefix.is_empty()
&& (
// Precedence of internal expression is less than or equal to precedence of `&expr`.
arg_expression.precedence() <= PREC_PREFIX || is_range_literal(arg_expression)
arg_expression.precedence() <= ExprPrecedence::Prefix || is_range_literal(arg_expression)
)
{
arg_snip = format!("({arg_snip})").into();

4
clippy_lints/src/matches/manual_utils.rs
View file

@ -7,7 +7,7 @@ use clippy_utils::{
CaptureKind, can_move_expr_to_closure, is_else_clause, is_lint_allowed, is_res_lang_ctor, path_res,
path_to_local_id, peel_blocks, peel_hir_expr_refs, peel_hir_expr_while,
};
use rustc_ast::util::parser::PREC_UNAMBIGUOUS;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_hir::def::Res;
@ -117,7 +117,7 @@ where
// it's being passed by value.
let scrutinee = peel_hir_expr_refs(scrutinee).0;
let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app);
let scrutinee_str = if scrutinee.span.eq_ctxt(expr.span) && scrutinee.precedence() < PREC_UNAMBIGUOUS {
let scrutinee_str = if scrutinee.span.eq_ctxt(expr.span) && scrutinee.precedence() < ExprPrecedence::Unambiguous {
format!("({scrutinee_str})")
} else {
scrutinee_str.into()

4
clippy_lints/src/neg_multiply.rs
View file

@ -2,7 +2,7 @@ use clippy_utils::consts::{self, Constant};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_context;
use clippy_utils::sugg::has_enclosing_paren;
use rustc_ast::util::parser::PREC_PREFIX;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
@ -58,7 +58,7 @@ fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) {
{
let mut applicability = Applicability::MachineApplicable;
let (snip, from_macro) = snippet_with_context(cx, exp.span, span.ctxt(), "..", &mut applicability);
let suggestion = if !from_macro && exp.precedence() < PREC_PREFIX && !has_enclosing_paren(&snip) {
let suggestion = if !from_macro && exp.precedence() < ExprPrecedence::Prefix && !has_enclosing_paren(&snip) {
format!("-({snip})")
} else {
format!("-{snip}")

4
clippy_lints/src/redundant_slicing.rs
View file

@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::is_type_lang_item;
use clippy_utils::{get_parent_expr, peel_middle_ty_refs};
use rustc_ast::util::parser::PREC_PREFIX;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability};
use rustc_lint::{LateContext, LateLintPass, Lint};
@ -85,7 +85,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing {
let (expr_ty, expr_ref_count) = peel_middle_ty_refs(cx.typeck_results().expr_ty(expr));
let (indexed_ty, indexed_ref_count) = peel_middle_ty_refs(cx.typeck_results().expr_ty(indexed));
let parent_expr = get_parent_expr(cx, expr);
let needs_parens_for_prefix = parent_expr.is_some_and(|parent| parent.precedence() > PREC_PREFIX);
let needs_parens_for_prefix = parent_expr.is_some_and(|parent| parent.precedence() > ExprPrecedence::Prefix);
if expr_ty == indexed_ty {
if expr_ref_count > indexed_ref_count {

5
clippy_lints/src/transmute/transmutes_expressible_as_ptr_casts.rs
View file

@ -1,7 +1,7 @@
use super::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::sugg::Sugg;
use rustc_ast::util::parser::AssocOp;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_errors::Applicability;
use rustc_hir::{Expr, Node};
use rustc_hir_typeck::cast::check_cast;
@ -44,8 +44,7 @@ pub(super) fn check<'tcx>(
};
if let Node::Expr(parent) = cx.tcx.parent_hir_node(e.hir_id)
&& parent.precedence()
> i8::try_from(AssocOp::As.precedence()).expect("AssocOp always returns a precedence < 128")
&& parent.precedence() > ExprPrecedence::Cast
{
sugg = format!("({sugg})");
}