Skip to content

Add support for trailing commas in more places #16646

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
Aug 24, 2014
Merged

Add support for trailing commas in more places #16646

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
43 changes: 25 additions & 18 deletions src/libsyntax/parse/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ use std::collections::HashSet;
use std::mem::replace;
use std::rc::Rc;
use std::gc::{Gc, GC};
use std::iter;

#[allow(non_camel_case_types)]
#[deriving(PartialEq)]
Expand Down Expand Up @@ -762,20 +763,26 @@ impl<'a> Parser<'a> {
sep: Option<token::Token>,
f: |&mut Parser| -> T)
-> OwnedSlice<T> {
let mut first = true;
let mut v = Vec::new();
while self.token != token::GT
&& self.token != token::BINOP(token::SHR)
&& self.token != token::GE
&& self.token != token::BINOPEQ(token::SHR) {
match sep {
Some(ref t) => {
if first { first = false; }
else { self.expect(t); }
}
_ => ()
// This loop works by alternating back and forth between parsing types
// and commas. For example, given a string `A, B,>`, the parser would
// first parse `A`, then a comma, then `B`, then a comma. After that it
// would encounter a `>` and stop. This lets the parser handle trailing
// commas in generic parameters, because it can stop either after
// parsing a type or after parsing a comma.
for i in iter::count(0u, 1) {
if self.token == token::GT
|| self.token == token::BINOP(token::SHR)
|| self.token == token::GE
|| self.token == token::BINOPEQ(token::SHR) {
break;
}

if i % 2 == 0 {
v.push(f(self));
} else {
sep.as_ref().map(|t| self.expect(t));
}
v.push(f(self));
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems like an interesting pattern which is a little difficult to follow, could this just attempt to eat a comma after an expression and if it fails the loop bails out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but then I’d need to duplicate the condition in the while loop that asserts that the following token is a >, >>, >=, or >>=, right? Because for something like let x: Foo<int = 1;, it’d see that the = isn’t a comma, so it’d bail out, but it'd need to check that the = is one of the aforementioned >-like tokens. That would mean I’d need to duplicate the condition, which I didn’t want to do.

I went with the other solution because it seemed a lot simpler in my head (‘just parse a type, then a comma, then a type, then a comma, and repeat until you find a >-like token or it’s not what you expect’). The code did turn out a bit cryptic, though.

Copy link
Member

Choose a reason for hiding this comment

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

Hm ok, perhaps this could use something like count with a check if the variable modulo two is one? Could you also add a comment explaining how you're just alternating back and forth between parsing the two?

return OwnedSlice::from_vec(v);
}
Expand Down Expand Up @@ -2266,7 +2273,7 @@ impl<'a> Parser<'a> {
let mut es = self.parse_unspanned_seq(
&token::LPAREN,
&token::RPAREN,
seq_sep_trailing_disallowed(token::COMMA),
seq_sep_trailing_allowed(token::COMMA),
|p| p.parse_expr()
);
hi = self.last_span.hi;
Expand Down Expand Up @@ -3196,7 +3203,7 @@ impl<'a> Parser<'a> {
args = self.parse_enum_variant_seq(
&token::LPAREN,
&token::RPAREN,
seq_sep_trailing_disallowed(token::COMMA),
seq_sep_trailing_allowed(token::COMMA),
|p| p.parse_pat()
);
pat = PatEnum(enum_path, Some(args));
Expand Down Expand Up @@ -4068,7 +4075,7 @@ impl<'a> Parser<'a> {
match self.token {
token::COMMA => {
self.bump();
let sep = seq_sep_trailing_disallowed(token::COMMA);
let sep = seq_sep_trailing_allowed(token::COMMA);
let mut fn_inputs = self.parse_seq_to_before_end(
&token::RPAREN,
sep,
Expand All @@ -4091,7 +4098,7 @@ impl<'a> Parser<'a> {

let fn_inputs = match explicit_self {
SelfStatic => {
let sep = seq_sep_trailing_disallowed(token::COMMA);
let sep = seq_sep_trailing_allowed(token::COMMA);
self.parse_seq_to_before_end(&token::RPAREN, sep, parse_arg_fn)
}
SelfValue(id) => parse_remaining_arguments!(id),
Expand Down Expand Up @@ -4128,7 +4135,7 @@ impl<'a> Parser<'a> {
self.parse_optional_unboxed_closure_kind();
let args = self.parse_seq_to_before_end(
&token::BINOP(token::OR),
seq_sep_trailing_disallowed(token::COMMA),
seq_sep_trailing_allowed(token::COMMA),
|p| p.parse_fn_block_arg()
);
self.bump();
Expand Down Expand Up @@ -4950,7 +4957,7 @@ impl<'a> Parser<'a> {
let arg_tys = self.parse_enum_variant_seq(
&token::LPAREN,
&token::RPAREN,
seq_sep_trailing_disallowed(token::COMMA),
seq_sep_trailing_allowed(token::COMMA),
|p| p.parse_ty(true)
);
for ty in arg_tys.move_iter() {
Expand Down
26 changes: 24 additions & 2 deletions src/test/run-pass/trailing-comma.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,31 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn f(_: int,) {}
fn f<T,>(_: T,) {}

struct Foo<T,>;

struct Bar;

impl Bar {
fn f(_: int,) {}
fn g(self, _: int,) {}
fn h(self,) {}
}

enum Baz {
Qux(int,),
}

pub fn main() {
f(0i,);
f::<int,>(0i,);
let (_, _,) = (1i, 1i,);

let x: Foo<int,> = Foo::<int,>;

Bar::f(0i,);
Bar.g(0i,);
Bar.h();

let x = Qux(1,);
}