Skip to content

Optional list syntax separators #3

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

Draft
hanbings-agent wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Optional list syntax separators #3

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
190 changes: 134 additions & 56 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 @@ -3250,7 +3250,7 @@ impl<'a> Parser<'a> {
let (pat, guard) = this.parse_match_arm_pat_and_guard()?;

let span_before_body = this.prev_token.span;
let arm_body;
let mut arm_body;
let is_fat_arrow = this.check(exp!(FatArrow));
let is_almost_fat_arrow =
TokenKind::FatArrow.similar_tokens().contains(&this.token.kind);
Expand All @@ -3264,7 +3264,18 @@ impl<'a> Parser<'a> {
// A pattern without a body, allowed for never patterns.
arm_body = None;
let span = lo.to(this.prev_token.span);
this.expect_one_of(&[exp!(Comma)], &[exp!(CloseBrace)]).map(|x| {
// Allow omitting the comma if the next arm starts on a new line.
let sep_res = if this.eat(exp!(Comma))
|| this.check(exp!(CloseBrace))
|| (this.token_is_implicitly_separated_since_prev()
&& this.is_at_start_of_match_arm())
{
Ok(Recovered::No)
} else {
this.expect_one_of(&[exp!(Comma)], &[exp!(CloseBrace)])
};

sep_res.map(|x| {
// Don't gate twice
if !pat.contains_never_pattern() {
this.psess.gated_spans.gate(sym::never_patterns, span);
Expand Down Expand Up @@ -3299,30 +3310,31 @@ impl<'a> Parser<'a> {
let arrow_span = this.prev_token.span;
let arm_start_span = this.token.span;

let attrs = this.parse_outer_attributes()?;
let (expr, _) =
this.parse_expr_res(Restrictions::STMT_EXPR, attrs).map_err(|mut err| {
let expr = this
.parse_match_arm_body_expr_with_implicit_newline_sep()
.map_err(|mut err| {
err.span_label(arrow_span, "while parsing the `match` arm starting here");
err
})?;

let require_comma =
!classify::expr_is_complete(&expr) && this.token != token::CloseBrace;
let expr_span = expr.span;
arm_body = Some(expr);

if !require_comma {
arm_body = Some(expr);
// Eat a comma if it exists, though.
let _ = this.eat(exp!(Comma));
// Accept an explicit comma, the end of the match, or an implicit newline separator.
if this.eat(exp!(Comma)) || this.token == token::CloseBrace {
Ok(Recovered::No)
} else if let Some((span, guar)) =
this.parse_arm_body_missing_braces(&expr, arrow_span)
this.parse_arm_body_missing_braces(arm_body.as_ref().unwrap(), arrow_span)
{
let body = this.mk_expr_err(span, guar);
arm_body = Some(body);
Ok(Recovered::Yes(guar))
} else if this.token_is_implicitly_separated_since_prev()
&& this.is_at_start_of_match_arm()
{
Ok(Recovered::No)
} else {
let expr_span = expr.span;
arm_body = Some(expr);
// Same-line arms must still use an explicit comma.
this.expect_one_of(&[exp!(Comma)], &[exp!(CloseBrace)]).map_err(|mut err| {
if this.token == token::FatArrow {
let sm = this.psess.source_map();
Expand All @@ -3331,17 +3343,6 @@ impl<'a> Parser<'a> {
&& expr_lines.lines.len() == 2
{
if arm_start_lines.lines[0].end_col == expr_lines.lines[0].end_col {
// We check whether there's any trailing code in the parse span,
// if there isn't, we very likely have the following:
//
// X | &Y => "y"
// | -- - missing comma
// | |
// | arrow_span
// X | &X => "x"
// | - ^^ self.token.span
// | |
// | parsed until here as `"y" & X`
err.span_suggestion_short(
arm_start_span.shrink_to_hi(),
"missing a comma here to end this `match` arm",
Expand All @@ -3351,7 +3352,6 @@ impl<'a> Parser<'a> {
} else if arm_start_lines.lines[0].end_col + rustc_span::CharPos(1)
== expr_lines.lines[0].end_col
{
// similar to the above, but we may typo a `.` or `/` at the end of the line
let comma_span = arm_start_span
.shrink_to_hi()
.with_hi(arm_start_span.hi() + rustc_span::BytePos(1));
Expand Down Expand Up @@ -3439,6 +3439,76 @@ impl<'a> Parser<'a> {
})
}

/// Returns `true` if the current position looks like the start of a `match` arm
/// (`ATTRS? PAT [if GUARD] => ...`).
fn is_at_start_of_match_arm(&mut self) -> bool {
let mut snapshot = self.create_snapshot_for_diagnostic();
// Match arms may begin with outer attributes.
let _ = snapshot.parse_outer_attributes().map_err(|err| err.cancel());
snapshot
.parse_match_arm_pat_and_guard()
.map_err(|err| err.cancel())
.is_ok()
&& snapshot.check(exp!(FatArrow))
}

/// Parses a match arm body expression while allowing a newline to act as an implicit arm
/// separator (i.e. `,` may be omitted if the next arm starts on a new line).
///
/// This prevents the arm body expression parser from "swallowing" the next arm when the comma
/// is omitted.
fn parse_match_arm_body_expr_with_implicit_newline_sep(&mut self) -> PResult<'a, P<Expr>> {
let start = self.create_snapshot_for_diagnostic();

// Scan ahead and collect the token trees that belong to this arm body. Stop before `,`,
// `;`, `}` (end of match), or the start of the next arm on a new line.
let mut scan = self.create_snapshot_for_diagnostic();
let mut trees: Vec<rustc_ast::tokenstream::TokenTree> = Vec::new();
loop {
if matches!(scan.token.kind, token::Semi | token::CloseBrace) {
break;
}
if scan.token.kind.is_close_delim_or_eof() {
break;
}
if scan.token == token::Comma {
// Only treat `,` as an arm separator if it is followed by `}` or the start of
// another arm. This avoids misinterpreting commas that appear within the arm
// body expression (e.g. turbofish `::<T, U>`).
let mut look = scan.create_snapshot_for_diagnostic();
look.bump(); // consume `,`
if look.token == token::CloseBrace || look.is_at_start_of_match_arm() {
break;
}
} else if scan.token_is_on_new_line_since_prev() && scan.is_at_start_of_match_arm() {
break;
}
trees.push(scan.parse_token_tree());
}

let stream = rustc_ast::tokenstream::TokenStream::new(trees.clone());
let mut sub = Parser::new(self.psess, stream, Some("match_arm_body"));
let attrs = sub.parse_outer_attributes()?;
let (expr, _) = sub.parse_expr_res(Restrictions::STMT_EXPR, attrs)?;

// If the scan boundary was wrong (e.g. the token stream doesn't parse as a single expr),
// fall back to the normal parser to produce the usual diagnostic.
if sub.token != token::Eof {
self.restore_snapshot(start);
let attrs = self.parse_outer_attributes()?;
return self.parse_expr_res(Restrictions::STMT_EXPR, attrs).map(|res| res.0);
}

// Consume the same token trees in the main parser so the outer parser continues from the
// correct boundary token.
self.restore_snapshot(start);
for _ in 0..trees.len() {
let _ = self.parse_token_tree();
}

Ok(expr)
}

fn parse_match_arm_guard(&mut self) -> PResult<'a, Option<P<Expr>>> {
// Used to check the `if_let_guard` feature mostly by scanning
// `&&` tokens.
Expand Down Expand Up @@ -3776,39 +3846,47 @@ impl<'a> Parser<'a> {
// We should point this out.
self.check_or_expected(!is_shorthand, TokenType::Colon);

match self.expect_one_of(&[exp!(Comma)], &[close]) {
Ok(_) => {
if let Ok(f) = parsed_field.or_else(|guar| field_ident(self, guar).ok_or(guar))
{
// Only include the field if there's no parse error for the field name.
fields.push(f);
}
let implicit_sep = self.token_is_implicitly_separated_since_prev()
&& (self.token.is_non_reserved_ident()
|| self.token == TokenKind::Pound
|| self.token == token::DotDot
|| matches!(self.token.kind, token::DocComment(..)));

if self.eat(exp!(Comma)) || self.check(close) || implicit_sep {
if let Ok(f) = parsed_field.or_else(|guar| field_ident(self, guar).ok_or(guar)) {
// Only include the field if there's no parse error for the field name.
fields.push(f);
}
Err(mut e) => {
if pth == kw::Async {
async_block_err(&mut e, pth.span);
} else {
e.span_label(pth.span, "while parsing this struct");
if peek.is_some() {
e.span_suggestion(
self.prev_token.span.shrink_to_hi(),
"try adding a comma",
",",
Applicability::MachineApplicable,
);
} else {
// Same-line fields must still use an explicit comma.
match self.expect_one_of(&[exp!(Comma)], &[close]) {
Ok(_) => unreachable!("handled above"),
Err(mut e) => {
if pth == kw::Async {
async_block_err(&mut e, pth.span);
} else {
e.span_label(pth.span, "while parsing this struct");
if peek.is_some() {
e.span_suggestion(
self.prev_token.span.shrink_to_hi(),
"try adding a comma",
",",
Applicability::MachineApplicable,
);
}
}
if !recover {
return Err(e);
}
let guar = e.emit();
if pth == kw::Async {
recovered_async = Some(guar);
} else if let Some(f) = field_ident(self, guar) {
fields.push(f);
}
self.recover_stmt_(SemiColonMode::Comma, BlockMode::Ignore);
let _ = self.eat(exp!(Comma));
}
if !recover {
return Err(e);
}
let guar = e.emit();
if pth == kw::Async {
recovered_async = Some(guar);
} else if let Some(f) = field_ident(self, guar) {
fields.push(f);
}
self.recover_stmt_(SemiColonMode::Comma, BlockMode::Ignore);
let _ = self.eat(exp!(Comma));
}
}
}
Expand Down
19 changes: 16 additions & 3 deletions compiler/rustc_parse/src/parser/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1564,9 +1564,12 @@ impl<'a> Parser<'a> {
self.bump();
(thin_vec![], Trailing::No)
} else {
self.parse_delim_comma_seq(exp!(OpenBrace), exp!(CloseBrace), |p| {
p.parse_enum_variant(ident.span)
})
self.parse_unspanned_seq(
exp!(OpenBrace),
exp!(CloseBrace),
super::SeqSep::trailing_allowed_with_implicit_newline(exp!(Comma)),
|p| p.parse_enum_variant(ident.span),
)
.map_err(|mut err| {
err.span_label(ident.span, "while parsing this enum");
// Try to recover `enum Foo { ident : Ty }`.
Expand Down Expand Up @@ -1940,6 +1943,16 @@ impl<'a> Parser<'a> {
token::Comma => {
self.bump();
}
// Allow an implicit separator between fields when the next field starts on a new line.
//
// This intentionally does *not* allow omitting commas between fields written on the
// same line.
token::DocComment(..) if self.token_is_implicitly_separated_since_prev() => {}
_ if self.token_is_implicitly_separated_since_prev()
&& (self.token == TokenKind::Pound
|| self.token.is_keyword(kw::Pub)
|| self.token.is_keyword(kw::Unsafe)
|| self.token.is_ident()) => {}
token::Semi => {
self.bump();
let sp = self.prev_token.span;
Expand Down
44 changes: 42 additions & 2 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 @@ -267,15 +267,24 @@ struct SeqSep<'a> {
sep: Option<ExpTokenPair<'a>>,
/// `true` if a trailing separator is allowed.
trailing_sep_allowed: bool,
/// `true` if a separator may be omitted when the next element starts on a new line.
///
/// This is only intended for a small set of "list-like" syntaxes where a newline can act as
/// an implicit separator (e.g. certain brace-delimited lists).
implicit_sep_on_newline: bool,
}

impl<'a> SeqSep<'a> {
fn trailing_allowed(sep: ExpTokenPair<'a>) -> SeqSep<'a> {
SeqSep { sep: Some(sep), trailing_sep_allowed: true }
SeqSep { sep: Some(sep), trailing_sep_allowed: true, implicit_sep_on_newline: false }
}

fn trailing_allowed_with_implicit_newline(sep: ExpTokenPair<'a>) -> SeqSep<'a> {
SeqSep { sep: Some(sep), trailing_sep_allowed: true, implicit_sep_on_newline: true }
}

fn none() -> SeqSep<'a> {
SeqSep { sep: None, trailing_sep_allowed: false }
SeqSep { sep: None, trailing_sep_allowed: false, implicit_sep_on_newline: false }
}
}

Expand Down Expand Up @@ -869,6 +878,16 @@ impl<'a> Parser<'a> {
break;
}
Err(mut expect_err) => {
// Some lists allow a newline to act as an implicit separator.
// In those cases, accept a missing separator without emitting an error,
// but only if the next element starts on a new line.
if sep.implicit_sep_on_newline
&& self.token_is_implicitly_separated_since_prev()
{
self.current_closure.take();
expect_err.cancel();
// Proceed to parse the next element.
} else {
let sp = self.prev_token.span.shrink_to_hi();
let token_str = pprust::token_kind_to_string(exp.tok);

Expand Down Expand Up @@ -957,6 +976,7 @@ impl<'a> Parser<'a> {
}
}
}
}
}
}
}
Expand All @@ -975,6 +995,26 @@ impl<'a> Parser<'a> {
Ok((v, trailing, recovered))
}

/// Returns `true` if the current token starts on a new line compared to the previous token.
fn token_is_on_new_line_since_prev(&self) -> bool {
let sm = self.psess.source_map();
match (sm.lookup_line(self.prev_token.span.hi()), sm.lookup_line(self.token.span.lo())) {
(Ok(l), Ok(r)) => l.line < r.line,
_ => false,
}
}

/// Like [`token_is_on_new_line_since_prev`], but also treats macro-expanded tokens as
/// "implicitly separated".
///
/// This is used for a small set of list-like syntaxes where we allow omitting separators in
/// macro expansions without relying on line information.
fn token_is_implicitly_separated_since_prev(&self) -> bool {
self.token_is_on_new_line_since_prev()
|| self.prev_token.span.from_expansion()
|| self.token.span.from_expansion()
}

fn recover_missing_braces_around_closure_body(
&mut self,
closure_spans: ClosureSpans,
Expand Down