Skip to content

Commit 58242bc

Browse files
dtolnaydtolnay
committed
Reduce precedence of expressions that have an outer attr
1 parent 303e8bd commit 58242bc

File tree

10 files changed

+108
-37
lines changed

10 files changed

+108
-37
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,11 +1322,20 @@ impl Expr {
13221322
}
13231323

13241324
pub fn precedence(&self) -> ExprPrecedence {
1325+
fn prefix_attrs_precedence(attrs: &AttrVec) -> ExprPrecedence {
1326+
for attr in attrs {
1327+
if let AttrStyle::Outer = attr.style {
1328+
return ExprPrecedence::Prefix;
1329+
}
1330+
}
1331+
ExprPrecedence::Unambiguous
1332+
}
1333+
13251334
match &self.kind {
13261335
ExprKind::Closure(closure) => {
13271336
match closure.fn_decl.output {
13281337
FnRetTy::Default(_) => ExprPrecedence::Jump,
1329-
FnRetTy::Ty(_) => ExprPrecedence::Unambiguous,
1338+
FnRetTy::Ty(_) => prefix_attrs_precedence(&self.attrs),
13301339
}
13311340
}
13321341

@@ -1358,7 +1367,7 @@ impl Expr {
13581367
| ExprKind::Let(..)
13591368
| ExprKind::Unary(..) => ExprPrecedence::Prefix,
13601369

1361-
// Never need parens
1370+
// Need parens if and only if there are prefix attributes.
13621371
ExprKind::Array(_)
13631372
| ExprKind::Await(..)
13641373
| ExprKind::Block(..)
@@ -1391,7 +1400,7 @@ impl Expr {
13911400
| ExprKind::UnsafeBinderCast(..)
13921401
| ExprKind::While(..)
13931402
| ExprKind::Err(_)
1394-
| ExprKind::Dummy => ExprPrecedence::Unambiguous,
1403+
| ExprKind::Dummy => prefix_attrs_precedence(&self.attrs),
13951404
}
13961405
}
13971406

‎compiler/rustc_ast/src/mut_visit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,7 @@ fn walk_local<T: MutVisitor>(vis: &mut T, local: &mut P<Local>) {
696696
vis.visit_span(span);
697697
}
698698

699-
fn walk_attribute<T: MutVisitor>(vis: &mut T, attr: &mut Attribute) {
699+
pub fn walk_attribute<T: MutVisitor>(vis: &mut T, attr: &mut Attribute) {
700700
let Attribute { kind, id: _, style: _, span } = attr;
701701
match kind {
702702
AttrKind::Normal(normal) => {

‎compiler/rustc_hir/src/hir.rs

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

19441944
impl Expr<'_> {
1945-
pub fn precedence(&self) -> ExprPrecedence {
1945+
pub fn precedence(
1946+
&self,
1947+
for_each_attr: &dyn Fn(HirId, &mut dyn FnMut(&Attribute)),
1948+
) -> ExprPrecedence {
1949+
let prefix_attrs_precedence = || -> ExprPrecedence {
1950+
let mut has_outer_attr = false;
1951+
for_each_attr(self.hir_id, &mut |attr: &Attribute| {
1952+
has_outer_attr |= matches!(attr.style, AttrStyle::Outer)
1953+
});
1954+
if has_outer_attr { ExprPrecedence::Prefix } else { ExprPrecedence::Unambiguous }
1955+
};
1956+
19461957
match &self.kind {
19471958
ExprKind::Closure(closure) => {
19481959
match closure.fn_decl.output {
19491960
FnRetTy::DefaultReturn(_) => ExprPrecedence::Jump,
1950-
FnRetTy::Return(_) => ExprPrecedence::Unambiguous,
1961+
FnRetTy::Return(_) => prefix_attrs_precedence(),
19511962
}
19521963
}
19531964

@@ -1972,7 +1983,7 @@ impl Expr<'_> {
19721983
| ExprKind::Let(..)
19731984
| ExprKind::Unary(..) => ExprPrecedence::Prefix,
19741985

1975-
// Never need parens
1986+
// Need parens if and only if there are prefix attributes.
19761987
ExprKind::Array(_)
19771988
| ExprKind::Block(..)
19781989
| ExprKind::Call(..)
@@ -1993,9 +2004,9 @@ impl Expr<'_> {
19932004
| ExprKind::Tup(_)
19942005
| ExprKind::Type(..)
19952006
| ExprKind::UnsafeBinderCast(..)
1996-
| ExprKind::Err(_) => ExprPrecedence::Unambiguous,
2007+
| ExprKind::Err(_) => prefix_attrs_precedence(),
19972008

1998-
ExprKind::DropTemps(expr, ..) => expr.precedence(),
2009+
ExprKind::DropTemps(expr, ..) => expr.precedence(for_each_attr),
19992010
}
20002011
}
20012012

‎compiler/rustc_hir_pretty/src/lib.rs

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

81+
fn precedence(&self, expr: &hir::Expr<'_>) -> ExprPrecedence {
82+
let for_each_attr = |id: HirId, callback: &mut dyn FnMut(&hir::Attribute)| {
83+
self.attrs(id).iter().for_each(callback);
84+
};
85+
expr.precedence(&for_each_attr)
86+
}
87+
8188
fn print_inner_attributes(&mut self, attrs: &[hir::Attribute]) -> bool {
8289
self.print_either_attributes(attrs, ast::AttrStyle::Inner, false, true)
8390
}
@@ -1155,7 +1162,7 @@ impl<'a> State<'a> {
11551162
}
11561163
self.space();
11571164
self.word_space("=");
1158-
let npals = || parser::needs_par_as_let_scrutinee(init.precedence());
1165+
let npals = || parser::needs_par_as_let_scrutinee(self.precedence(init));
11591166
self.print_expr_cond_paren(init, Self::cond_needs_par(init) || npals())
11601167
}
11611168

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

12681275
self.print_expr_cond_paren(func, needs_paren);
@@ -1276,7 +1283,10 @@ impl<'a> State<'a> {
12761283
args: &[hir::Expr<'_>],
12771284
) {
12781285
let base_args = args;
1279-
self.print_expr_cond_paren(receiver, receiver.precedence() < ExprPrecedence::Unambiguous);
1286+
self.print_expr_cond_paren(
1287+
receiver,
1288+
self.precedence(receiver) < ExprPrecedence::Unambiguous,
1289+
);
12801290
self.word(".");
12811291
self.print_ident(segment.ident);
12821292

@@ -1291,8 +1301,8 @@ impl<'a> State<'a> {
12911301
fn print_expr_binary(&mut self, op: hir::BinOp, lhs: &hir::Expr<'_>, rhs: &hir::Expr<'_>) {
12921302
let assoc_op = AssocOp::from_ast_binop(op.node);
12931303
let binop_prec = assoc_op.precedence();
1294-
let left_prec = lhs.precedence();
1295-
let right_prec = rhs.precedence();
1304+
let left_prec = self.precedence(lhs);
1305+
let right_prec = self.precedence(rhs);
12961306

12971307
let (mut left_needs_paren, right_needs_paren) = match assoc_op.fixity() {
12981308
Fixity::Left => (left_prec < binop_prec, right_prec <= binop_prec),
@@ -1321,7 +1331,7 @@ impl<'a> State<'a> {
13211331

13221332
fn print_expr_unary(&mut self, op: hir::UnOp, expr: &hir::Expr<'_>) {
13231333
self.word(op.as_str());
1324-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Prefix);
1334+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Prefix);
13251335
}
13261336

13271337
fn print_expr_addr_of(
@@ -1338,7 +1348,7 @@ impl<'a> State<'a> {
13381348
self.print_mutability(mutability, true);
13391349
}
13401350
}
1341-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Prefix);
1351+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Prefix);
13421352
}
13431353

13441354
fn print_literal(&mut self, lit: &hir::Lit) {
@@ -1476,7 +1486,7 @@ impl<'a> State<'a> {
14761486
self.print_literal(lit);
14771487
}
14781488
hir::ExprKind::Cast(expr, ty) => {
1479-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Cast);
1489+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Cast);
14801490
self.space();
14811491
self.word_space("as");
14821492
self.print_type(ty);
@@ -1577,25 +1587,31 @@ impl<'a> State<'a> {
15771587
self.print_block(blk);
15781588
}
15791589
hir::ExprKind::Assign(lhs, rhs, _) => {
1580-
self.print_expr_cond_paren(lhs, lhs.precedence() <= ExprPrecedence::Assign);
1590+
self.print_expr_cond_paren(lhs, self.precedence(lhs) <= ExprPrecedence::Assign);
15811591
self.space();
15821592
self.word_space("=");
1583-
self.print_expr_cond_paren(rhs, rhs.precedence() < ExprPrecedence::Assign);
1593+
self.print_expr_cond_paren(rhs, self.precedence(rhs) < ExprPrecedence::Assign);
15841594
}
15851595
hir::ExprKind::AssignOp(op, lhs, rhs) => {
1586-
self.print_expr_cond_paren(lhs, lhs.precedence() <= ExprPrecedence::Assign);
1596+
self.print_expr_cond_paren(lhs, self.precedence(lhs) <= ExprPrecedence::Assign);
15871597
self.space();
15881598
self.word(op.node.as_str());
15891599
self.word_space("=");
1590-
self.print_expr_cond_paren(rhs, rhs.precedence() < ExprPrecedence::Assign);
1600+
self.print_expr_cond_paren(rhs, self.precedence(rhs) < ExprPrecedence::Assign);
15911601
}
15921602
hir::ExprKind::Field(expr, ident) => {
1593-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Unambiguous);
1603+
self.print_expr_cond_paren(
1604+
expr,
1605+
self.precedence(expr) < ExprPrecedence::Unambiguous,
1606+
);
15941607
self.word(".");
15951608
self.print_ident(ident);
15961609
}
15971610
hir::ExprKind::Index(expr, index, _) => {
1598-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Unambiguous);
1611+
self.print_expr_cond_paren(
1612+
expr,
1613+
self.precedence(expr) < ExprPrecedence::Unambiguous,
1614+
);
15991615
self.word("[");
16001616
self.print_expr(index);
16011617
self.word("]");
@@ -1609,7 +1625,7 @@ impl<'a> State<'a> {
16091625
}
16101626
if let Some(expr) = opt_expr {
16111627
self.space();
1612-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Jump);
1628+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Jump);
16131629
}
16141630
}
16151631
hir::ExprKind::Continue(destination) => {
@@ -1623,13 +1639,13 @@ impl<'a> State<'a> {
16231639
self.word("return");
16241640
if let Some(expr) = result {
16251641
self.word(" ");
1626-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Jump);
1642+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Jump);
16271643
}
16281644
}
16291645
hir::ExprKind::Become(result) => {
16301646
self.word("become");
16311647
self.word(" ");
1632-
self.print_expr_cond_paren(result, result.precedence() < ExprPrecedence::Jump);
1648+
self.print_expr_cond_paren(result, self.precedence(result) < ExprPrecedence::Jump);
16331649
}
16341650
hir::ExprKind::InlineAsm(asm) => {
16351651
self.word("asm!");
@@ -1667,7 +1683,7 @@ impl<'a> State<'a> {
16671683
}
16681684
hir::ExprKind::Yield(expr, _) => {
16691685
self.word_space("yield");
1670-
self.print_expr_cond_paren(expr, expr.precedence() < ExprPrecedence::Jump);
1686+
self.print_expr_cond_paren(expr, self.precedence(expr) < ExprPrecedence::Jump);
16711687
}
16721688
hir::ExprKind::Err(_) => {
16731689
self.popen();

‎compiler/rustc_hir_typeck/src/callee.rs

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

607607
if let Ok(rest_snippet) = rest_snippet {
608-
let sugg = if callee_expr.precedence() >= ExprPrecedence::Unambiguous {
608+
let sugg = if self.precedence(callee_expr) >= ExprPrecedence::Unambiguous {
609609
vec![
610610
(up_to_rcvr_span, "".to_string()),
611611
(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
@@ -1105,7 +1105,7 @@ impl<'a, 'tcx> CastCheck<'tcx> {
11051105
}
11061106

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

11111111
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;
@@ -18,7 +19,7 @@ use rustc_hir::def::{CtorKind, DefKind, Res};
1819
use rustc_hir::def_id::DefId;
1920
use rustc_hir::intravisit::Visitor;
2021
use rustc_hir::lang_items::LangItem;
21-
use rustc_hir::{ExprKind, HirId, QPath};
22+
use rustc_hir::{Attribute, ExprKind, HirId, QPath};
2223
use rustc_hir_analysis::hir_ty_lowering::{FeedConstTy, HirTyLowerer as _};
2324
use rustc_infer::infer;
2425
use rustc_infer::infer::{DefineOpaqueTypes, InferOk};
@@ -55,6 +56,30 @@ use crate::{
5556
};
5657

5758
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
59+
pub(crate) fn precedence(&self, expr: &hir::Expr<'_>) -> ExprPrecedence {
60+
let for_each_attr = |id: HirId, callback: &mut dyn FnMut(&Attribute)| {
61+
for attr in self.tcx.hir().attrs(id) {
62+
// For the purpose of rendering suggestions, disregard attributes
63+
// that originate from desugaring of any kind. For example, `x?`
64+
// desugars to `#[allow(unreachable_code)] match ...`. Failing to
65+
// ignore the prefix attribute in the desugaring would cause this
66+
// suggestion:
67+
//
68+
// let y: u32 = x?.try_into().unwrap();
69+
// ++++++++++++++++++++
70+
//
71+
// to be rendered as:
72+
//
73+
// let y: u32 = (x?).try_into().unwrap();
74+
// + +++++++++++++++++++++
75+
if attr.span.desugaring_kind().is_none() {
76+
callback(attr);
77+
}
78+
}
79+
};
80+
expr.precedence(&for_each_attr)
81+
}
82+
5883
/// Check an expr with an expectation type, and also demand that the expr's
5984
/// evaluated type is a subtype of the expectation at the end. This is a
6085
/// *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
@@ -396,7 +396,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
396396
// so we remove the user's `clone` call.
397397
{
398398
vec![(receiver_method.ident.span, conversion_method.name.to_string())]
399-
} else if expr.precedence() < ExprPrecedence::Unambiguous {
399+
} else if self.precedence(expr) < ExprPrecedence::Unambiguous {
400400
vec![
401401
(expr.span.shrink_to_lo(), "(".to_string()),
402402
(expr.span.shrink_to_hi(), format!(").{}()", conversion_method.name)),
@@ -1374,7 +1374,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13741374
{
13751375
let span = expr.span.find_oldest_ancestor_in_same_ctxt();
13761376

1377-
let mut sugg = if expr.precedence() >= ExprPrecedence::Unambiguous {
1377+
let mut sugg = if self.precedence(expr) >= ExprPrecedence::Unambiguous {
13781378
vec![(span.shrink_to_hi(), ".into()".to_owned())]
13791379
} else {
13801380
vec![
@@ -2988,7 +2988,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
29882988
"change the type of the numeric literal from `{checked_ty}` to `{expected_ty}`",
29892989
);
29902990

2991-
let close_paren = if expr.precedence() < ExprPrecedence::Unambiguous {
2991+
let close_paren = if self.precedence(expr) < ExprPrecedence::Unambiguous {
29922992
sugg.push((expr.span.shrink_to_lo(), "(".to_string()));
29932993
")"
29942994
} else {
@@ -3013,7 +3013,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
30133013
let len = src.trim_end_matches(&checked_ty.to_string()).len();
30143014
expr.span.with_lo(expr.span.lo() + BytePos(len as u32))
30153015
},
3016-
if expr.precedence() < ExprPrecedence::Unambiguous {
3016+
if self.precedence(expr) < ExprPrecedence::Unambiguous {
30173017
// Readd `)`
30183018
format!("{expected_ty})")
30193019
} else {

‎tests/ui-fulldeps/auxiliary/parser.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ extern crate rustc_parse;
77
extern crate rustc_session;
88
extern crate rustc_span;
99

10-
use rustc_ast::ast::{DUMMY_NODE_ID, Expr};
11-
use rustc_ast::mut_visit::MutVisitor;
10+
use rustc_ast::ast::{DUMMY_NODE_ID, Attribute, Expr};
11+
use rustc_ast::mut_visit::{self, MutVisitor};
1212
use rustc_ast::node_id::NodeId;
1313
use rustc_ast::ptr::P;
1414
use rustc_ast::token;
1515
use rustc_errors::Diag;
1616
use rustc_parse::parser::Recovery;
1717
use rustc_session::parse::ParseSess;
18-
use rustc_span::{DUMMY_SP, FileName, Span};
18+
use rustc_span::{DUMMY_SP, AttrId, FileName, Span};
1919

2020
pub fn parse_expr(psess: &ParseSess, source_code: &str) -> Option<P<Expr>> {
2121
let parser = rustc_parse::unwrap_or_emit_fatal(rustc_parse::new_parser_from_source_str(
@@ -48,4 +48,9 @@ impl MutVisitor for Normalize {
4848
fn visit_span(&mut self, span: &mut Span) {
4949
*span = DUMMY_SP;
5050
}
51+
52+
fn visit_attribute(&mut self, attr: &mut Attribute) {
53+
attr.id = AttrId::from_u32(0);
54+
mut_visit::walk_attribute(self, attr);
55+
}
5156
}

‎tests/ui-fulldeps/pprust-parenthesis-insertion.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ static EXPRS: &[&str] = &[
8585
// expressions.
8686
"match 2 { _ => 1 - 1 }",
8787
"match 2 { _ => ({ 1 }) - 1 }",
88+
// Expressions with an outer attr have lower precedence than expressions
89+
// with an inner attr.
90+
"#[attr] loop {}.field",
91+
"(#[attr] loop {}).field",
92+
"loop { #![attr] }.field",
8893
// Grammar restriction: break value starting with a labeled loop is not
8994
// allowed, except if the break is also labeled.
9095
"break 'outer 'inner: loop {} + 2",

0 commit comments

Comments
 (0)