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

compiler: Add zip generators for comprehensions #8926

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lucioleKi
Copy link
Contributor

We introduce zip generators for comprehensions to reduce the need for users to use lists:zip.

Zip generators have the syntax of generator1 && ... && generatorN, where each generator can be a list, binary, or map generator.

Zip generators are evaluated as if they are zipped to be a list of tuples. They can be mixed with all existing generators and filters freely.

Two examples to show how zip generators is used and evaluated:

1> [{X,Y} || X <- [a,b,c] && Y <- [1,2,3]].
[{a,1},{b,2},{c,3}]
2> << <<(X+Y)/integer>> || X <- [1,2,3] && <<Y>> <= <<1,1,1>>, X < 3 >>.
<<2,3>>

@lucioleKi lucioleKi added team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI labels Oct 10, 2024
@lucioleKi lucioleKi self-assigned this Oct 10, 2024
Copy link
Contributor

github-actions bot commented Oct 10, 2024

CT Test Results

     8 files     588 suites   2h 0m 23s ⏱️
 4 437 tests  4 316 ✅ 120 💤 1 ❌
10 588 runs  10 441 ✅ 146 💤 1 ❌

For more details on these failures, see this check.

Results for commit 9f0e080.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@lucioleKi lucioleKi force-pushed the isabell/compiler/zip-comprehension/OTP-19184 branch from 19911a2 to cac4be3 Compare October 10, 2024 14:37
Copy link
Contributor

@jhogberg jhogberg left a comment

Choose a reason for hiding this comment

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

Great work! :-)

lib/compiler/src/v3_core.erl Outdated Show resolved Hide resolved
lib/compiler/src/v3_core.erl Outdated Show resolved Hide resolved
lib/debugger/src/dbg_ieval.erl Outdated Show resolved Hide resolved
@michalmuskala
Copy link
Contributor

Does this need an EEP?

There's also a question in my mind with how this interacts with #8625?

In particular that EEP/feature argues that it's error prone to have values silently dropped from a comprehension - but this feature introduces even more cases where this happens if the zipped generators are not of equal length. It seems to me like those two features send opposing signals as to what kind of code and behaviour is expected and desirable.

@lucioleKi
Copy link
Contributor Author

lucioleKi commented Oct 11, 2024

Does this need an EEP?

Yes. This implementation is for EEP 73. It will go through the EEP process. The forum post for discussion is here.

There's also a question in my mind with how this interacts with #8625?

In particular that EEP/feature argues that it's error prone to have values silently dropped from a comprehension - but this feature introduces even more cases where this happens if the zipped generators are not of equal length. It seems to me like those two features send opposing signals as to what kind of code and behaviour is expected and desirable.

#8625 and zip generators are orthogonal features. For now, the implementation does not support strict generators, only because the syntax for strict generators isn't in master. I will add support for it after #8625 is merged. Implementation-wise it's not something too difficult to add.

Design-wise, zip generators should support all kinds of generators in all comprehensions, including strict generators. The skipping behavior of individual generators within a zip generator is always respected. When a strict generator is zipped together with a relaxed generator, like PatternA <:- List1 && PatternB <- List2, it will badarg if pattern matching for PatternA fails, simply skipping (1 element from List1 and 1 element from List2) if pattern matching for PatternB fails. We will always try to pattern match PatternA, so it will not be silently skipped.

Containing a mix of strict and relaxed generators isn't a reason that a zip generator having generators of different lengths. When evaluating a zip generator, 3 things can happen in every round: one element taken from every generator without skipping, one element taken from every generator and are all skipped, or badarg because of a strict generator. It's impossible for generators zipped together to start with the same length and end with different lengths.

@RaimoNiskanen
Copy link
Contributor

@michalmuskala: I wouldn't say EEP 70 "argues that it's error prone to have values silently dropped from a comprehension". It merely says that silently dropping values "can hide the presence of unexpected elements". Whether non-matching elements are expected or not is up to the application.

Both behaviours are desirable in different scenarios.

But I agree that it should be stated, in EEP 73, how zip generators would interact with strict generators.

Copy link
Contributor

@bjorng bjorng left a comment

Choose a reason for hiding this comment

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

Great work! I am looking forward to using zip comprehensions in the code I write.

@bjorng
Copy link
Contributor

bjorng commented Oct 12, 2024

Both behaviours are desirable in different scenarios.

Here are some concrete examples where the relaxed behavior is useful.

In the compiler, the SSA instructions have an argument list where each element in the list is either a #b_var{} or #b_literal{} record. Quite often, we need to extract only #b_var{} records. We do it using some variation of this list comprehension:

    [Var || #b_var{}=Var <- Args]

That said, we obviously also have many list comprehensions in the compiler where the strict behavior is appropriate.

Here is an example from asn1 where we want to collect only mandatory components:

    M = [[{asis,Name},":=",Var] ||
            #'ComponentType'{prop=mandatory,name=Name} <- CompList && Var <- Vars],

(The current version of the code uses lists:zip/2 instead of the zip generator.)


For more examples showing how zip comprehensions are useful, in this branch I've replaced the use of lists:zip/2 with zip comprehensions:

https://github.com/bjorng/otp/tree/bjorn/use-zip-comprehensions

@lucioleKi lucioleKi force-pushed the isabell/compiler/zip-comprehension/OTP-19184 branch from 8da3330 to 2dc063c Compare October 14, 2024 09:21
@lucioleKi lucioleKi force-pushed the isabell/compiler/zip-comprehension/OTP-19184 branch from 2dc063c to 7b62036 Compare October 14, 2024 09:39
We introduce zip generators for comprehensions to reduce the need for
users to use lists:zip.

Zip generators have the syntax of generator1 && ... && generatorN,
where each generator can be a list, binary, or map generator.

Zip generators are evaluated as if they are zipped to be a list of
tuples. They can be mixed with all existing generators and filters
freely.

Two examples to show how zip generators is used and evaluated:

    1> [{X,Y} || X <- [a,b,c] && Y <- [1,2,3]].
    [{a,1},{b,2},{c,3}]
    2> << <<(X+Y)/integer>> || X <- [1,2,3] && <<Y>> <= <<1,1,1>>, X < 3 >>.
    <<2,3>>
@lucioleKi lucioleKi force-pushed the isabell/compiler/zip-comprehension/OTP-19184 branch from 7b62036 to 9f0e080 Compare October 15, 2024 09:28
@attah
Copy link
Contributor

attah commented Oct 15, 2024

I know crossposting is bad; but i feel there are still more eyeballs here than in the forum.
Q: why && and not ;?
https://erlangforums.com/t/eep-73-zip-generators/4072

@lucioleKi
Copy link
Contributor Author

I know crossposting is bad; but i feel there are still more eyeballs here than in the forum. Q: why && and not ;? https://erlangforums.com/t/eep-73-zip-generators/4072

Pasted from the forum:

I vaguely remembered that within OTP this year, ; was brought up but we decided to stay with &&, which was proposed by EEP 19. I forgot the reason both for and against ; unfortunately. If someone else in OTP remembers, feel free to fill us in.

Readability-wise, I prefer && over ;. && looks more different to , than ; does. People who read/write code can't mix them up easily. [{X,Y,Z} || X <- L1; Y <- L2, Z <- L3] takes more squinting than [{X,Y,Z} || X <- L1 && Y <- L2, Z <- L3] to realize which generators are zipped, which ones are not.

I could see how ; is the "opposite" of ,. On the other hand, && conveys the meaning of "and" in most languages, which is close to the idea that generators are zipped or combined together in a way. And since || is already used in comprehensions, and it never means "or" in Erlang, I think it's reasonable to use && in comprehensions too.

@lucioleKi lucioleKi removed the testing currently being tested, tag is used by OTP internal CI label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:VM Assigned to OTP team VM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants