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

Fix "not match" and "not if" causing a syntax error #3449

Merged
merged 7 commits into from
Jan 28, 2020

Conversation

adri326
Copy link
Member

@adri326 adri326 commented Jan 15, 2020

This pull request fixes issue #2988, where not if, not for and not while resulted in a parse error, even though these should technically be valid.

Tests have been included to prevent this issue from happening again in the future.

This fixes issue ponylang#2988, by allowing `if` and `while` to be used as
expressions following the `not` operator (bzw. any other unary operator).
`not for` had not been mentionned in ponylang#2988. Also updates the comment
preceding `DEF(atom)`.
This reverts commit 8121944.
@SeanTAllen SeanTAllen changed the title Patch issue #2988 Fix "not match" and "not if" causing a syntax error Jan 15, 2020
@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Jan 15, 2020
@adri326
Copy link
Member Author

adri326 commented Jan 16, 2020

For the record: adding cond to the atom rule caused a collision between match's guard expressions and the regular condition expression (if this then that end), as both caseexpr and prefix rules end up using the atom rule.

The only viable fix I could come up with is to separate the rule tree that caseexpr and prefix use, such that caseexpr's tree does not contain the cond rule.

5 new rules had to be created, each being a slightly modified copy of their respective, already existing rule in prefix's rule tree:

  • casepattern
  • caseparampattern
  • caseprefix
  • casepostfix
  • caseatom (which does not contain cond)

@SeanTAllen SeanTAllen requested a review from jemc January 21, 2020 19:13
@SeanTAllen
Copy link
Member

@jemc can you give this a review?

@jemc jemc merged commit 4081905 into ponylang:master Jan 28, 2020
github-actions bot pushed a commit that referenced this pull request Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants