Merge branch 'bjorn/compiler/map-bugs/OTP-12451' into maint

* bjorn/compiler/map-bugs/OTP-12451:
  Be more careful about map patterns when evalutating element/2
  Do not convert map patterns to map expressions

3 files changed, 89 insertions, 76 deletions

diff --git a/lib/compiler/src/sys_core_fold.erl b/lib/compiler/src/sys_core_fold.erl
index 09716d0866..f4f5c3f361 100644
--- a/lib/compiler/src/sys_core_fold.erl
+++ b/lib/compiler/src/sys_core_fold.erl
@@ -1338,9 +1338,12 @@ eval_element(Call, #c_literal{val=Pos}, #c_var{name=V}, Types)
 	{ok,#c_tuple{es=Elements}} ->
 	    if
 		1 =< Pos, Pos =< length(Elements) ->
-		    case lists:nth(Pos, Elements) of
-			#c_alias{var=Alias} -> Alias;
-			Res -> Res
+		    El = lists:nth(Pos, Elements),
+		    try
+			pat_to_expr(El)
+		    catch
+			throw:impossible ->
+			    Call
 		    end;
 		true ->
 		    eval_failure(Call, badarg)
@@ -2040,17 +2043,18 @@ case_opt_args([], Cs, _Sub, _LitExpr, Acc) ->
 %%  Try to expand one argument to several arguments (if tuple/list)
 %%  or to remove a literal argument.
 %%
-case_opt_arg(E0, Sub, Cs, LitExpr) ->
+case_opt_arg(E0, Sub, Cs0, LitExpr) ->
     E = maybe_replace_var(E0, Sub),
     case cerl:is_data(E) of
 	false ->
-            {error,Cs};
+            {error,Cs0};
 	true ->
+	    Cs = case_opt_nomatch(E, Cs0, LitExpr),
 	    case cerl:data_type(E) of
 		{atomic,_} ->
-		    case_opt_lit(E, Cs, LitExpr);
+		    case_opt_lit(E, Cs);
 		_ ->
-		    case_opt_data(E, Cs, LitExpr)
+		    case_opt_data(E, Cs)
 	    end
     end.
 
@@ -2113,8 +2117,26 @@ coerce_to_data(C) ->
 	    coerce_to_data(cerl:alias_pat(C))
     end.
 
-%% case_opt_lit(Literal, Clauses0, LitExpr) ->
-%%           {ok,[],Clauses} | error
+%% case_opt_nomatch(E, Clauses, LitExpr) -> Clauses'
+%%  Remove all clauses that cannot possibly match.
+
+case_opt_nomatch(E, [{[P|_],C,_,_}=Current|Cs], LitExpr) ->
+    case cerl_clauses:match(P, E) of
+        none ->
+            %% The pattern will not match the case expression. Remove
+            %% the clause.  Unless the entire case expression is a
+            %% literal, also emit a warning.
+            case LitExpr of
+                false -> add_warning(C, nomatch_clause_type);
+                true -> ok
+            end,
+            case_opt_nomatch(E, Cs, LitExpr);
+        _ ->
+            [Current|case_opt_nomatch(E, Cs, LitExpr)]
+    end;
+case_opt_nomatch(_, [], _) -> [].
+
+%% case_opt_lit(Literal, Clauses0) -> {ok,[],Clauses} | error
 %%  The current part of the case expression is a literal. That
 %%  means that we will know at compile-time whether a clause
 %%  will match, and we can remove the corresponding pattern from
@@ -2123,68 +2145,48 @@ coerce_to_data(C) ->
 %%  The only complication is if the literal is a binary. Binary
 %%  pattern matching is tricky, so we will give up in that case.
 
-case_opt_lit(Lit, Cs0, LitExpr) ->
-    Cs1 = case_opt_lit_1(Lit, Cs0, LitExpr),
-    try case_opt_lit_2(Lit, Cs1) of
+case_opt_lit(Lit, Cs0) ->
+    try case_opt_lit_1(Lit, Cs0) of
 	Cs ->
 	    {ok,[],Cs}
     catch
 	throw:impossible ->
-            {error,Cs1}
+            {error,Cs0}
     end.
 
-case_opt_lit_1(E, [{[P|_],C,_,_}=Current|Cs], LitExpr) ->
-    case cerl_clauses:match(P, E) of
-        none ->
-            %% The pattern will not match the literal. Remove the clause.
-            %% Unless the entire case expression is a literal, also
-            %% emit a warning.
-            case LitExpr of
-                false -> add_warning(C, nomatch_clause_type);
-                true -> ok
-            end,
-            case_opt_lit_1(E, Cs, LitExpr);
-        _ ->
-            [Current|case_opt_lit_1(E, Cs, LitExpr)]
-    end;
-case_opt_lit_1(_, [], _) -> [].
-
-case_opt_lit_2(E, [{[P|Ps],C,PsAcc,Bs0}|Cs]) ->
-    %% Non-matching clauses have already been removed in case_opt_lit_1/3.
+case_opt_lit_1(E, [{[P|Ps],C,PsAcc,Bs0}|Cs]) ->
+    %% Non-matching clauses have already been removed
+    %% in case_opt_nomatch/3.
     case cerl_clauses:match(P, E) of
 	{true,Bs} ->
 	    %% The pattern matches the literal. Remove the pattern
 	    %% and update the bindings.
-            [{Ps,C,PsAcc,Bs++Bs0}|case_opt_lit_2(E, Cs)];
+            [{Ps,C,PsAcc,Bs++Bs0}|case_opt_lit_1(E, Cs)];
 	{false,_} ->
 	    %% Binary literal and pattern. We are not sure whether
 	    %% the pattern will match.
 	    throw(impossible)
     end;
-case_opt_lit_2(_, []) -> [].
+case_opt_lit_1(_, []) -> [].
 
 %% case_opt_data(Expr, Clauses0, LitExpr) -> {ok,Exprs,Clauses}
 
-case_opt_data(E, Cs0, LitExpr) ->
+case_opt_data(E, Cs0) ->
     Es = cerl:data_es(E),
-    Cs = case_opt_data_1(Cs0, Es,
-			 {cerl:data_type(E),cerl:data_arity(E)},
-			 LitExpr),
-    {ok,Es,Cs}.
-
-case_opt_data_1([{[P|Ps0],C,PsAcc,Bs0}|Cs], Es, TypeSig, LitExpr) ->
-    case case_data_pat(P, TypeSig) of
-	{ok,Ps1,Bs1} ->
-	    [{Ps1++Ps0,C,PsAcc,Bs1++Bs0}|
-	     case_opt_data_1(Cs, Es, TypeSig,LitExpr)];
-	error ->
-	    case LitExpr of
-		false -> add_warning(C, nomatch_clause_type);
-		true -> ok
-	    end,
-	    case_opt_data_1(Cs, Es, TypeSig, LitExpr)
-    end;
-case_opt_data_1([], _, _, _) -> [].
+    TypeSig = {cerl:data_type(E),cerl:data_arity(E)},
+    try case_opt_data_1(Cs0, Es, TypeSig) of
+	Cs ->
+	    {ok,Es,Cs}
+    catch
+	throw:impossible ->
+	    {error,Cs0}
+    end.
+
+case_opt_data_1([{[P|Ps0],C,PsAcc,Bs0}|Cs], Es, TypeSig) ->
+    {ok,Ps1,Bs1} = case_data_pat(P, TypeSig),
+    [{Ps1++Ps0,C,PsAcc,Bs1++Bs0}|
+     case_opt_data_1(Cs, Es, TypeSig)];
+case_opt_data_1([], _, _) -> [].
 
 %% case_data_pat(Pattern, Type, Arity) -> {ok,[Pattern],[{AliasVar,Pat}]} | error.
 
@@ -2193,12 +2195,7 @@ case_data_pat(P, TypeSig) ->
 	false ->
 	    case_data_pat_var(P, TypeSig);
 	true ->
-	    case {cerl:data_type(P),cerl:data_arity(P)} of
-		TypeSig ->
-		    {ok,cerl:data_es(P),[]};
-		{_,_} ->
-		    error
-	    end
+	    {ok,cerl:data_es(P),[]}
     end.
 
 %% case_data_pat_var(Pattern, {DataType,ArityType}) ->
@@ -2218,35 +2215,38 @@ case_data_pat_var(P, {Type,Arity}=TypeSig) ->
 	alias ->
 	    V = cerl:alias_var(P),
 	    Apat = cerl:alias_pat(P),
-	    case case_data_pat(Apat, TypeSig) of
-		{ok,Ps,Bs} ->
-		    {ok,Ps,[{V,cerl:ann_make_data(Ann, Type, unalias_pat_list(Ps))}|Bs]};
-		error ->
-		    error
-	    end;
-	_ ->
-	    error
+	    {ok,Ps,Bs} = case_data_pat(Apat, TypeSig),
+	    {ok,Ps,[{V,cerl:ann_make_data(Ann, Type,
+					  pat_to_expr_list(Ps))}|Bs]}
     end.
 
-%% unalias_pat(Pattern) -> Pattern.
-%%  Remove all the aliases in a pattern but using the alias variables
-%%  instead of the values.  We KNOW they will be bound.
+%% pat_to_expr(Pattern) -> Expression.
+%%  Convert a pattern to an expression if possible. We KNOW that
+%%  all variables in the pattern will be bound.
+%%
+%%  Throw an 'impossible' exception if a map or (non-literal)
+%%  binary is encountered. Trying to use a map pattern as an
+%%  expression is incorrect, while rebuilding a potentially
+%%  huge binary in an expression would be wasteful.
 
-unalias_pat(P) ->
-    case cerl:is_c_alias(P) of
-	true ->
+pat_to_expr(P) ->
+    case cerl:type(P) of
+	alias ->
 	    cerl:alias_var(P);
-	false ->
+	var ->
+	    P;
+	_ ->
 	    case cerl:is_data(P) of
 		false ->
-		    P;
+		    %% Map or binary.
+		    throw(impossible);
 		true ->
-		    Es = unalias_pat_list(cerl:data_es(P)),
+		    Es = pat_to_expr_list(cerl:data_es(P)),
 		    cerl:update_data(P, cerl:data_type(P), Es)
 	    end
     end.
 
-unalias_pat_list(Ps) -> [unalias_pat(P) || P <- Ps].
+pat_to_expr_list(Ps) -> [pat_to_expr(P) || P <- Ps].
 
 make_vars(A, Max) ->
     make_vars(A, 1, Max).
diff --git a/lib/compiler/test/core_fold_SUITE.erl b/lib/compiler/test/core_fold_SUITE.erl
index 6a7036d728..9c7e97a1bf 100644
--- a/lib/compiler/test/core_fold_SUITE.erl
+++ b/lib/compiler/test/core_fold_SUITE.erl
@@ -60,6 +60,12 @@ t_element(Config) when is_list(Config) ->
     X = make_ref(),
     ?line X = id(element(1, {X,y,z})),
     ?line b = id(element(2, {a,b,c,d})),
+    (fun() ->
+	    case {a,#{k=>X}} of
+		{a,#{k:=X}}=Tuple ->
+		    #{k:=X} = id(element(2, Tuple))
+	    end
+    end)(),
 
     %% No optimization, but should work.
     Tuple = id({x,y,z}),
diff --git a/lib/compiler/test/match_SUITE.erl b/lib/compiler/test/match_SUITE.erl
index e5aaf49d6f..1e778dca24 100644
--- a/lib/compiler/test/match_SUITE.erl
+++ b/lib/compiler/test/match_SUITE.erl
@@ -406,12 +406,19 @@ underscore(Config) when is_list(Config) ->
 match_map(Config) when is_list(Config) ->
     Map = #{key=>{x,y},ignore=>anything},
     #s{map=Map,t={x,y}} = do_match_map(#s{map=Map}),
+    {a,#{k:={a,b,c}}} = do_match_map_2(#{k=>{a,b,c}}),
     ok.
 
 do_match_map(#s{map=#{key:=Val}}=S) ->
     %% Would crash with a 'badarg' exception.
     S#s{t=Val}.
 
+do_match_map_2(Map) ->
+    case {a,Map} of
+	{a,#{k:=_}}=Tuple ->
+	    Tuple
+    end.
+
 coverage(Config) when is_list(Config) ->
     %% Cover beam_dead.
     ok = coverage_1(x, a),