Skip to content
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.

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

Add support for trailing commas in more places #16646

Merged
merged 1 commit into from
Aug 24, 2014

Conversation

ftxqxd
Copy link
Contributor

@ftxqxd ftxqxd commented Aug 21, 2014

This lets the parser understand trailing commas in method calls, method definitions, enum variants, and type parameters.

Closes #14240.
Closes #15887.

}
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?

This lets the parser understand trailing commas in method calls, method
definitions, enum variants, and type parameters.

Closes rust-lang#14240.
Closes rust-lang#15887.
@ftxqxd
Copy link
Contributor Author

ftxqxd commented Aug 24, 2014

Comments addressed; tests pass locally. r?

bors added a commit that referenced this pull request Aug 24, 2014
This lets the parser understand trailing commas in method calls, method definitions, enum variants, and type parameters.

Closes #14240.
Closes #15887.
@bors bors closed this Aug 24, 2014
@bors bors merged commit fde41a3 into rust-lang:master Aug 24, 2014
@ftxqxd ftxqxd deleted the trailing-commas branch August 25, 2014 03:45
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trailing comma in method definition doesn't parse Trailing comma in method call doesn’t parse
3 participants