Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level #126893

Merged
merged 2 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 21 additions & 24 deletions compiler/rustc_ast/src/util/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,7 @@ pub const PREC_JUMP: i8 = -30;
pub const PREC_RANGE: i8 = -10;
// The range 2..=14 is reserved for AssocOp binary operator precedences.
pub const PREC_PREFIX: i8 = 50;
pub const PREC_POSTFIX: i8 = 60;
pub const PREC_PAREN: i8 = 99;
pub const PREC_UNAMBIGUOUS: i8 = 60;
pub const PREC_FORCE_PAREN: i8 = 100;

#[derive(Debug, Clone, Copy)]
Expand Down Expand Up @@ -325,37 +324,35 @@ impl ExprPrecedence {
| ExprPrecedence::Let
| ExprPrecedence::Unary => PREC_PREFIX,

// Unary, postfix
ExprPrecedence::Await
// Never need parens
ExprPrecedence::Array
| ExprPrecedence::Await
| ExprPrecedence::Block
| ExprPrecedence::Call
| ExprPrecedence::MethodCall
| ExprPrecedence::ConstBlock
| ExprPrecedence::Field
| ExprPrecedence::ForLoop
| ExprPrecedence::FormatArgs
| ExprPrecedence::Gen
| ExprPrecedence::If
| ExprPrecedence::Index
| ExprPrecedence::Try
| ExprPrecedence::InlineAsm
| ExprPrecedence::Lit
| ExprPrecedence::Loop
| ExprPrecedence::Mac
| ExprPrecedence::FormatArgs
| ExprPrecedence::Match
| ExprPrecedence::MethodCall
| ExprPrecedence::OffsetOf
| ExprPrecedence::PostfixMatch => PREC_POSTFIX,

// Never need parens
ExprPrecedence::Array
| ExprPrecedence::Paren
| ExprPrecedence::Path
| ExprPrecedence::PostfixMatch
| ExprPrecedence::Repeat
| ExprPrecedence::Struct
| ExprPrecedence::Try
| ExprPrecedence::TryBlock
| ExprPrecedence::Tup
| ExprPrecedence::Lit
| ExprPrecedence::Path
| ExprPrecedence::Paren
| ExprPrecedence::If
| ExprPrecedence::While
| ExprPrecedence::ForLoop
| ExprPrecedence::Loop
| ExprPrecedence::Match
| ExprPrecedence::ConstBlock
| ExprPrecedence::Block
| ExprPrecedence::TryBlock
| ExprPrecedence::Gen
| ExprPrecedence::Struct
| ExprPrecedence::Err => PREC_PAREN,
| ExprPrecedence::Err => PREC_UNAMBIGUOUS,
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_ast_pretty/src/pprust/state/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl<'a> State<'a> {
fn print_expr_call(&mut self, func: &ast::Expr, args: &[P<ast::Expr>], fixup: FixupContext) {
let prec = match func.kind {
ast::ExprKind::Field(..) => parser::PREC_FORCE_PAREN,
_ => parser::PREC_POSTFIX,
_ => parser::PREC_UNAMBIGUOUS,
};

// Independent of parenthesization related to precedence, we must
Expand Down Expand Up @@ -257,7 +257,7 @@ impl<'a> State<'a> {
// boundaries, `$receiver.method()` can be parsed back as a statement
// containing an expression if and only if `$receiver` can be parsed as
// a statement containing an expression.
self.print_expr_maybe_paren(receiver, parser::PREC_POSTFIX, fixup);
self.print_expr_maybe_paren(receiver, parser::PREC_UNAMBIGUOUS, fixup);

self.word(".");
self.print_ident(segment.ident);
Expand Down Expand Up @@ -489,7 +489,7 @@ impl<'a> State<'a> {
self.space();
}
MatchKind::Postfix => {
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX, fixup);
self.print_expr_maybe_paren(expr, parser::PREC_UNAMBIGUOUS, fixup);
self.word_nbsp(".match");
}
}
Expand Down Expand Up @@ -549,7 +549,7 @@ impl<'a> State<'a> {
self.print_block_with_attrs(blk, attrs);
}
ast::ExprKind::Await(expr, _) => {
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX, fixup);
self.print_expr_maybe_paren(expr, parser::PREC_UNAMBIGUOUS, fixup);
self.word(".await");
}
ast::ExprKind::Assign(lhs, rhs, _) => {
Expand All @@ -568,14 +568,14 @@ impl<'a> State<'a> {
self.print_expr_maybe_paren(rhs, prec, fixup.subsequent_subexpression());
}
ast::ExprKind::Field(expr, ident) => {
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX, fixup);
self.print_expr_maybe_paren(expr, parser::PREC_UNAMBIGUOUS, fixup);
self.word(".");
self.print_ident(*ident);
}
ast::ExprKind::Index(expr, index, _) => {
self.print_expr_maybe_paren(
expr,
parser::PREC_POSTFIX,
parser::PREC_UNAMBIGUOUS,
fixup.leftmost_subexpression(),
);
self.word("[");
Expand Down Expand Up @@ -713,7 +713,7 @@ impl<'a> State<'a> {
}
}
ast::ExprKind::Try(e) => {
self.print_expr_maybe_paren(e, parser::PREC_POSTFIX, fixup);
self.print_expr_maybe_paren(e, parser::PREC_UNAMBIGUOUS, fixup);
self.word("?")
}
ast::ExprKind::TryBlock(blk) => {
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ impl<'a> State<'a> {
fn print_expr_call(&mut self, func: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
let prec = match func.kind {
hir::ExprKind::Field(..) => parser::PREC_FORCE_PAREN,
_ => parser::PREC_POSTFIX,
_ => parser::PREC_UNAMBIGUOUS,
};

self.print_expr_maybe_paren(func, prec);
Expand All @@ -1134,7 +1134,7 @@ impl<'a> State<'a> {
args: &[hir::Expr<'_>],
) {
let base_args = args;
self.print_expr_maybe_paren(receiver, parser::PREC_POSTFIX);
self.print_expr_maybe_paren(receiver, parser::PREC_UNAMBIGUOUS);
self.word(".");
self.print_ident(segment.ident);

Expand Down Expand Up @@ -1478,12 +1478,12 @@ impl<'a> State<'a> {
self.print_expr_maybe_paren(rhs, prec);
}
hir::ExprKind::Field(expr, ident) => {
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
self.print_expr_maybe_paren(expr, parser::PREC_UNAMBIGUOUS);
self.word(".");
self.print_ident(ident);
}
hir::ExprKind::Index(expr, index, _) => {
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
self.print_expr_maybe_paren(expr, parser::PREC_UNAMBIGUOUS);
self.word("[");
self.print_expr(index);
self.word("]");
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::method::MethodCallee;
use super::{Expectation, FnCtxt, TupleArgumentsFlag};

use crate::errors;
use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_ast::util::parser::PREC_UNAMBIGUOUS;
use rustc_errors::{Applicability, Diag, ErrorGuaranteed, StashKey};
use rustc_hir::def::{self, CtorKind, Namespace, Res};
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -656,7 +656,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};

if let Ok(rest_snippet) = rest_snippet {
let sugg = if callee_expr.precedence().order() >= PREC_POSTFIX {
let sugg = if callee_expr.precedence().order() >= PREC_UNAMBIGUOUS {
vec![
(up_to_rcvr_span, "".to_string()),
(rest_span, format!(".{}({rest_snippet}", segment.ident)),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {

fn lossy_provenance_ptr2int_lint(&self, fcx: &FnCtxt<'a, 'tcx>, t_c: ty::cast::IntTy) {
let expr_prec = self.expr.precedence().order();
let needs_parens = expr_prec < rustc_ast::util::parser::PREC_POSTFIX;
let needs_parens = expr_prec < rustc_ast::util::parser::PREC_UNAMBIGUOUS;

let needs_cast = !matches!(t_c, ty::cast::IntTy::U(ty::UintTy::Usize));
let cast_span = self.expr_span.shrink_to_hi().to(self.cast_span);
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::method::probe::{IsSuggestion, Mode, ProbeScope};
use core::cmp::min;
use core::iter;
use hir::def_id::LocalDefId;
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
use rustc_ast::util::parser::{ExprPrecedence, PREC_UNAMBIGUOUS};
use rustc_data_structures::packed::Pu128;
use rustc_errors::{Applicability, Diag, MultiSpan};
use rustc_hir as hir;
Expand Down Expand Up @@ -1287,7 +1287,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{
let span = expr.span.find_oldest_ancestor_in_same_ctxt();

let mut sugg = if expr.precedence().order() >= PREC_POSTFIX {
let mut sugg = if expr.precedence().order() >= PREC_UNAMBIGUOUS {
vec![(span.shrink_to_hi(), ".into()".to_owned())]
} else {
vec![
Expand Down Expand Up @@ -2826,7 +2826,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"change the type of the numeric literal from `{checked_ty}` to `{expected_ty}`",
);

let close_paren = if expr.precedence().order() < PREC_POSTFIX {
let close_paren = if expr.precedence().order() < PREC_UNAMBIGUOUS {
sugg.push((expr.span.shrink_to_lo(), "(".to_string()));
")"
} else {
Expand All @@ -2851,7 +2851,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let len = src.trim_end_matches(&checked_ty.to_string()).len();
expr.span.with_lo(expr.span.lo() + BytePos(len as u32))
},
if expr.precedence().order() < PREC_POSTFIX {
if expr.precedence().order() < PREC_UNAMBIGUOUS {
// Readd `)`
format!("{expected_ty})")
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use clippy_utils::{
expr_use_ctxt, get_parent_expr, is_block_like, is_lint_allowed, path_to_local, DefinedTy, ExprUseNode,
};
use core::mem;
use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX};
use rustc_ast::util::parser::{PREC_UNAMBIGUOUS, PREC_PREFIX};
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_ty, Visitor};
Expand Down Expand Up @@ -1013,7 +1013,7 @@ fn report<'tcx>(
let (precedence, calls_field) = 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_POSTFIX, matches!(expr.kind, ExprKind::Field(..))),
ExprKind::Call(..) => (PREC_UNAMBIGUOUS, matches!(expr.kind, ExprKind::Field(..))),
_ => (e.precedence().order(), false),
},
_ => (0, false),
Expand Down Expand Up @@ -1160,7 +1160,7 @@ impl<'tcx> Dereferencing<'tcx> {
},
Some(parent) if !parent.span.from_expansion() => {
// Double reference might be needed at this point.
if parent.precedence().order() == PREC_POSTFIX {
if parent.precedence().order() == PREC_UNAMBIGUOUS {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line contains the only observable behavior change in this PR.

rust-lang/rust-clippy#12990 has a writeup of what this code is doing. It is using precedence of the parent expression of an expression x to decide whether it's possible to suggest replacing x by &x, or by (&x).

Precedence of the parent expression is the wrong thing for Clippy to be looking at in this code, and causes the false negatives reported in the link. This PR increases the number of cases which encounter that false negative in the short term, such as [x; 2], but does not make the issue harder to fix in Clippy.

// Parentheses would be needed here, don't lint.
*outer_pat = None;
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/matches/manual_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use clippy_utils::{
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, CaptureKind,
};
use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_ast::util::parser::PREC_UNAMBIGUOUS;
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::LangItem::{OptionNone, OptionSome};
Expand Down Expand Up @@ -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().order() < PREC_POSTFIX {
let scrutinee_str = if scrutinee.span.eq_ctxt(expr.span) && scrutinee.precedence().order() < PREC_UNAMBIGUOUS {
format!("({scrutinee_str})")
} else {
scrutinee_str.into()
Expand Down
Loading