Skip to content

Commit 004c64f

Browse files
dtolnaydtolnay
committed
Reduce precedence of expressions that have an outer attr
1 parent 6c8138d commit 004c64f

File tree

17 files changed

+163
-48
lines changed

17 files changed

+163
-48
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,11 +1433,20 @@ impl Expr {
14331433
}
14341434

14351435
pub fn precedence(&self) -> ExprPrecedence {
1436+
fn prefix_attrs_precedence(attrs: &AttrVec) -> ExprPrecedence {
1437+
for attr in attrs {
1438+
if let AttrStyle::Outer = attr.style {
1439+
return ExprPrecedence::Prefix;
1440+
}
1441+
}
1442+
ExprPrecedence::Unambiguous
1443+
}
1444+
14361445
match &self.kind {
14371446
ExprKind::Closure(closure) => {
14381447
match closure.fn_decl.output {
14391448
FnRetTy::Default(_) => ExprPrecedence::Jump,
1440-
FnRetTy::Ty(_) => ExprPrecedence::Unambiguous,
1449+
FnRetTy::Ty(_) => prefix_attrs_precedence(&self.attrs),
14411450
}
14421451
}
14431452

@@ -1469,7 +1478,7 @@ impl Expr {
14691478
| ExprKind::Let(..)
14701479
| ExprKind::Unary(..) => ExprPrecedence::Prefix,
14711480

1472-
// Never need parens
1481+
// Need parens if and only if there are prefix attributes.
14731482
ExprKind::Array(_)
14741483
| ExprKind::Await(..)
14751484
| ExprKind::Use(..)
@@ -1503,7 +1512,7 @@ impl Expr {
15031512
| ExprKind::UnsafeBinderCast(..)
15041513
| ExprKind::While(..)
15051514
| ExprKind::Err(_)
1506-
| ExprKind::Dummy => ExprPrecedence::Unambiguous,
1515+
| ExprKind::Dummy => prefix_attrs_precedence(&self.attrs),
15071516
}
15081517
}
15091518

‎compiler/rustc_hir/src/hir.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2280,12 +2280,23 @@ pub struct Expr<'hir> {
22802280
}
22812281

22822282
impl Expr<'_> {
2283-
pub fn precedence(&self) -> ExprPrecedence {
2283+
pub fn precedence(
2284+
&self,
2285+
for_each_attr: &dyn Fn(HirId, &mut dyn FnMut(&Attribute)),
2286+
) -> ExprPrecedence {
2287+
let prefix_attrs_precedence = || -> ExprPrecedence {
2288+
let mut has_outer_attr = false;
2289+
for_each_attr(self.hir_id, &mut |attr: &Attribute| {
2290+
has_outer_attr |= matches!(attr.style(), AttrStyle::Outer)
2291+
});
2292+
if has_outer_attr { ExprPrecedence::Prefix } else { ExprPrecedence::Unambiguous }
2293+
};
2294+
22842295
match &self.kind {
22852296
ExprKind::Closure(closure) => {
22862297
match closure.fn_decl.output {
22872298
FnRetTy::DefaultReturn(_) => ExprPrecedence::Jump,
2288-
FnRetTy::Return(_) => ExprPrecedence::Unambiguous,
2299+
FnRetTy::Return(_) => prefix_attrs_precedence(),
22892300
}
22902301
}
22912302

@@ -2310,7 +2321,7 @@ impl Expr<'_> {
23102321
| ExprKind::Let(..)
23112322
| ExprKind::Unary(..) => ExprPrecedence::Prefix,
23122323

2313-
// Never need parens
2324+
// Need parens if and only if there are prefix attributes.
23142325
ExprKind::Array(_)
23152326
| ExprKind::Block(..)
23162327
| ExprKind::Call(..)
@@ -2332,9 +2343,9 @@ impl Expr<'_> {
23322343
| ExprKind::Type(..)
23332344
| ExprKind::UnsafeBinderCast(..)
23342345
| ExprKind::Use(..)
2335-
| ExprKind::Err(_) => ExprPrecedence::Unambiguous,
2346+
| ExprKind::Err(_) => prefix_attrs_precedence(),
23362347

2337-
ExprKind::DropTemps(expr, ..) => expr.precedence(),
2348+
ExprKind::DropTemps(expr, ..) => expr.precedence(for_each_attr),
23382349
}
23392350
}
23402351

‎compiler/rustc_hir_pretty/src/lib.rs

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ impl<'a> State<'a> {
8080
(self.attrs)(id)
8181
}
8282

83+
fn precedence(&self, expr: &hir::Expr<'_>) -> ExprPrecedence {
84+
let for_each_attr = |id: HirId, callback: &mut dyn FnMut(&hir::Attribute)| {
85+
self.attrs(id).iter().for_each(callback);
86+
};
87+
expr.precedence(&for_each_attr)
88+
}
89+
8390
fn print_attrs_as_inner(&mut self, attrs: &[hir::Attribute]) {
8491
self.print_either_attributes(attrs, ast::AttrStyle::Inner)
8592
}
@@ -1164,7 +1171,7 @@ impl<'a> State<'a> {
11641171
}
11651172
self.space();
11661173
self.word_space("=");
1167-
let npals = || parser::needs_par_as_let_scrutinee(init.precedence());
1174+
let npals = || parser::needs_par_as_let_scrutinee(self.precedence(init));
11681175
self.print_expr_cond_paren(init, Self::cond_needs_par(init) || npals())
11691176
}
11701177

@@ -1265,7 +1272,7 @@ impl<'a> State<'a> {
12651272
fn print_expr_call(&mut self, func: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
12661273
let needs_paren = match func.kind {
12671274
hir::ExprKind::Field(..) => true,
1268-
_ => func.precedence() < ExprPrecedence::Unambiguous,
1275+
_ => self.precedence(func) < ExprPrecedence::Unambiguous,
12691276
};
12701277

12711278
self.print_expr_cond_paren(func, needs_paren);
@@ -1279,7 +1286,10 @@ impl<'a> State<'a> {
12791286
args: &[hir::Expr<'_>],
12801287
) {
12811288
let base_args = args;
1282-
self.print_expr_cond_paren(receiver, receiver.precedence() < ExprPrecedence::Unambiguous);
1289+
self.print_expr_cond_paren(
1290+
receiver,
1291+
self.precedence(receiver) < ExprPrecedence::Unambiguous,
1292+
);
12831293
self.word(".");
12841294
self.print_ident(segment.ident);
12851295

@@ -1293,8 +1303,8 @@ impl<'a> State<'a> {
12931303

12941304
fn print_expr_binary(&mut self, op: hir::BinOpKind, lhs: &hir::Expr<'_>, rhs: &hir::Expr<'_>) {
12951305
let binop_prec = op.precedence();
1296-
let left_prec = lhs.precedence();
1297-
let right_prec = rhs.precedence();
1306+
let left_prec = self.precedence(lhs);
1307+
let right_prec = self.precedence(rhs);
12981308

12991309
let (mut left_needs_paren, right_needs_paren) = match op.fixity() {
13001310
Fixity::Left => (left_prec < binop_prec, right_prec <= binop_prec),
@@ -1323,7 +1333,7 @@ impl<'a> State<'a> {
13231333

13241334
fn print_expr_unary(&mut self, op: hir::UnOp, expr: &hir::Expr<'_>) {
13251335
self.word(op.as_str());
1326-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Prefix);
1336+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Prefix);
13271337
}
13281338

13291339
fn print_expr_addr_of(
@@ -1340,7 +1350,7 @@ impl<'a> State<'a> {
13401350
self.print_mutability(mutability, true);
13411351
}
13421352
}
1343-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Prefix);
1353+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Prefix);
13441354
}
13451355

13461356
fn print_literal(&mut self, lit: &hir::Lit) {
@@ -1483,7 +1493,7 @@ impl<'a> State<'a> {
14831493
self.print_literal(lit);
14841494
}
14851495
hir::ExprKind::Cast(expr, ty) => {
1486-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Cast);
1496+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Cast);
14871497
self.space();
14881498
self.word_space("as");
14891499
self.print_type(ty);
@@ -1580,24 +1590,30 @@ impl<'a> State<'a> {
15801590
self.print_block(blk, cb, ib);
15811591
}
15821592
hir::ExprKind::Assign(lhs, rhs, _) => {
1583-
self.print_expr_cond_paren(lhs, lhs.precedence() <= ExprPrecedence::Assign);
1593+
self.print_expr_cond_paren(lhs, self.precedence(lhs) <= ExprPrecedence::Assign);
15841594
self.space();
15851595
self.word_space("=");
1586-
self.print_expr_cond_paren(rhs, rhs.precedence() < ExprPrecedence::Assign);
1596+
self.print_expr_cond_paren(rhs, self.precedence(rhs) < ExprPrecedence::Assign);
15871597
}
15881598
hir::ExprKind::AssignOp(op, lhs, rhs) => {
1589-
self.print_expr_cond_paren(lhs, lhs.precedence() <= ExprPrecedence::Assign);
1599+
self.print_expr_cond_paren(lhs, self.precedence(lhs) <= ExprPrecedence::Assign);
15901600
self.space();
15911601
self.word_space(op.node.as_str());
1592-
self.print_expr_cond_paren(rhs, rhs.precedence() < ExprPrecedence::Assign);
1602+
self.print_expr_cond_paren(rhs, self.precedence(rhs) < ExprPrecedence::Assign);
15931603
}
15941604
hir::ExprKind::Field(expr, ident) => {
1595-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Unambiguous);
1605+
self.print_expr_cond_paren(
1606+
expr,
1607+
self.precedence(expr) < ExprPrecedence::Unambiguous,
1608+
);
15961609
self.word(".");
15971610
self.print_ident(ident);
15981611
}
15991612
hir::ExprKind::Index(expr, index, _) => {
1600-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Unambiguous);
1613+
self.print_expr_cond_paren(
1614+
expr,
1615+
self.precedence(expr) < ExprPrecedence::Unambiguous,
1616+
);
16011617
self.word("[");
16021618
self.print_expr(index);
16031619
self.word("]");
@@ -1611,7 +1627,7 @@ impl<'a> State<'a> {
16111627
}
16121628
if let Some(expr) = opt_expr {
16131629
self.space();
1614-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Jump);
1630+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Jump);
16151631
}
16161632
}
16171633
hir::ExprKind::Continue(destination) => {
@@ -1625,13 +1641,13 @@ impl<'a> State<'a> {
16251641
self.word("return");
16261642
if let Some(expr) = result {
16271643
self.word(" ");
1628-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Jump);
1644+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Jump);
16291645
}
16301646
}
16311647
hir::ExprKind::Become(result) => {
16321648
self.word("become");
16331649
self.word(" ");
1634-
self.print_expr_cond_paren(result, result.precedence() < ExprPrecedence::Jump);
1650+
self.print_expr_cond_paren(result, self.precedence(result) < ExprPrecedence::Jump);
16351651
}
16361652
hir::ExprKind::InlineAsm(asm) => {
16371653
self.word("asm!");
@@ -1669,7 +1685,7 @@ impl<'a> State<'a> {
16691685
}
16701686
hir::ExprKind::Yield(expr, _) => {
16711687
self.word_space("yield");
1672-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Jump);
1688+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Jump);
16731689
}
16741690
hir::ExprKind::Err(_) => {
16751691
self.popen();

‎compiler/rustc_hir_typeck/src/callee.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
602602
};
603603

604604
if let Ok(rest_snippet) = rest_snippet {
605-
let sugg = if callee_expr.precedence() >= ExprPrecedence::Unambiguous {
605+
let sugg = if self.precedence(callee_expr) >= ExprPrecedence::Unambiguous {
606606
vec![
607607
(up_to_rcvr_span, "".to_string()),
608608
(rest_span, format!(".{}({rest_snippet}", segment.ident)),

‎compiler/rustc_hir_typeck/src/cast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1100,7 +1100,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {
11001100
}
11011101

11021102
fn lossy_provenance_ptr2int_lint(&self, fcx: &FnCtxt<'a, 'tcx>, t_c: ty::cast::IntTy) {
1103-
let expr_prec = self.expr.precedence();
1103+
let expr_prec = fcx.precedence(self.expr);
11041104
let needs_parens = expr_prec < ExprPrecedence::Unambiguous;
11051105

11061106
let needs_cast = !matches!(t_c, ty::cast::IntTy::U(ty::UintTy::Usize));

‎compiler/rustc_hir_typeck/src/expr.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
//! See [`rustc_hir_analysis::check`] for more context on type checking in general.
77
88
use rustc_abi::{FIRST_VARIANT, FieldIdx};
9+
use rustc_ast::util::parser::ExprPrecedence;
910
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1011
use rustc_data_structures::stack::ensure_sufficient_stack;
1112
use rustc_data_structures::unord::UnordMap;
@@ -17,7 +18,7 @@ use rustc_errors::{
1718
use rustc_hir::def::{CtorKind, DefKind, Res};
1819
use rustc_hir::def_id::DefId;
1920
use rustc_hir::lang_items::LangItem;
20-
use rustc_hir::{ExprKind, HirId, QPath};
21+
use rustc_hir::{Attribute, ExprKind, HirId, QPath};
2122
use rustc_hir_analysis::NoVariantNamed;
2223
use rustc_hir_analysis::hir_ty_lowering::{FeedConstTy, HirTyLowerer as _};
2324
use rustc_infer::infer;
@@ -54,6 +55,30 @@ use crate::{
5455
};
5556

5657
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
58+
pub(crate) fn precedence(&self, expr: &hir::Expr<'_>) -> ExprPrecedence {
59+
let for_each_attr = |id: HirId, callback: &mut dyn FnMut(&Attribute)| {
60+
for attr in self.tcx.hir_attrs(id) {
61+
// For the purpose of rendering suggestions, disregard attributes
62+
// that originate from desugaring of any kind. For example, `x?`
63+
// desugars to `#[allow(unreachable_code)] match ...`. Failing to
64+
// ignore the prefix attribute in the desugaring would cause this
65+
// suggestion:
66+
//
67+
// let y: u32 = x?.try_into().unwrap();
68+
// ++++++++++++++++++++
69+
//
70+
// to be rendered as:
71+
//
72+
// let y: u32 = (x?).try_into().unwrap();
73+
// + +++++++++++++++++++++
74+
if attr.span().desugaring_kind().is_none() {
75+
callback(attr);
76+
}
77+
}
78+
};
79+
expr.precedence(&for_each_attr)
80+
}
81+
5782
/// Check an expr with an expectation type, and also demand that the expr's
5883
/// evaluated type is a subtype of the expectation at the end. This is a
5984
/// *hard* requirement.

‎compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
399399
// so we remove the user's `clone` call.
400400
{
401401
vec![(receiver_method.ident.span, conversion_method_name.to_string())]
402-
} else if expr.precedence() < ExprPrecedence::Unambiguous {
402+
} else if self.precedence(expr) < ExprPrecedence::Unambiguous {
403403
vec![
404404
(expr.span.shrink_to_lo(), "(".to_string()),
405405
(expr.span.shrink_to_hi(), format!(").{}()", conversion_method_name)),
@@ -1395,7 +1395,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13951395
{
13961396
let span = expr.span.find_oldest_ancestor_in_same_ctxt();
13971397

1398-
let mut sugg = if expr.precedence() >= ExprPrecedence::Unambiguous {
1398+
let mut sugg = if self.precedence(expr) >= ExprPrecedence::Unambiguous {
13991399
vec![(span.shrink_to_hi(), ".into()".to_owned())]
14001400
} else {
14011401
vec![
@@ -3106,7 +3106,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
31063106
"change the type of the numeric literal from `{checked_ty}` to `{expected_ty}`",
31073107
);
31083108

3109-
let close_paren = if expr.precedence() < ExprPrecedence::Unambiguous {
3109+
let close_paren = if self.precedence(expr) < ExprPrecedence::Unambiguous {
31103110
sugg.push((expr.span.shrink_to_lo(), "(".to_string()));
31113111
")"
31123112
} else {
@@ -3131,7 +3131,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
31313131
let len = src.trim_end_matches(&checked_ty.to_string()).len();
31323132
span.with_lo(span.lo() + BytePos(len as u32))
31333133
},
3134-
if expr.precedence() < ExprPrecedence::Unambiguous {
3134+
if self.precedence(expr) < ExprPrecedence::Unambiguous {
31353135
// Readd `)`
31363136
format!("{expected_ty})")
31373137
} else {

‎compiler/rustc_lint/src/context.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use std::cell::Cell;
77
use std::slice;
88

99
use rustc_ast::BindingMode;
10+
use rustc_ast::util::parser::ExprPrecedence;
1011
use rustc_data_structures::fx::FxIndexMap;
1112
use rustc_data_structures::sync;
1213
use rustc_data_structures::unord::UnordMap;
@@ -850,6 +851,20 @@ impl<'tcx> LateContext<'tcx> {
850851
})
851852
}
852853

854+
/// Returns the effective precedence of an expression for the purpose of
855+
/// rendering diagnostic. This is not the same as the precedence that would
856+
/// be used for pretty-printing HIR by rustc_hir_pretty.
857+
pub fn precedence(&self, expr: &hir::Expr<'_>) -> ExprPrecedence {
858+
let for_each_attr = |id: hir::HirId, callback: &mut dyn FnMut(&hir::Attribute)| {
859+
for attr in self.tcx.hir_attrs(id) {
860+
if attr.span().desugaring_kind().is_none() {
861+
callback(attr);
862+
}
863+
}
864+
};
865+
expr.precedence(&for_each_attr)
866+
}
867+
853868
/// If the given expression is a local binding, find the initializer expression.
854869
/// If that initializer expression is another local binding, find its initializer again.
855870
///

‎src/tools/clippy/clippy_lints/src/casts/unnecessary_cast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ pub(super) fn check<'tcx>(
185185
Node::Expr(parent) if is_borrow_expr(cx, parent) && !is_in_allowed_macro(cx, parent) => {
186186
MaybeParenOrBlock::Block
187187
},
188-
Node::Expr(parent) if cast_expr.precedence() < parent.precedence() => MaybeParenOrBlock::Paren,
188+
Node::Expr(parent) if cx.precedence(cast_expr) < cx.precedence(parent) => MaybeParenOrBlock::Paren,
189189
_ => MaybeParenOrBlock::Nothing,
190190
};
191191

0 commit comments

Comments
 (0)