Skip to content

Commit

Permalink
[WIP] Implement non-skipping generators as an experimental feature.
Browse files Browse the repository at this point in the history
Currently existing generators are "skipping": they ignore terms in
the right-hand side expression that do not match the left-hand side
pattern. Non-skipping generators on the other hand fail with
exception badmatch.

The motivation for non-skipping generators is that skipping generators
can hide the presence of unexpected elements in the input data of a
comprehension. For example consider the below snippet:

[{User, Email} || #{user := User, email := Email} <- all_users()]

This list comprehension would skip users that don't have an email
address. This may be an issue if we suspect potentionally incorrect
input data, like in case all_users/0 would read the users from a
JSON file. Therefore caucious code that would prefer crashing
instead of silently skipping incorrect input would have to use
a more verbose map function:

lists:map(fun(#{user := User, email := Email}) -> {User, Email} end,
          all_users())

Unlike the generator, the anonymous function would crash on a user
without an email address. Non-skipping generators would allow
similar semantics in comprehensions too:

[{User, Email} || #{user := User, email := Email} <:- all_users()]

This generator would crash (with a badmatch error) if the pattern
wouldn't match an element of the list.

Syntactically non-skipping generators use <:- (for lists and maps)
and <:= (for binaries) instead of <- and <=. This syntax was chosen
because `<:-` and `<:=` somewhat resemble the `=:=` operator that
tests whether two terms match, and at the same time keep the operators
short and easy to type. Having the two types of operators differ by
a single character, `:`, also makes the operators easy to remember as
"`:` means non-skipping."

Nevertheless, non-skipping generators are added as an experimental
feature, that has to be explicitly enabled:

-feature(non_skipping_generators,enable).

change operators
  • Loading branch information
dszoboszlay committed Aug 24, 2024
1 parent cf3d187 commit ad90a17
Show file tree
Hide file tree
Showing 34 changed files with 1,066 additions and 286 deletions.
7 changes: 7 additions & 0 deletions erts/doc/guides/absform.md
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,18 @@ A qualifier Q is one of the following:
- If Q is a filter `E`, where `E` is an expression, then Rep(Q) = `Rep(E)`.
- If Q is a list generator `P <- E`, where `P` is a pattern and `E` is an
expression, then Rep(Q) = `{generate,ANNO,Rep(P),Rep(E)}`.
- If Q is a list generator `P <:- E`, where `P` is a pattern and `E` is an
expression, then Rep(Q) = `{generate_ns,ANNO,Rep(P),Rep(E)}`.
- If Q is a bitstring generator `P <= E`, where `P` is a pattern and `E` is an
expression, then Rep(Q) = `{b_generate,ANNO,Rep(P),Rep(E)}`.
- If Q is a bitstring generator `P <:= E`, where `P` is a pattern and `E` is an
expression, then Rep(Q) = `{b_generate_ns,ANNO,Rep(P),Rep(E)}`.
- If Q is a map generator `P <- E`, where `P` is an association pattern
`P_1 := P_2` and `E` is an expression, then Rep(Q) =
`{m_generate,ANNO,Rep(P),Rep(E)}`. For Rep(P), see below.
- If Q is a map generator `P <:- E`, where `P` is an association pattern
`P_1 := P_2` and `E` is an expression, then Rep(Q) =
`{m_generate_ns,ANNO,Rep(P),Rep(E)}`.

### Bitstring Element Type Specifiers

Expand Down
12 changes: 12 additions & 0 deletions lib/compiler/src/sys_coverage.erl
Original file line number Diff line number Diff line change
Expand Up @@ -553,14 +553,26 @@ munge_qs([{generate,Anno,Pattern,Expr}|Qs], Vars0, MQs) ->
A = element(2, Expr),
{MungedExpr, Vars1} = munge_expr(Expr, Vars0),
munge_qs1(Qs, A, {generate,Anno,Pattern,MungedExpr}, Vars0, Vars1, MQs);
munge_qs([{generate_ns,Anno,Pattern,Expr}|Qs], Vars0, MQs) ->
A = element(2, Expr),
{MungedExpr, Vars1} = munge_expr(Expr, Vars0),
munge_qs1(Qs, A, {generate_ns,Anno,Pattern,MungedExpr}, Vars0, Vars1, MQs);
munge_qs([{b_generate,Anno,Pattern,Expr}|Qs], Vars0, MQs) ->
A = element(2, Expr),
{MExpr, Vars1} = munge_expr(Expr, Vars0),
munge_qs1(Qs, A, {b_generate,Anno,Pattern,MExpr}, Vars0, Vars1, MQs);
munge_qs([{b_generate_ns,Anno,Pattern,Expr}|Qs], Vars0, MQs) ->
A = element(2, Expr),
{MExpr, Vars1} = munge_expr(Expr, Vars0),
munge_qs1(Qs, A, {b_generate_ns,Anno,Pattern,MExpr}, Vars0, Vars1, MQs);
munge_qs([{m_generate,Anno,Pattern,Expr}|Qs], Vars0, MQs) ->
A = element(2, Expr),
{MExpr, Vars1} = munge_expr(Expr, Vars0),
munge_qs1(Qs, A, {m_generate,Anno,Pattern,MExpr}, Vars0, Vars1, MQs);
munge_qs([{m_generate_ns,Anno,Pattern,Expr}|Qs], Vars0, MQs) ->
A = element(2, Expr),
{MExpr, Vars1} = munge_expr(Expr, Vars0),
munge_qs1(Qs, A, {m_generate_ns,Anno,Pattern,MExpr}, Vars0, Vars1, MQs);
munge_qs([Expr|Qs], Vars0, MQs) ->
A = element(2, Expr),
{MungedExpr, Vars1} = munge_expr(Expr, Vars0),
Expand Down
241 changes: 188 additions & 53 deletions lib/compiler/src/v3_core.erl

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions lib/compiler/test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,9 @@ RELSYSDIR = $(RELEASE_PATH)/compiler_test
# FLAGS
# ----------------------------------------------------

NON_SKIPPING_GENERATORS_OPT = '+{enable_feature,non_skipping_generators}'
ERL_MAKE_FLAGS +=
ERL_COMPILE_FLAGS += +clint +clint0 +ssalint +nowarn_missing_spec_documented
ERL_COMPILE_FLAGS += +clint +clint0 +ssalint +nowarn_missing_spec_documented ${NON_SKIPPING_GENERATORS_OPT}
ERL_COMPILE_FLAGS := $(filter-out +deterministic,$(ERL_COMPILE_FLAGS))

EBIN = .
Expand Down Expand Up @@ -265,7 +266,7 @@ docs:

# ----------------------------------------------------
# Release Target
# ----------------------------------------------------
# ----------------------------------------------------
include $(ERL_TOP)/make/otp_release_targets.mk

release_spec: opt
Expand Down
38 changes: 30 additions & 8 deletions lib/compiler/test/bs_bincomp_SUITE.erl
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
%%
%% %CopyrightBegin%
%%
%%
%% Copyright Ericsson AB 2006-2024. All Rights Reserved.
%%
%%
%% Licensed under the Apache License, Version 2.0 (the "License");
%% you may not use this file except in compliance with the License.
%% You may obtain a copy of the License at
Expand All @@ -14,22 +14,22 @@
%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
%% See the License for the specific language governing permissions and
%% limitations under the License.
%%
%%
%% %CopyrightEnd%
%%
%% Originally based on Per Gustafsson's test suite.
%%

-module(bs_bincomp_SUITE).

-export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1,
-export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1,
init_per_group/2,end_per_group/2,
verify_highest_opcode/1,
byte_aligned/1,bit_aligned/1,extended_byte_aligned/1,
extended_bit_aligned/1,mixed/1,filters/1,trim_coverage/1,
nomatch/1,sizes/1,general_expressions/1,
no_generator/1,zero_pattern/1,multiple_segments/1,
grab_bag/1]).
grab_bag/1, non_skipping_generators/1]).

-include_lib("common_test/include/ct.hrl").

Expand All @@ -41,7 +41,7 @@ all() ->
extended_bit_aligned, mixed, filters, trim_coverage,
nomatch, sizes, general_expressions,
no_generator, zero_pattern, multiple_segments,
grab_bag].
grab_bag, non_skipping_generators].

groups() ->
[].
Expand Down Expand Up @@ -322,7 +322,7 @@ trim_coverage(Config) when is_list(Config) ->
<<0,0,0,2,0,0,5,48,0,11,219,174,0,0,0,0>> = coverage_materialiv(a, b, {1328,777134}),
<<67,40,0,0,66,152,0,0,69,66,64,0>> = coverage_trimmer([42,19,777]),
<<0,0,2,43,0,0,3,9,0,0,0,3,64,8,0,0,0,0,0,0,
64,68,0,0,0,0,0,0,192,171,198,0,0,0,0,0>> =
64,68,0,0,0,0,0,0,192,171,198,0,0,0,0,0>> =
coverage_lightfv(555, 777, {3.0,40.0,-3555.0}),
<<"abcabc">> = coverage_strange(0, <<"abc">>),
ok.
Expand Down Expand Up @@ -680,6 +680,28 @@ grab_bag_gh_8617(Bin) ->
[0 || <<_:0, _:(tuple_size({self()}))>> <= Bin,
is_pid(id(self()))].

non_skipping_generators(_Config) ->
%% Basic non-skipping generators (each generator type)
<<2,3,4>> = << <<(X+1)>> || X <:- [1,2,3]>>,
<<2,3,4>> = << <<(X+1)>> || <<X>> <:= <<1,2,3>> >>,
<<2,12>> = << <<(X*Y)>> || X := Y <:- #{1 => 2, 3 => 4} >>,

%% A failing guard following a non-skipping generator is ok
<<3,4>> = << <<(X+1)>> || X <:- [1,2,3], X > 1>>,
<<3,4>> = << <<(X+1)>> || <<X>> <:= <<1,2,3>>, X > 1 >>,
<<12>> = << <<(X*Y)>> || X := Y <:- #{1 => 2, 3 => 4}, X > 1 >>,

%% Non-matching elements cause a badmatch error for non-skipping generators
{'EXIT',{{badmatch,2},_}} = (catch << <<X>> || {ok, X} <:- [{ok,1},2,{ok,3}] >>),
{'EXIT',{{badmatch,<<128,2>>},_}} = (catch << <<X>> || <<0:1, X:7>> <:= <<1,128,2>> >>),
{'EXIT',{{badmatch,{2,error}},_}} = (catch << <<X>> || X := ok <:- #{1 => ok, 2 => error, 3 => ok} >>),

%% Extra bits cannot be skipped at the end of the binary either
{'EXIT',{{badmatch,<<0:2>>},_}} = (catch [X || <<X:3>> <:= <<0>>]),
{'EXIT',{{badmatch,<<9,2>>},_}} = (catch [Y || <<X, Y:X>> <:= <<8,1,9,2>>]),

ok.

cs_init() ->
erts_debug:set_internal_state(available_internal_state, true),
ok.
Expand Down Expand Up @@ -717,7 +739,7 @@ cs(Bin) ->
%% Verify that the allocated size of the binary is the default size.
cs_default(Bin) ->
ByteSize = byte_size(Bin),
{refc_binary,ByteSize,{binary,256},_} =
{refc_binary,ByteSize,{binary,256},_} =
erts_debug:get_internal_state({binary_info,Bin}),
Bin.

Expand Down
17 changes: 16 additions & 1 deletion lib/compiler/test/lc_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,16 @@ basic(Config) when is_list(Config) ->
%% Not matching.
[] = [3 || {3=4} <- []],

%% Non-skipping generators (each generator type)
[2,3,4] = [X+1 || X <:- [1,2,3]],
[2,3,4] = [X+1 || <<X>> <:= <<1,2,3>>],
[2,12] = [X*Y || X := Y <:- #{1 => 2, 3 => 4}],

%% A failing guard following a non-skipping generator is ok
[3,4] = [X+1 || X <:- [1,2,3], X > 1],
[3,4] = [X+1 || <<X>> <:= <<1,2,3>>, X > 1],
[12] = [X*Y || X := Y <:- #{1 => 2, 3 => 4}, X > 1],

%% Error cases.
[] = [{xx,X} || X <- L0, element(2, X) == no_no_no],
{'EXIT',_} = (catch [X || X <- L1, list_to_atom(X) == dum]),
Expand All @@ -109,6 +119,11 @@ basic(Config) when is_list(Config) ->
{'EXIT',{{bad_generator,x},_}} = (catch [E || E <- id(x)]),
{'EXIT',{{bad_filter,not_bool},_}} = (catch [E || E <- [1,2], id(not_bool)]),

%% Non-matching elements cause a badmatch error for non-skipping generators
{'EXIT',{{badmatch,2},_}} = (catch [X || {ok, X} <:- [{ok,1},2,{ok,3}]]),
{'EXIT',{{badmatch,<<128,2>>},_}} = (catch [X || <<0:1, X:7>> <:= <<1,128,2>>]),
{'EXIT',{{badmatch,{2,error}},_}} = (catch [X || X := ok <:- #{1 => ok, 2 => error, 3 => ok}]),

%% Make sure that line numbers point out the generator.
case ?MODULE of
lc_inline_SUITE ->
Expand Down Expand Up @@ -173,7 +188,7 @@ no_generator(Config) when is_list(Config) ->
[a,b,c] = [a || true] ++ [b,c],
ok.

no_gen(A, B) ->
no_gen(A, B) ->
[{A,B} || A+B =:= 0] ++
[{A,B} || A*B =:= 0] ++
[{A,B} || A rem B =:= 3] ++
Expand Down
15 changes: 15 additions & 0 deletions lib/compiler/test/mc_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@ basic(_Config) ->
N rem 2 =:= 1]),
Odd = lists:sort([V || #foo{a=N} := V <- RecordMap, N rem 2 =:= 1]),

%% Non-skipping generators (each generator type)
#{1 := 2, 2 := 3, 3 := 4} = #{X => X+1 || X <:- [1,2,3]},
#{1 := 2, 2 := 3, 3 := 4} = #{X => X+1 || <<X>> <:= <<1,2,3>>},
#{2 := 4, 4 := 8} = #{X+1 => Y*2 || X := Y <:- #{1 => 2, 3 => 4}},

%% A failing guard following a non-skipping generator is ok
#{2 := 3, 3 := 4} = #{X => X+1 || X <:- [1,2,3], X > 1},
#{2 := 3, 3 := 4} = #{X => X+1 || <<X>> <:= <<1,2,3>>, X > 1},
#{4 := 8} = #{X+1 => Y*2 || X := Y <:- #{1 => 2, 3 => 4}, X > 1},

%% Non-matching elements cause a badmatch error for non-skipping generators
{'EXIT',{{badmatch,2},_}} = (catch #{X => X+1 || {ok, X} <:- [{ok,1},2,{ok,3}]}),
{'EXIT',{{badmatch,<<128,2>>},_}} = (catch #{X => X+1 || <<0:1, X:7>> <:= <<1,128,2>>}),
{'EXIT',{{badmatch,{2,error}},_}} = (catch #{X => X+1 || X := ok <:-#{1 => ok, 2 => error, 3 => ok}}),

ok.

mc_double(Size) ->
Expand Down
Loading

0 comments on commit ad90a17

Please sign in to comment.