Skip to content

Grammar for expressions #264

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

Merged
merged 2 commits into from
Mar 11, 2018
Merged

Conversation

brauliobz
Copy link
Contributor

Please notice that the link to the GroupedExpression is technically broken, but the PR #261 corrects it.

Also, there must be a ExpressionNoStructLiteral and a ExpressionStmtExpr that other productions use, but I'm still trying to find a good way to show this without cluttering the grammar.

Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

Looks good.

Also, there must be a ExpressionNoStructLiteral and a ExpressionStmtExpr that other productions use, but I'm still trying to find a good way to show this without cluttering the grammar.

Raised as #266

Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

I cross-checked the list with the table of contents. This was harder than it seemed because this list is out of sync with the table of contents. IMO, they should be in sync. So are you proposing this is a better list than the table of contents currently is? Or do you have a different reason for this specific ordering?

@brauliobz
Copy link
Contributor Author

I don't quite remember why I chose this order or if there was any thinking involved. I will change them to match the TOC.

@Havvy Havvy merged commit b4c41c7 into rust-lang:master Mar 11, 2018
@Havvy
Copy link
Contributor

Havvy commented Mar 11, 2018

💟 😍 Thanks!

@brauliobz brauliobz deleted the grammar_expr_toplevel branch March 11, 2018 19:46
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.

3 participants