File 2744-v3_core-Handle-binary-generators-with-repe... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 2744-v3_core-Handle-binary-generators-with-repeated-varia.patch of Package erlang (Revision 49aa26b943fcbed41dda0b0a8caee5fb)

Currently displaying revision 49aa26b943fcbed41dda0b0a8caee5fb , Show latest

From 40c8bd7f14e512953b153565edf5d8daea674aea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= <bjorn@erlang.org>
Date: Sat, 21 Nov 2020 08:32:00 +0100
Subject: [PATCH 4/5] v3_core: Handle binary generators with repeated variables

v3_core would generate incorrect code for binary generators
such as:

foo() ->
         [V || <<V,V>> <= Gen].

The generated code (translated back to Erlang) would look similar to
this:

foo(Gen) ->
        foo_lc(Gen).

foo_lc(<<V,V,Tail/binary>>) ->
        [V|foo_lc(Tail)];
    foo_lc(_) ->
        [].

That is, the generated code lacks a clause that will continue
the iteration if the first clause fails to match.

Correct the code generation to ensure that a skip clause is generated:

foo_lc(<<V,V,Tail/binary>>) ->
        [V|foo_lc(Tail)];
    foo_lc(<<V,NewVar,Tail/binary>>) ->
        foo_lc(Tail);
    foo_lc(_) ->
        [].
---
 lib/compiler/src/v3_core.erl           | 103 +++++++++++++------------
 lib/compiler/test/bs_bincomp_SUITE.erl |  52 ++++++++++---
 2 files changed, 94 insertions(+), 61 deletions(-)

diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl
index a14ad988ae..7b14e15b8c 100644
--- a/lib/compiler/src/v3_core.erl
+++ b/lib/compiler/src/v3_core.erl
@@ -1389,27 +1389,19 @@ lc_tq(Line, E, [#igen{anno=#a{anno=GA}=GAnno,
     Nc = #iapply{anno=GAnno,op=F,args=[Tail]},
     {[FcVar,Var],St2} = new_vars(2, St1),
     Fc = bad_generator([FcVar], FcVar, Arg),
+    SkipClause = #iclause{anno=#a{anno=[skip_clause,compiler_generated|LA]},
+                          pats=[SkipPat],guard=[],body=[Nc]},
     TailClause = #iclause{anno=LAnno,pats=[TailPat],guard=[],body=[Mc]},
-    Cs0 = case {AccPat,AccGuard} of
-              {SkipPat,[]} ->
-                  %% Skip and accumulator patterns are the same and there is
-                  %% no guard, no need to generate a skip clause.
-                  [TailClause];
-              _ ->
-                  [#iclause{anno=#a{anno=[compiler_generated|LA]},
-                            pats=[SkipPat],guard=[],body=[Nc]},
-                   TailClause]
-          end,
-    {Cs,St4} = case AccPat of
-                   nomatch ->
-                       %% The accumulator pattern never matches, no need
-                       %% for an accumulator clause.
-                       {Cs0,St2};
+    {Cs,St4} = case {AccPat,SkipPat} of
+                   {nomatch,nomatch} ->
+                       {[TailClause],St2};
+                   {nomatch,_} ->
+                       {[SkipClause,TailClause],St2};
                    _ ->
                        {Lc,Lps,St3} = lc_tq(Line, E, Qs, Nc, St2),
-                       {[#iclause{anno=LAnno,pats=[AccPat],guard=AccGuard,
-                                  body=Lps ++ [Lc]}|Cs0],
-                        St3}
+                       AccClause = #iclause{anno=LAnno,pats=[AccPat],guard=AccGuard,
+                                            body=Lps ++ [Lc]},
+                       {[AccClause,SkipClause,TailClause],St3}
                end,
     Fun = #ifun{anno=GAnno,id=[],vars=[Var],clauses=Cs,fc=Fc},
     {#iletrec{anno=GAnno#a{anno=[list_comprehension|GA]},defs=[{{Name,1},Fun}],
@@ -1459,30 +1451,21 @@ bc_tq1(Line, E, [#igen{anno=GAnno,
     F = #c_var{anno=LA,name={Name,2}},
     Nc = #iapply{anno=GAnno,op=F,args=[Tail,AccVar]},
     Fc = bad_generator(FcVars, hd(FcVars), Arg),
+    SkipClause = #iclause{anno=#a{anno=[compiler_generated,skip_clause|LA]},
+                          pats=[SkipPat,IgnoreVar],guard=[],body=[Nc]},
     TailClause = #iclause{anno=LAnno,pats=[TailPat,IgnoreVar],guard=[],
                           body=[AccVar]},
-    Cs0 = case {AccPat,AccGuard} of
-              {SkipPat,[]} ->
-                  %% Skip and accumulator patterns are the same and there is
-                  %% no guard, no need to generate a skip clause.
-                  [TailClause];
-              _ ->
-                  [#iclause{anno=#a{anno=[compiler_generated|LA]},
-                            pats=[SkipPat,IgnoreVar],guard=[],body=[Nc]},
-                   TailClause]
-          end,
-    {Cs,St} = case AccPat of
-                  nomatch ->
-                      %% The accumulator pattern never matches, no need
-                      %% for an accumulator clause.
-                      {Cs0,St4};
-                  _ ->
+    {Cs,St} = case {AccPat,SkipPat} of
+                  {nomatch,nomatch} ->
+                      {[TailClause],St4};
+                  {nomatch,_} ->
+                      {[SkipClause,TailClause],St4};
+                  {_,_} ->
                       {Bc,Bps,St5} = bc_tq1(Line, E, Qs, AccVar, St4),
                       Body = Bps ++ [#iset{var=AccVar,arg=Bc},Nc],
-                      {[#iclause{anno=LAnno,
-                                 pats=[AccPat,IgnoreVar],guard=AccGuard,
-                                 body=Body}|Cs0],
-                       St5}
+                      AccClause = #iclause{anno=LAnno,pats=[AccPat,IgnoreVar],
+                                           guard=AccGuard,body=Body},
+                      {[AccClause,SkipClause,TailClause],St5}
               end,
     Fun = #ifun{anno=LAnno,id=[],vars=Vars,clauses=Cs,fc=Fc},
 
@@ -1668,7 +1651,7 @@ generator(Line, {b_generate,Lg,P,E}, Gs, St0) ->
             {AccSegs,Tail,TailSeg,St2} = append_tail_segment(Segs, St1),
             AccPat = Cp#ibinary{segments=AccSegs},
             {Cg,St3} = lc_guard_tests(Gs, St2),
-            {SkipSegs,St4} = emasculate_segments(AccSegs, St3),
+            {SkipSegs,St4} = skip_segments(AccSegs, St3, []),
             SkipPat = Cp#ibinary{segments=SkipSegs},
             {Ce,Pre,St5} = safe(E, St4),
             Gen = #igen{anno=#a{anno=GA},acc_pat=AccPat,acc_guard=Cg,
@@ -1694,15 +1677,21 @@ append_tail_segment(Segs, St0) ->
                     flags=#c_literal{val=[unsigned,big]}},
     {Segs++[Tail],Var,Tail,St}.
 
-emasculate_segments(Segs, St) ->
-    emasculate_segments(Segs, St, []).
-
-emasculate_segments([#ibitstr{val=#c_var{}}=B|Rest], St, Acc) ->
-    emasculate_segments(Rest, St, [B|Acc]);
-emasculate_segments([B|Rest], St0, Acc) ->
+%% skip_segments(Segments, St0, Acc) -> {SkipSegments,St}.
+%%  Generate the segments for a binary pattern that can be used
+%%  in the skip clause that will continue the iteration when
+%%  the accumulator pattern didn't match.
+
+skip_segments([#ibitstr{val=#c_var{}}=B|Rest], St, Acc) ->
+    %% We must keep the names of existing variables to ensure that
+    %% patterns such as <<Size,X:Size>> will work.
+    skip_segments(Rest, St, [B|Acc]);
+skip_segments([B|Rest], St0, Acc) ->
+    %% Replace literal or expression with a variable (whose value will
+    %% be ignored).
     {Var,St1} = new_var(St0),
-    emasculate_segments(Rest, St1, [B#ibitstr{val=Var}|Acc]);
-emasculate_segments([], St, Acc) ->
+    skip_segments(Rest, St1, [B#ibitstr{val=Var}|Acc]);
+skip_segments([], St, Acc) ->
     {reverse(Acc),St}.
 
 lc_guard_tests([], St) -> {[],St};
@@ -2145,8 +2134,22 @@ uclause(Cl0, Ks, St0) ->
     A = A0#a{us=Used,ns=New},
     {Cl1#iclause{anno=A},St1}.
 
-do_uclause(#iclause{anno=Anno,pats=Ps0,guard=G0,body=B0}, Ks0, St0) ->
-    {Ps1,Pg,Pvs,Pus,St1} = upattern_list(Ps0, Ks0, St0),
+do_uclause(#iclause{anno=A0,pats=Ps0,guard=G0,body=B0}, Ks0, St0) ->
+    {Ps1,Pg0,Pvs,Pus,St1} = upattern_list(Ps0, Ks0, St0),
+    Anno = A0#a.anno,
+    {Pg,A} = case member(skip_clause, Anno) of
+                 true ->
+                     %% This is the skip clause for a binary generator.
+                     %% To ensure that it will properly skip the nonmatching
+                     %% patterns in generators such as:
+                     %%
+                     %%   <<V,V>> <= Gen
+                     %%
+                     %% we must remove any generated pre guard.
+                     {[],A0#a{anno=Anno -- [skip_clause]}};
+                 false ->
+                     {Pg0,A0}
+             end,
     Pu = union(Pus, intersection(Pvs, Ks0)),
     Pn = subtract(Pvs, Pu),
     Ks1 = union(Pn, Ks0),
@@ -2157,7 +2160,7 @@ do_uclause(#iclause{anno=Anno,pats=Ps0,guard=G0,body=B0}, Ks0, St0) ->
     {B1,St3} = uexprs(B0, Ks2, St2),
     Used = intersection(union([Pu,Gu,used_in_any(B1)]), Ks0),
     New = union([Pn,Gn,new_in_any(B1)]),
-    {#iclause{anno=Anno,pats=Ps1,guard=G1,body=B1},Pvs,Used,New,St3}.
+    {#iclause{anno=A,pats=Ps1,guard=G1,body=B1},Pvs,Used,New,St3}.
 
 %% uguard([Test], [Kexpr], [KnownVar], State) -> {[Kexpr],State}.
 %%  Build a guard expression list by folding in the equality tests.
diff --git a/lib/compiler/test/bs_bincomp_SUITE.erl b/lib/compiler/test/bs_bincomp_SUITE.erl
index b9d3d3eb4a..6386bce752 100644
--- a/lib/compiler/test/bs_bincomp_SUITE.erl
+++ b/lib/compiler/test/bs_bincomp_SUITE.erl
@@ -26,8 +26,8 @@
 	 init_per_group/2,end_per_group/2,
 	 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,matched_out_size/1,
-         no_generator/1,zero_pattern/1]).
+	 nomatch/1,sizes/1,general_expressions/1,
+         no_generator/1,zero_pattern/1,multiple_segments/1]).
 
 -include_lib("common_test/include/ct.hrl").
 
@@ -36,8 +36,8 @@ suite() -> [{ct_hooks,[ts_install_cth]}].
 all() -> 
     [byte_aligned, bit_aligned, extended_byte_aligned,
      extended_bit_aligned, mixed, filters, trim_coverage,
-     nomatch, sizes, general_expressions, matched_out_size,
-     no_generator, zero_pattern].
+     nomatch, sizes, general_expressions,
+     no_generator, zero_pattern, multiple_segments].
 
 groups() -> 
     [].
@@ -446,13 +446,6 @@ enc_char_cm(C0, Lb, Limit) ->
 
 -undef(BAD).
 
-matched_out_size(Config) when is_list(Config) ->
-    <<1, 2>> = matched_out_size_1(<<4, 1:4, 4, 2:4>>),
-    ok.
-
-matched_out_size_1(Binary) ->
-    << <<X>> || <<S, X:S>> <= Binary>>.
-
 no_generator(_Config) ->
     [<<"abc">>] = [<<(id(<<"abc">>)) || true >>],
     {<<>>} = {<<(id(<<"abc">>)) || false >>},
@@ -483,6 +476,43 @@ zero_pattern(Config) ->
             ok
     end.
 
+multiple_segments(_Config) ->
+    cs_init(),
+
+    [1,2] = matched_out_size(<<4, 1:4, 4, 2:4>>),
+    [42] = matched_out_size(<<16, 42:16, 72>>),
+
+    [] = do_multiple_segments_1(<<>>),
+    [] = do_multiple_segments_1(<<1>>),
+    [] = do_multiple_segments_1(<<1,2>>),
+    [] = do_multiple_segments_1(<<1,2,3>>),
+    [1,4] = do_multiple_segments_1(<<99,0,1,1,2,3,4,4>>),
+
+    [] = do_multiple_segments_2(<<1,2>>),
+    [6] = do_multiple_segments_2(<<1,2,3>>),
+    [6,15] = do_multiple_segments_2(<<1,2,3,4,5,6,7,8>>),
+
+    cs_end(),
+    ok.
+
+matched_out_size(Gen) ->
+    Bin = cs_default(<< <<X>> || <<S,X:S>> <= Gen >>),
+    List = [X || <<S,X:S>> <= Gen],
+    Bin = list_to_binary(List),
+    List.
+
+do_multiple_segments_1(Gen) ->
+    Bin = cs_default(<< <<V>> || <<V,V>> <= Gen >>),
+    List = [V || <<V,V>> <= Gen],
+    Bin = list_to_binary(List),
+    List.
+
+do_multiple_segments_2(Gen) ->
+    Bin = cs(<< <<(A+B+C)>> || <<A,B,C>> <= Gen >>),
+    List = [A+B+C || <<A,B,C>> <= Gen],
+    Bin = list_to_binary(List),
+    List.
+
 cs_init() ->
     erts_debug:set_internal_state(available_internal_state, true),
     ok.
-- 
2.26.2

Places

File 2744-v3_core-Handle-binary-generators-with-repeated-varia.patch of Package erlang (Revision 49aa26b943fcbed41dda0b0a8caee5fb)

Places