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 list comprehensions to parser #119

Merged
merged 5 commits into from
Jan 31, 2023

Conversation

tohrnii
Copy link
Contributor

@tohrnii tohrnii commented Jan 24, 2023

Partly addressing #87. This PR adds list comprehension support to the parser. This PR should be merged after #120.

@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch 7 times, most recently from 91e2212 to ec96dbe Compare January 25, 2023 12:28
@tohrnii tohrnii changed the base branch from next to tohrnii-refactor-exp January 25, 2023 12:29
@tohrnii tohrnii force-pushed the tohrnii-refactor-exp branch from 352437d to ddafabd Compare January 25, 2023 12:39
@tohrnii tohrnii changed the base branch from tohrnii-refactor-exp to next January 25, 2023 12:43
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch 2 times, most recently from a2cc0be to 6f18076 Compare January 25, 2023 22:55
@tohrnii tohrnii marked this pull request as ready for review January 25, 2023 22:56
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch 4 times, most recently from 15a4c3c to 7593ee9 Compare January 26, 2023 12:05
@tohrnii tohrnii marked this pull request as draft January 26, 2023 17:08
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch from 0d941e5 to ed1abb3 Compare January 26, 2023 17:50
@tohrnii tohrnii changed the base branch from next to tohrnii-refactor-exp January 26, 2023 17:51
@tohrnii tohrnii force-pushed the tohrnii-refactor-exp branch from a2e5990 to 1d00295 Compare January 26, 2023 17:53
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch 2 times, most recently from 44ec2ee to 2445536 Compare January 26, 2023 17:57
@grjte grjte requested a review from bobbinth January 27, 2023 09:13
@tohrnii tohrnii force-pushed the tohrnii-refactor-exp branch from 1d00295 to 9bfac4c Compare January 27, 2023 09:42
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch 2 times, most recently from 98f7e0a to 49bf627 Compare January 27, 2023 09:49
@tohrnii tohrnii force-pushed the tohrnii-refactor-exp branch from 9bfac4c to 0a46017 Compare January 27, 2023 10:18
Base automatically changed from tohrnii-refactor-exp to next January 27, 2023 12:54
@tohrnii
Copy link
Contributor Author

tohrnii commented Jan 27, 2023

Based on an example by @bobbinth (in rescue prime constraints), we would need to be able to do something like:

  enf x = y for (x, y) in (s1, s2)

However, the PR in the current form doesn't support these kinds of expressions. I'll update this PR to allow such expressions.

Edit: This would be handled in another PR.

@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch from dc8fa06 to 8db9bf4 Compare January 30, 2023 09:09
Copy link
Contributor

@bobbinth bobbinth 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 to me! Thank you! I did leave one comment inline: basically, not sure if the current implementation is a simplification of where we want to be. Fine if so, but if not - I think we need to have a more general structure which could handle many iterable lists.

air-script-core/src/variable.rs Outdated Show resolved Hide resolved
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch 6 times, most recently from 5fd2d8d to d358bec Compare January 31, 2023 01:21
@tohrnii tohrnii requested review from bobbinth and grjte January 31, 2023 01:21
Copy link
Contributor

@bobbinth bobbinth 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! Thank you! I left one small non-blocking comment inline.

air-script-core/src/variable.rs Outdated Show resolved Hide resolved
@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch from d358bec to 5d30017 Compare January 31, 2023 06:57
Copy link
Contributor

@Fumuran Fumuran left a comment

Choose a reason for hiding this comment

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

Looks great to me, thank you @tohrnii!

@tohrnii tohrnii force-pushed the tohrnii-list-comprehension branch from 5d30017 to 580e060 Compare January 31, 2023 14:27
Copy link
Contributor

@grjte grjte left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @tohrnii!

@tohrnii tohrnii merged commit 42e2648 into next Jan 31, 2023
@tohrnii tohrnii deleted the tohrnii-list-comprehension branch January 31, 2023 14:36
@tohrnii tohrnii mentioned this pull request Mar 7, 2023
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.

5 participants