Skip to content

Commit 894f7a4

Browse files
committed
Auto merge of rust-lang#126678 - nnethercote:fix-duplicated-attrs-on-nt-expr, r=petrochenkov
Fix duplicated attributes on nonterminal expressions This PR fixes a long-standing bug (rust-lang#86055) whereby expression attributes can be duplicated when expanded through declarative macros. First, consider how items are parsed in declarative macros: ``` Items: - parse_nonterminal - parse_item(ForceCollect::Yes) - parse_item_ - attrs = parse_outer_attributes - parse_item_common(attrs) - maybe_whole! - collect_tokens_trailing_token ``` The important thing is that the parsing of outer attributes is outside token collection, so the item's tokens don't include the attributes. This is how it's supposed to be. Now consider how expression are parsed in declarative macros: ``` Exprs: - parse_nonterminal - parse_expr_force_collect - collect_tokens_no_attrs - collect_tokens_trailing_token - parse_expr - parse_expr_res(None) - parse_expr_assoc_with - parse_expr_prefix - parse_or_use_outer_attributes - parse_expr_dot_or_call ``` The important thing is that the parsing of outer attributes is inside token collection, so the the expr's tokens do include the attributes, i.e. in `AttributesData::tokens`. This PR fixes the bug by rearranging expression parsing to that outer attribute parsing happens outside of token collection. This requires a number of small refactorings because expression parsing is somewhat complicated. While doing so the PR makes the code a bit cleaner and simpler, by eliminating `parse_or_use_outer_attributes` and `Option<AttrWrapper>` arguments (in favour of the simpler `parse_outer_attributes` and `AttrWrapper` arguments), and simplifying `LhsExpr`. r? `@petrochenkov`
2 parents 3186d17 + 64c2e9e commit 894f7a4

File tree

13 files changed

+118
-144
lines changed

13 files changed

+118
-144
lines changed

Diff for: compiler/rustc_ast/src/ast.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,9 @@ impl UnOp {
975975
}
976976
}
977977

978-
/// A statement
978+
/// A statement. No `attrs` or `tokens` fields because each `StmtKind` variant
979+
/// contains an AST node with those fields. (Except for `StmtKind::Empty`,
980+
/// which never has attrs or tokens)
979981
#[derive(Clone, Encodable, Decodable, Debug)]
980982
pub struct Stmt {
981983
pub id: NodeId,

Diff for: compiler/rustc_parse/messages.ftl

+2
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ parse_dot_dot_dot_for_remaining_fields = expected field pattern, found `{$token_
133133
parse_dot_dot_dot_range_to_pattern_not_allowed = range-to patterns with `...` are not allowed
134134
.suggestion = use `..=` instead
135135
136+
parse_dot_dot_range_attribute = attributes are not allowed on range expressions starting with `..`
137+
136138
parse_dotdotdot = unexpected token: `...`
137139
.suggest_exclusive_range = use `..` for an exclusive range
138140
.suggest_inclusive_range = or `..=` for an inclusive range

Diff for: compiler/rustc_parse/src/errors.rs

+7
Original file line numberDiff line numberDiff line change
@@ -2990,3 +2990,10 @@ pub(crate) struct ExprRArrowCall {
29902990
#[suggestion(style = "short", applicability = "machine-applicable", code = ".")]
29912991
pub span: Span,
29922992
}
2993+
2994+
#[derive(Diagnostic)]
2995+
#[diag(parse_dot_dot_range_attribute)]
2996+
pub(crate) struct DotDotRangeAttribute {
2997+
#[primary_span]
2998+
pub span: Span,
2999+
}

Diff for: compiler/rustc_parse/src/parser/attr_wrapper.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use std::ops::Range;
1515
/// for the attribute target. This allows us to perform cfg-expansion on
1616
/// a token stream before we invoke a derive proc-macro.
1717
///
18-
/// This wrapper prevents direct access to the underlying `ast::AttrVec>`.
18+
/// This wrapper prevents direct access to the underlying `ast::AttrVec`.
1919
/// Parsing code can only get access to the underlying attributes
2020
/// by passing an `AttrWrapper` to `collect_tokens_trailing_tokens`.
2121
/// This makes it difficult to accidentally construct an AST node
@@ -177,6 +177,10 @@ impl<'a> Parser<'a> {
177177
/// into a `LazyAttrTokenStream`, and returned along with the result
178178
/// of the callback.
179179
///
180+
/// The `attrs` passed in are in `AttrWrapper` form, which is opaque. The
181+
/// `AttrVec` within is passed to `f`. See the comment on `AttrWrapper` for
182+
/// details.
183+
///
180184
/// Note: If your callback consumes an opening delimiter
181185
/// (including the case where you call `collect_tokens`
182186
/// when the current token is an opening delimiter),

Diff for: compiler/rustc_parse/src/parser/diagnostics.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -2499,7 +2499,8 @@ impl<'a> Parser<'a> {
24992499
/// wrapped in braces.
25002500
pub(super) fn handle_unambiguous_unbraced_const_arg(&mut self) -> PResult<'a, P<Expr>> {
25012501
let start = self.token.span;
2502-
let expr = self.parse_expr_res(Restrictions::CONST_EXPR, None).map_err(|mut err| {
2502+
let attrs = self.parse_outer_attributes()?;
2503+
let expr = self.parse_expr_res(Restrictions::CONST_EXPR, attrs).map_err(|mut err| {
25032504
err.span_label(
25042505
start.shrink_to_lo(),
25052506
"while parsing a const generic argument starting here",
@@ -2621,7 +2622,10 @@ impl<'a> Parser<'a> {
26212622
if is_op_or_dot {
26222623
self.bump();
26232624
}
2624-
match self.parse_expr_res(Restrictions::CONST_EXPR, None) {
2625+
match (|| {
2626+
let attrs = self.parse_outer_attributes()?;
2627+
self.parse_expr_res(Restrictions::CONST_EXPR, attrs)
2628+
})() {
26252629
Ok(expr) => {
26262630
// Find a mistake like `MyTrait<Assoc == S::Assoc>`.
26272631
if token::EqEq == snapshot.token.kind {
@@ -2675,7 +2679,10 @@ impl<'a> Parser<'a> {
26752679
&mut self,
26762680
mut snapshot: SnapshotParser<'a>,
26772681
) -> Option<P<ast::Expr>> {
2678-
match snapshot.parse_expr_res(Restrictions::CONST_EXPR, None) {
2682+
match (|| {
2683+
let attrs = self.parse_outer_attributes()?;
2684+
snapshot.parse_expr_res(Restrictions::CONST_EXPR, attrs)
2685+
})() {
26792686
// Since we don't know the exact reason why we failed to parse the type or the
26802687
// expression, employ a simple heuristic to weed out some pathological cases.
26812688
Ok(expr) if let token::Comma | token::Gt = snapshot.token.kind => {

0 commit comments

Comments
 (0)