Merge branch 'bjorn/compiler/fix-mutable-variables/24/GH-6873/OTP-18470' into maint-24

* bjorn/compiler/fix-mutable-variables/24/GH-6873/OTP-18470:
  Eliminate "mutable variables"

3 files changed, 117 insertions, 90 deletions

diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl
index 2fd31656ca..8681c42d57 100644
--- a/lib/compiler/src/v3_core.erl
+++ b/lib/compiler/src/v3_core.erl
@@ -748,85 +748,47 @@ expr({call,L,FunExp,As0}, St0) ->
     Lanno = lineno_anno(L, St2),
     {#iapply{anno=#a{anno=Lanno},op=Fun,args=As1},Fps ++ Aps,St2};
 expr({match,L,P0,E0}, St0) ->
-    %% First fold matches together to create aliases.
-    {P1,E1} = fold_match(E0, P0),
-    St1 = set_wanted(P1, St0),
-    {E2,Eps1,St2} = novars(E1, St1),
-    St3 = St2#core{wanted=St0#core.wanted},
-    {P2,St4} = try
-                   pattern(P1, St3)
-               catch
-                   throw:Thrown ->
-                       {Thrown,St3}
-               end,
-    {Fpat,St5} = new_var(St4),
-    Lanno = lineno_anno(L, St5),
-    Fc = fail_clause([Fpat], Lanno, c_tuple([#c_literal{val=badmatch},Fpat])),
-    case P2 of
-	nomatch ->
-	    %% The pattern will not match. We must take care here to
-	    %% bind all variables that the pattern would have bound
-	    %% so that subsequent expressions do not refer to unbound
-	    %% variables.
-	    %%
-	    %% As an example, this code:
-	    %%
-	    %%   [X] = {Y} = E,
-	    %%   X + Y.
-	    %%
-	    %% will be rewritten to:
-	    %%
-	    %%   error({badmatch,E}),
-	    %%   case E of
-	    %%      {[X],{Y}} ->
-	    %%        X + Y;
-	    %%      Other ->
-	    %%        error({badmatch,Other})
-	    %%   end.
-	    %%
-	    St6 = add_warning(L, {nomatch,pattern}, St5),
-	    {Expr,Eps3,St7} = safe(E1, St6),
-	    SanPat0 = sanitize(P1),
-	    {SanPat,St} = pattern(SanPat0, St7),
-	    Badmatch = c_tuple([#c_literal{val=badmatch},Expr]),
-	    Fail = #iprimop{anno=#a{anno=Lanno},
-			    name=#c_literal{val=match_fail},
-			    args=[Badmatch]},
-	    Eps = Eps3 ++ [Fail],
-	    {#imatch{anno=#a{anno=Lanno},pat=SanPat,arg=Expr,fc=Fc},Eps,St};
-	Other when not is_atom(Other) ->
-            %% We must rewrite top-level aliases to lets to avoid unbound
-            %% variables in code such as:
+    St1 = set_wanted(P0, St0),
+    case fold_match(E0, P0) of
+        {{sequential_match,_,_,_}=P1,E1} ->
+            %% Matching of an expression to more than one pattern. Example:
             %%
-            %%     <<42:Sz>> = Sz = B
+            %%    #rec{f=Val} = A = Expr
+            {E2,Eps1,St2} = safe(E1, St1),
+            St3 = St2#core{wanted=St0#core.wanted},
+
+            %% If necessary, bind the expression to a variable to ensure it is
+            %% only evaluted once.
+            {Var,Eps2,St4} =
+                case E2 of
+                    #c_var{} ->
+                        {E2,[],St3};
+                    _ ->
+                        {Var0,StInt} = new_var(St3),
+                        {Var0,[#iset{var=Var0,arg=E2}],StInt}
+                end,
+
+            %% Rewrite to a begin/end block matching one pattern at the time
+            %% (using the `single_match` operator). Example:
             %%
-            %% If we would keep the top-level aliases the example would
-            %% be translated like this:
-            %%
-            %% 	   case B of
-            %%         <Sz = #{#<42>(Sz,1,'integer',['unsigned'|['big']])}#>
-            %%            when 'true' ->
-            %%            .
-            %%            .
-            %%            .
-            %%
-            %% Here the variable Sz would be unbound in the binary pattern.
-            %%
-            %% Instead we bind Sz in a let to ensure it is bound when
-            %% used in the binary pattern:
-            %%
-            %%     let <Sz> = B
-            %% 	   in case Sz of
-            %%         <#{#<42>(Sz,1,'integer',['unsigned'|['big']])}#>
-            %%            when 'true' ->
-            %%            .
-            %%            .
-            %%            .
-            %%
-            {P3,E3,Eps2} = letify_aliases(P2, E2),
-            Eps = Eps1 ++ Eps2,
-            {#imatch{anno=#a{anno=Lanno},pat=P3,arg=E3,fc=Fc},Eps,St5}
+            %% begin
+            %%   V = Expr,
+            %%   A = V,
+            %%   #rec{f=Val} = V
+            %% end
+            Block = blockify(L, P1, Var),
+            {E3,Eps3,St5} = expr({block,L,Block}, St4),
+            {E3,Eps1 ++ Eps2 ++ Eps3,St5};
+        {P0,E1} ->
+            %% Matching of an expression to a single pattern. Example:
+            %%    {A,B} = Expr
+            {E2,Eps1,St2} = novars(E1, St1),
+            St3 = St2#core{wanted=St0#core.wanted},
+            {E3,Eps2,St4} = single_match(L, P0, E2, St3),
+            {E3,Eps1 ++ Eps2,St4}
     end;
+expr({single_match,L,P,#c_var{}=E}, St0) ->
+    single_match(L, P, E, St0);
 expr({op,_,'++',{lc,Llc,E,Qs0},More}, St0) ->
     %% Optimise '++' here because of the list comprehension algorithm.
     %%
@@ -867,6 +829,56 @@ expr({op,L,Op,L0,R0}, St0) ->
 	    module=#c_literal{anno=LineAnno,val=erlang},
 	    name=#c_literal{anno=LineAnno,val=Op},args=As},Aps,St1}.
 
+blockify(L0, {sequential_match,_L1,First,Then}, E) ->
+    [{single_match,L0,First,E}|blockify(L0, Then, E)];
+blockify(L, P, E) ->
+    [{single_match,L,P,E}].
+
+%% single_match(Line, AbstractPattern, CoreExpr, State0) -> {Expr,Pre,State}.
+%%  Generate the code for matching an expression against a single pattern.
+single_match(L, P0, E, St0) ->
+    {Fpat,St1} = new_var(St0),
+    Lanno = lineno_anno(L, St1),
+    Fc = fail_clause([Fpat], Lanno, c_tuple([#c_literal{val=badmatch},Fpat])),
+    try pattern(P0, St1) of
+        {P1,St2} ->
+            St3 = set_wanted(P0, St2),
+            St4 = St3#core{wanted=St0#core.wanted},
+            {#imatch{anno=#a{anno=Lanno},pat=P1,arg=E,fc=Fc},[],St4}
+    catch
+        throw:nomatch ->
+            %% The pattern will not match. We must take care here to
+            %% bind all variables that the pattern would have bound
+            %% so that subsequent expressions do not refer to unbound
+            %% variables.
+            %%
+            %% As an example, this code:
+            %%
+            %%   ([X] = {Y}) = E,
+            %%   X + Y.
+            %%
+            %% will be rewritten to:
+            %%
+            %%   error({badmatch,E}),
+            %%   case E of
+            %%      {[X],{Y}} ->
+            %%        X + Y;
+            %%      Other ->
+            %%        error({badmatch,Other})
+            %%   end.
+            %%
+            St2 = add_warning(L, {nomatch,pattern}, St1),
+            {Expr,Eps0,St3} = force_safe(E, St2),
+            SanPat0 = sanitize(P0),
+            {SanPat,St} = pattern(SanPat0, St3),
+            Badmatch = c_tuple([#c_literal{val=badmatch},Expr]),
+            Fail = #iprimop{anno=#a{anno=Lanno},
+                            name=#c_literal{val=match_fail},
+                            args=[Badmatch]},
+            Eps = Eps0 ++ [Fail],
+            {#imatch{anno=#a{anno=Lanno},pat=SanPat,arg=Expr,fc=Fc},Eps,St}
+    end.
+
 %% set_wanted(Pattern, St) -> St'.
 %%  Suppress warnings for expressions that are bound to the '_'
 %%  variable and variables that begin with '_'.
@@ -881,12 +893,6 @@ set_wanted({var,_,Var}, St) ->
     end;
 set_wanted(_, St) -> St.
 
-letify_aliases(#c_alias{var=V,pat=P0}, E0) ->
-    {P1,E1,Eps0} = letify_aliases(P0, V),
-    {P1,E1,[#iset{var=V,arg=E0}|Eps0]};
-letify_aliases(P, E) ->
-    {P,E,[]}.
-
 %% sanitize(Pat) -> SanitizedPattern
 %%  Rewrite Pat so that it will be accepted by pattern/2 and will
 %%  bind the same variables as the original pattern.
@@ -1838,10 +1844,10 @@ is_safe(_) -> false.
 %% fold_match(MatchExpr, Pat) -> {MatchPat,Expr}.
 %%  Fold nested matches into one match with aliased patterns.
 
-fold_match({match,L,P0,E0}, P) ->
-    {P1,E1} = fold_match(E0, P),
-    {{match,L,P0,P1},E1};
-fold_match(E, P) -> {P,E}.
+fold_match({match, L, P, E}, E0) ->
+    fold_match(E, {sequential_match, L, P, E0});
+fold_match(E, E0) ->
+    {E0, E}.
 
 %% pattern(Pattern, State) -> {CorePat,[PreExp],State}.
 %%  Transform a pattern by removing line numbers.  We also normalise
diff --git a/lib/compiler/test/match_SUITE.erl b/lib/compiler/test/match_SUITE.erl
index 422d3a6cf9..68d96ce970 100644
--- a/lib/compiler/test/match_SUITE.erl
+++ b/lib/compiler/test/match_SUITE.erl
@@ -26,7 +26,8 @@
 	 selectify/1,deselectify/1,underscore/1,match_map/1,map_vars_used/1,
 	 coverage/1,grab_bag/1,literal_binary/1,
          unary_op/1,eq_types/1,match_after_return/1,match_right_tuple/1,
-         tuple_size_in_try/1,match_boolean_list/1]).
+         tuple_size_in_try/1,match_boolean_list/1,
+         mutable_variables/1]).
 	 
 -include_lib("common_test/include/ct.hrl").
 
@@ -43,7 +44,8 @@ groups() ->
        underscore,match_map,map_vars_used,coverage,
        grab_bag,literal_binary,unary_op,eq_types,
        match_after_return,match_right_tuple,
-       tuple_size_in_try,match_boolean_list]}].
+       tuple_size_in_try,match_boolean_list,
+       mutable_variables]}].
 
 
 init_per_suite(Config) ->
@@ -1018,4 +1020,24 @@ match_boolean_list(Config) when is_list(Config) ->
              [false | _] -> ok
          end.
 
+%% GH-6873. Bound variables would be overwritten.
+mutable_variables(_Config) ->
+    {'EXIT',{{badmatch,0},_}} = catch mutable_variables_1(),
+
+    F = fun() -> id({tag,whatever}) end,
+    whatever = mutable_variables_2(id({tag,whatever}), F),
+    {'EXIT',{{badmatch,{tag,whatever}},_}} = catch mutable_variables_2(id(a), F),
+
+    ok.
+
+mutable_variables_1() ->
+    Zero = 0,
+    One = 1,
+    Result = One = Zero,
+    {Result,One,Zero}.
+
+mutable_variables_2(Middle, Fun) ->
+    {tag,V} = Middle = Fun(),
+    V.
+
 id(I) -> I.
diff --git a/lib/compiler/test/warnings_SUITE.erl b/lib/compiler/test/warnings_SUITE.erl
index 461b0e2b1a..e66acf2e38 100644
--- a/lib/compiler/test/warnings_SUITE.erl
+++ b/lib/compiler/test/warnings_SUITE.erl
@@ -100,8 +100,7 @@ pattern(Config) when is_list(Config) ->
            [warn_unused_vars],
            {warnings,
             [{{2,15},v3_core,{nomatch,pattern}},
-             {{6,20},v3_core,{nomatch,pattern}},
-             {{11,18},v3_core,{nomatch,pattern}}
+             {{6,20},v3_core,{nomatch,pattern}}
             ]}}],
     [] = run(Config, Ts),
     ok.