Skip to content

Remove TrailingToken. #127842

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Remove TrailingToken. #127842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 10 additions & 22 deletions compiler/rustc_parse/src/parser/attr_wrapper.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{Capturing, FlatToken, ForceCollect, Parser, ReplaceRange, TokenCursor, TrailingToken};
use rustc_ast::token::{self, Delimiter, Token, TokenKind};
use super::{Capturing, FlatToken, ForceCollect, Parser, ReplaceRange, TokenCursor};
use rustc_ast::token::{Delimiter, Token, TokenKind};
use rustc_ast::tokenstream::{AttrTokenStream, AttrTokenTree, AttrsTarget, DelimSpacing};
use rustc_ast::tokenstream::{DelimSpan, LazyAttrTokenStream, Spacing, ToAttrTokenStream};
use rustc_ast::{self as ast};
Expand Down Expand Up @@ -165,8 +165,10 @@ impl ToAttrTokenStream for LazyAttrTokenStreamImpl {
impl<'a> Parser<'a> {
/// Records all tokens consumed by the provided callback,
/// including the current token. These tokens are collected
/// into a `LazyAttrTokenStream`, and returned along with the result
/// of the callback.
/// into a `LazyAttrTokenStream`, and returned along with the first part of
/// the callback's result. The second (bool) part of the callback's result
/// indicates if an extra token should be captured, e.g. a comma or
/// semicolon.
///
/// The `attrs` passed in are in `AttrWrapper` form, which is opaque. The
/// `AttrVec` within is passed to `f`. See the comment on `AttrWrapper` for
Expand All @@ -187,7 +189,7 @@ impl<'a> Parser<'a> {
&mut self,
attrs: AttrWrapper,
force_collect: ForceCollect,
f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, TrailingToken)>,
f: impl FnOnce(&mut Self, ast::AttrVec) -> PResult<'a, (R, bool)>,
) -> PResult<'a, R> {
// We only bail out when nothing could possibly observe the collected tokens:
// 1. We cannot be force collecting tokens (since force-collecting requires tokens
Expand All @@ -212,7 +214,7 @@ impl<'a> Parser<'a> {
let has_outer_attrs = !attrs.attrs.is_empty();
let replace_ranges_start = self.capture_state.replace_ranges.len();

let (mut ret, trailing) = {
let (mut ret, capture_trailing) = {
let prev_capturing = mem::replace(&mut self.capture_state.capturing, Capturing::Yes);
let ret_and_trailing = f(self, attrs.attrs);
self.capture_state.capturing = prev_capturing;
Expand Down Expand Up @@ -266,27 +268,13 @@ impl<'a> Parser<'a> {

let replace_ranges_end = self.capture_state.replace_ranges.len();

// Capture a trailing token if requested by the callback 'f'
let captured_trailing = match trailing {
TrailingToken::None => false,
TrailingToken::Gt => {
assert_eq!(self.token.kind, token::Gt);
false
}
TrailingToken::Semi => {
assert_eq!(self.token.kind, token::Semi);
true
}
TrailingToken::MaybeComma => self.token.kind == token::Comma,
};

assert!(
!(self.break_last_token && captured_trailing),
!(self.break_last_token && capture_trailing),
"Cannot set break_last_token and have trailing token"
);

let end_pos = self.num_bump_calls
+ captured_trailing as u32
+ capture_trailing as u32
// If we 'broke' the last token (e.g. breaking a '>>' token to two '>' tokens), then
// extend the range of captured tokens to include it, since the parser was not actually
// bumped past it. When the `LazyAttrTokenStream` gets converted into an
Expand Down
26 changes: 10 additions & 16 deletions compiler/rustc_parse/src/parser/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::pat::{CommaRecoveryMode, Expected, RecoverColon, RecoverComma};
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
use super::{
AttrWrapper, BlockMode, ClosureSpans, ForceCollect, Parser, PathStyle, Restrictions,
SemiColonMode, SeqSep, TokenType, Trailing, TrailingToken,
SemiColonMode, SeqSep, TokenType, Trailing,
};

use crate::errors;
Expand Down Expand Up @@ -2474,7 +2474,7 @@ impl<'a> Parser<'a> {
id: DUMMY_NODE_ID,
is_placeholder: false,
},
TrailingToken::MaybeComma,
this.token == token::Comma,
))
})
}
Expand Down Expand Up @@ -3257,7 +3257,7 @@ impl<'a> Parser<'a> {
id: DUMMY_NODE_ID,
is_placeholder: false,
},
TrailingToken::None,
false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would expect a next_token == token::Comma here, because match arms are separated by commas.

I suspect this may cause issues with configured out arms inside derive/cfg_eval.

#[cfg_eval]
#[proc_macro_that_tries_to_parse_it_again] // will fail with a syntax error because the comma
                                           // is not removed from the token representation?
fn foo() {
  match 10 {
    #[cfg(FALSE)]
    10 => true,
     _ => false,
  }
}

))
})
}
Expand Down Expand Up @@ -3766,7 +3766,7 @@ impl<'a> Parser<'a> {
id: DUMMY_NODE_ID,
is_placeholder: false,
},
TrailingToken::MaybeComma,
this.token == token::Comma,
))
})
}
Expand Down Expand Up @@ -3862,18 +3862,12 @@ impl<'a> Parser<'a> {
) -> PResult<'a, P<Expr>> {
self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| {
let res = f(this, attrs)?;
let trailing = if this.restrictions.contains(Restrictions::STMT_EXPR)
&& this.token.kind == token::Semi
{
TrailingToken::Semi
} else if this.token.kind == token::Gt {
TrailingToken::Gt
} else {
// FIXME - pass this through from the place where we know
// we need a comma, rather than assuming that `#[attr] expr,`
// always captures a trailing comma
TrailingToken::MaybeComma
};
let trailing = (this.restrictions.contains(Restrictions::STMT_EXPR)
&& this.token.kind == token::Semi)
// FIXME: pass an additional condition through from the place
// where we know we need a comma, rather than assuming that
// `#[attr] expr,` always captures a trailing comma.
|| this.token.kind == token::Comma;
Ok((res, trailing))
})
}
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_parse/src/parser/generics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::errors::{
WhereClauseBeforeTupleStructBodySugg,
};

use super::{ForceCollect, Parser, TrailingToken};
use super::{ForceCollect, Parser};

use ast::token::Delimiter;
use rustc_ast::token;
Expand Down Expand Up @@ -229,13 +229,13 @@ impl<'a> Parser<'a> {
span: where_predicate.span(),
});
// FIXME - try to continue parsing other generics?
return Ok((None, TrailingToken::None));
return Ok((None, false));
}
Err(err) => {
err.cancel();
// FIXME - maybe we should overwrite 'self' outside of `collect_tokens`?
this.restore_snapshot(snapshot);
return Ok((None, TrailingToken::None));
return Ok((None, false));
}
}
} else {
Expand All @@ -249,14 +249,14 @@ impl<'a> Parser<'a> {
.emit_err(errors::AttrWithoutGenerics { span: attrs[0].span });
}
}
return Ok((None, TrailingToken::None));
return Ok((None, false));
};

if !this.eat(&token::Comma) {
done = true;
}
// We just ate the comma, so no need to use `TrailingToken`
Ok((param, TrailingToken::None))
// We just ate the comma, so no need to capture the trailing token.
Ok((param, false))
Copy link
Contributor

Choose a reason for hiding this comment

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

Including commas into the node's tokens in node1, node2, node3 doesn't feel right, but maybe it's ok if proc macro attributes are not supported on this kind of nodes, only cfgs.
That's assuming a proc macro should only see node's tokens without comma, and cfg(FALSE) should remove tokens including tokens.

On the other hand, proc macros probably should see the trailing comma as a part of their input, so they could correctly expand into nothing if necessary, like cfg(FALSE).
That would be a change in behavior, but it should only affect unstable proc macros (?).
That would eliminate the need in TrailingToken::MaybeComma entirely, because the commas would always be included and would never be trailing.

})?;

if let Some(param) = param {
Expand Down
25 changes: 11 additions & 14 deletions compiler/rustc_parse/src/parser/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use super::diagnostics::{dummy_arg, ConsumeClosingDelim};
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
use super::{
AttrWrapper, FollowedByType, ForceCollect, Parser, PathStyle, Trailing, TrailingToken,
};
use super::{AttrWrapper, FollowedByType, ForceCollect, Parser, PathStyle, Trailing};
use crate::errors::{self, MacroExpandsToAdtField};
use crate::fluent_generated as fluent;
use crate::maybe_whole;
Expand Down Expand Up @@ -146,7 +144,7 @@ impl<'a> Parser<'a> {
let span = lo.to(this.prev_token.span);
let id = DUMMY_NODE_ID;
let item = Item { ident, attrs, id, kind, vis, span, tokens: None };
return Ok((Some(item), TrailingToken::None));
return Ok((Some(item), false));
}

// At this point, we have failed to parse an item.
Expand All @@ -161,7 +159,7 @@ impl<'a> Parser<'a> {
if !attrs_allowed {
this.recover_attrs_no_item(&attrs)?;
}
Ok((None, TrailingToken::None))
Ok((None, false))
})
}

Expand Down Expand Up @@ -1555,7 +1553,7 @@ impl<'a> Parser<'a> {

let vis = this.parse_visibility(FollowedByType::No)?;
if !this.recover_nested_adt_item(kw::Enum)? {
return Ok((None, TrailingToken::None));
return Ok((None, false));
}
let ident = this.parse_field_ident("enum", vlo)?;

Expand All @@ -1567,7 +1565,7 @@ impl<'a> Parser<'a> {
this.bump();
this.parse_delim_args()?;

return Ok((None, TrailingToken::MaybeComma));
return Ok((None, this.token == token::Comma));
}

let struct_def = if this.check(&token::OpenDelim(Delimiter::Brace)) {
Expand Down Expand Up @@ -1624,7 +1622,7 @@ impl<'a> Parser<'a> {
is_placeholder: false,
};

Ok((Some(vr), TrailingToken::MaybeComma))
Ok((Some(vr), this.token == token::Comma))
},
)
.map_err(|mut err| {
Expand Down Expand Up @@ -1816,7 +1814,7 @@ impl<'a> Parser<'a> {
attrs,
is_placeholder: false,
},
TrailingToken::MaybeComma,
p.token == token::Comma,
))
})
})
Expand All @@ -1831,8 +1829,7 @@ impl<'a> Parser<'a> {
self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| {
let lo = this.token.span;
let vis = this.parse_visibility(FollowedByType::No)?;
this.parse_single_struct_field(adt_ty, lo, vis, attrs)
.map(|field| (field, TrailingToken::None))
this.parse_single_struct_field(adt_ty, lo, vis, attrs).map(|field| (field, false))
})
}

Expand Down Expand Up @@ -2735,7 +2732,7 @@ impl<'a> Parser<'a> {
if let Some(mut param) = this.parse_self_param()? {
param.attrs = attrs;
let res = if first_param { Ok(param) } else { this.recover_bad_self_param(param) };
return Ok((res?, TrailingToken::None));
return Ok((res?, false));
}

let is_name_required = match this.token.kind {
Expand All @@ -2751,7 +2748,7 @@ impl<'a> Parser<'a> {
this.parameter_without_type(&mut err, pat, is_name_required, first_param)
{
let guar = err.emit();
Ok((dummy_arg(ident, guar), TrailingToken::None))
Ok((dummy_arg(ident, guar), false))
} else {
Err(err)
};
Expand Down Expand Up @@ -2794,7 +2791,7 @@ impl<'a> Parser<'a> {

Ok((
Param { attrs, id: ast::DUMMY_NODE_ID, is_placeholder: false, pat, span, ty },
TrailingToken::None,
false,
))
})
}
Expand Down
12 changes: 1 addition & 11 deletions compiler/rustc_parse/src/parser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,6 @@ pub enum ForceCollect {
No,
}

#[derive(Debug, Eq, PartialEq)]
pub enum TrailingToken {
None,
Semi,
Gt,
/// If the trailing token is a comma, then capture it
/// Otherwise, ignore the trailing token
MaybeComma,
}

#[macro_export]
macro_rules! maybe_whole {
($p:expr, $constructor:ident, |$x:ident| $e:expr) => {
Expand Down Expand Up @@ -1508,7 +1498,7 @@ impl<'a> Parser<'a> {
self.collect_tokens_trailing_token(
AttrWrapper::empty(),
ForceCollect::Yes,
|this, _attrs| Ok((f(this)?, TrailingToken::None)),
|this, _attrs| Ok((f(this)?, false)),
)
}

Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_parse/src/parser/pat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{ForceCollect, Parser, PathStyle, Restrictions, Trailing, TrailingToken};
use super::{ForceCollect, Parser, PathStyle, Restrictions, Trailing};
use crate::errors::{
self, AmbiguousRangePattern, DotDotDotForRemainingFields, DotDotDotRangeToPatternNotAllowed,
DotDotDotRestPattern, EnumPatternInsteadOfIdentifier, ExpectedBindingLeftOfAt,
Expand Down Expand Up @@ -1315,9 +1315,8 @@ impl<'a> Parser<'a> {

last_non_comma_dotdot_span = Some(this.prev_token.span);

// We just ate a comma, so there's no need to use
// `TrailingToken::Comma`
Ok((field, TrailingToken::None))
// We just ate a comma, so there's no need to capture a trailing token.
Ok((field, false))
})?;

fields.push(field)
Expand Down
20 changes: 4 additions & 16 deletions compiler/rustc_parse/src/parser/stmt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use super::diagnostics::AttemptLocalParseRecovery;
use super::expr::LhsExpr;
use super::pat::{PatternLocation, RecoverComma};
use super::path::PathStyle;
use super::TrailingToken;
use super::{
AttrWrapper, BlockMode, FnParseMode, ForceCollect, Parser, Restrictions, SemiColonMode,
};
Expand Down Expand Up @@ -149,11 +148,7 @@ impl<'a> Parser<'a> {

if this.eat(&token::Not) {
let stmt_mac = this.parse_stmt_mac(lo, attrs, path)?;
if this.token == token::Semi {
return Ok((stmt_mac, TrailingToken::Semi));
} else {
return Ok((stmt_mac, TrailingToken::None));
}
return Ok((stmt_mac, this.token == token::Semi));
}

let expr = if this.eat(&token::OpenDelim(Delimiter::Brace)) {
Expand All @@ -167,7 +162,7 @@ impl<'a> Parser<'a> {
this.parse_expr_dot_or_call_with(attrs, expr, lo)
})?;
// `DUMMY_SP` will get overwritten later in this function
Ok((this.mk_stmt(rustc_span::DUMMY_SP, StmtKind::Expr(expr)), TrailingToken::None))
Ok((this.mk_stmt(rustc_span::DUMMY_SP, StmtKind::Expr(expr)), false))
})?;

if let StmtKind::Expr(expr) = stmt.kind {
Expand Down Expand Up @@ -241,10 +236,7 @@ impl<'a> Parser<'a> {
self.collect_tokens_trailing_token(attrs, ForceCollect::Yes, |this, attrs| {
let local = this.parse_local(attrs)?;
// FIXME - maybe capture semicolon in recovery?
Ok((
this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)),
TrailingToken::None,
))
Ok((this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), false))
})?;
self.dcx()
.emit_err(errors::InvalidVariableDeclaration { span: lo, sub: subdiagnostic(lo) });
Expand All @@ -261,11 +253,7 @@ impl<'a> Parser<'a> {
self.collect_tokens_trailing_token(attrs, force_collect, |this, attrs| {
this.expect_keyword(kw::Let)?;
let local = this.parse_local(attrs)?;
let trailing = if capture_semi && this.token.kind == token::Semi {
TrailingToken::Semi
} else {
TrailingToken::None
};
let trailing = capture_semi && this.token.kind == token::Semi;
Ok((this.mk_stmt(lo.to(this.prev_token.span), StmtKind::Let(local)), trailing))
})
}
Expand Down
Loading