Merge branch 'egil/maps-compiler-coverage'

* egil/maps-compiler-coverage:
  compiler: Do not evaluate map expressions with bad keys
  compiler: Throw 'nomatch' on matching with bad binary keys
  compiler: Variable keys are not allowed in Maps
  compiler: Strengthen Maps warnings tests
  compiler: map_pair cannot be a type clause in v3_life
  compiler: Remove redudant code in v3_codegen
  compiler: Test deep map structure
  compiler: Remove redundant clause in v3_codegen
  compiler: Cover #{ [] => Var } in testcase

6 files changed, 107 insertions, 16 deletions

diff --git a/lib/compiler/src/v3_codegen.erl b/lib/compiler/src/v3_codegen.erl
index f1331d1fe7..47a357c23d 100644
--- a/lib/compiler/src/v3_codegen.erl
+++ b/lib/compiler/src/v3_codegen.erl
@@ -960,7 +960,6 @@ select_extract_map(Src, Vs, Fail, I, Vdb, Bef, St) ->
 	end, {{[],[]},Bef}, Vs),
 
     Code = case {HasKs,GetVs} of
-	{[],[]} -> {[],Aft,St};
 	{HasKs,[]} ->
 	    [{test,has_map_fields,{f,Fail},Rsrc,{list,HasKs}}];
 	{[],GetVs} ->
@@ -1553,8 +1552,7 @@ map_pair_strip_and_termsort(Es) ->
     Ls = [{K,V}||{_,K,V}<-Es],
     lists:sort(fun ({{_,A},_}, {{_,B},_}) -> erts_internal:cmp_term(A,B) =< 0;
                    ({nil,_},   {{_,B},_}) -> [] =< B;
-                   ({{_,A},_}, {nil,_})   -> A =< [];
-                   ({nil,_},   {nil,_})   -> true
+                   ({{_,A},_}, {nil,_})   -> A =< []
                end, Ls).
 
 %%%
diff --git a/lib/compiler/src/v3_core.erl b/lib/compiler/src/v3_core.erl
index 04210ae243..a548ba2f7c 100644
--- a/lib/compiler/src/v3_core.erl
+++ b/lib/compiler/src/v3_core.erl
@@ -514,15 +514,26 @@ expr({tuple,L,Es0}, St0) ->
 expr({map,L,Es0}, St0) ->
     % erl_lint should make sure only #{ K => V } are allowed
     % in map construction.
-    {Es1,Eps,St1} = map_pair_list(Es0, St0),
-    A = lineno_anno(L, St1),
-    {ann_c_map(A,Es1),Eps,St1};
+    try map_pair_list(Es0, St0) of
+	{Es1,Eps,St1} ->
+	    A = lineno_anno(L, St1),
+	    {ann_c_map(A,Es1),Eps,St1}
+    catch
+	throw:{bad_map,Warning} ->
+	    St = add_warning(L, Warning, St0),
+	    LineAnno = lineno_anno(L, St),
+	    As = [#c_literal{anno=LineAnno,val=badarg}],
+	    {#icall{anno=#a{anno=LineAnno},	%Must have an #a{}
+		    module=#c_literal{anno=LineAnno,val=erlang},
+		    name=#c_literal{anno=LineAnno,val=error},
+		    args=As},[],St}
+    end;
 expr({map,L,M0,Es0}, St0) ->
     try expr_map(M0,Es0,lineno_anno(L, St0),St0) of
 	{_,_,_}=Res -> Res
     catch
-	throw:bad_map ->
-	    St = add_warning(L, bad_map, St0),
+	throw:{bad_map,Warning} ->
+	    St = add_warning(L, Warning, St0),
 	    LineAnno = lineno_anno(L, St),
 	    As = [#c_literal{anno=LineAnno,val=badarg}],
 	    {#icall{anno=#a{anno=LineAnno},	%Must have an #a{}
@@ -762,7 +773,7 @@ expr_map(M0,Es0,A,St0) ->
 		    {Es1,Eps,St2} = map_pair_list(Es0, St1),
 		    {ann_c_map(A,M1,Es1),Mps++Eps,St2}
 	    end;
-	false -> throw(bad_map)
+	false -> throw({bad_map,bad_map})
     end.
 
 is_valid_map_src(#c_literal{val = M}) when is_map(M) -> true;
@@ -774,18 +785,23 @@ map_pair_list(Es, St) ->
     foldr(fun
 	    ({map_field_assoc,L,K0,V0}, {Ces,Esp,St0}) ->
 		{K,Ep0,St1} = safe(K0, St0),
+		ok = ensure_valid_map_key(K),
 		{V,Ep1,St2} = safe(V0, St1),
 		A = lineno_anno(L, St2),
 		Pair = #c_map_pair{op=#c_literal{val=assoc},anno=A,key=K,val=V},
 		{[Pair|Ces],Ep0 ++ Ep1 ++ Esp,St2};
 	    ({map_field_exact,L,K0,V0}, {Ces,Esp,St0}) ->
 		{K,Ep0,St1} = safe(K0, St0),
+		ok = ensure_valid_map_key(K),
 		{V,Ep1,St2} = safe(V0, St1),
 		A = lineno_anno(L, St2),
 		Pair = #c_map_pair{op=#c_literal{val=exact},anno=A,key=K,val=V},
 		{[Pair|Ces],Ep0 ++ Ep1 ++ Esp,St2}
 	end, {[],[],St}, Es).
 
+ensure_valid_map_key(#c_literal{}) -> ok;
+ensure_valid_map_key(_) -> throw({bad_map,bad_map_key}).
+
 %% try_exception([ExcpClause], St) -> {[ExcpVar],Handler,St}.
 
 try_exception(Ecs0, St0) ->
@@ -1595,7 +1611,9 @@ pattern_map_pair({map_field_exact,L,K,V}, St) ->
 	{bin,L,Es0} ->
 	    case constant_bin(Es0) of
 		error ->
-		    throw(badmatch);
+		    %% this will throw a cryptic error message
+		    %% but it is better than nothing
+		    throw(nomatch);
 		Bin ->
 		    #c_literal{anno=lineno_anno(L,St),val=Bin}
 	    end;
@@ -2299,6 +2317,8 @@ format_error(nomatch) ->
     "pattern cannot possibly match";
 format_error(bad_binary) ->
     "binary construction will fail because of a type mismatch";
+format_error(bad_map_key) ->
+    "map construction will fail because of none literal key (large binaries are not literals)";
 format_error(bad_map) ->
     "map construction will fail because of a type mismatch".
 
diff --git a/lib/compiler/src/v3_kernel.erl b/lib/compiler/src/v3_kernel.erl
index d3b785aa14..40d2f72b4c 100644
--- a/lib/compiler/src/v3_kernel.erl
+++ b/lib/compiler/src/v3_kernel.erl
@@ -581,8 +581,7 @@ map_key_clean(#k_literal{val=V}) -> {k_literal,V};
 map_key_clean(#k_int{val=V})     -> {k_int,V};
 map_key_clean(#k_float{val=V})   -> {k_float,V};
 map_key_clean(#k_atom{val=V})    -> {k_atom,V};
-map_key_clean(#k_nil{})          -> k_nil;
-map_key_clean(#k_var{name=V})    -> {k_var,V}.
+map_key_clean(#k_nil{})          -> k_nil.
 
 
 %% call_type(Module, Function, Arity) -> call | bif | apply | error.
diff --git a/lib/compiler/src/v3_life.erl b/lib/compiler/src/v3_life.erl
index c4f54a7970..cd4b5fd674 100644
--- a/lib/compiler/src/v3_life.erl
+++ b/lib/compiler/src/v3_life.erl
@@ -324,8 +324,7 @@ type(k_binary) -> binary;
 type(k_bin_seg) -> bin_seg;
 type(k_bin_int) -> bin_int;
 type(k_bin_end) -> bin_end;
-type(k_map) -> map;
-type(k_map_pair) -> map_pair.
+type(k_map) -> map.
 
 %% variable(Klit) -> Lit.
 %% var_list([Klit]) -> [Lit].
diff --git a/lib/compiler/test/map_SUITE.erl b/lib/compiler/test/map_SUITE.erl
index 90eae6fb4f..cc018e4305 100644
--- a/lib/compiler/test/map_SUITE.erl
+++ b/lib/compiler/test/map_SUITE.erl
@@ -40,6 +40,8 @@
 	t_build_and_match_over_alloc/1,
 	t_build_and_match_empty_val/1,
 	t_build_and_match_val/1,
+	t_build_and_match_nil/1,
+	t_build_and_match_structure/1,
 
 	%% errors in 17.0-rc1
 	t_update_values/1,
@@ -68,6 +70,8 @@ all() -> [
 	t_build_and_match_over_alloc,
 	t_build_and_match_empty_val,
 	t_build_and_match_val,
+	t_build_and_match_nil,
+	t_build_and_match_structure,
 
 	%% errors in 17.0-rc1
 	t_update_values,
@@ -118,6 +122,7 @@ t_build_and_match_literals(Config) when is_list(Config) ->
     {'EXIT',{{badmatch,_},_}} = (catch (#{x:=3} = id({a,b,c}))),
     {'EXIT',{{badmatch,_},_}} = (catch (#{x:=3} = id(#{y=>3}))),
     {'EXIT',{{badmatch,_},_}} = (catch (#{x:=3} = id(#{x=>"three"}))),
+    {'EXIT',{badarg,_}} = (catch id(#{<<0:258>> =>"three"})),
     ok.
 
 t_build_and_match_aliasing(Config) when is_list(Config) ->
@@ -234,7 +239,8 @@ t_update_assoc(Config) when is_list(Config) ->
     %% Errors cases.
     BadMap = id(badmap),
     {'EXIT',{badarg,_}} = (catch BadMap#{nonexisting=>val}),
-
+    {'EXIT',{badarg,_}} = (catch <<>>#{nonexisting=>val}),
+    {'EXIT',{badarg,_}} = (catch M0#{<<0:257>> => val}), %% limitation
     ok.
 
 t_update_exact(Config) when is_list(Config) ->
@@ -262,6 +268,8 @@ t_update_exact(Config) when is_list(Config) ->
     {'EXIT',{badarg,_}} = (catch M0#{1.0:=v,1.0=>v2}),
     {'EXIT',{badarg,_}} = (catch M0#{42.0:=v,42:=v2}),
     {'EXIT',{badarg,_}} = (catch M0#{42=>v1,42.0:=v2,42:=v3}),
+    {'EXIT',{badarg,_}} = (catch <<>>#{nonexisting:=val}),
+    {'EXIT',{badarg,_}} = (catch M0#{<<0:257>> := val}), %% limitation
     ok.
 
 t_update_values(Config) when is_list(Config) ->
@@ -561,6 +569,32 @@ t_build_and_match_val(Config) when is_list(Config) ->
 	    test_server:fail({no_match, Other})
     end.
 
+t_build_and_match_nil(Config) when is_list(Config) ->
+    %% literals removed the coverage
+    V1 = id(cookie),
+    V2 = id(cake),
+    V3 = id(crisps),
+
+    #{ [] := V1, "treat" := V2, {your,treat} := V3 } = id(#{
+	    {your,treat} => V3,
+	    "treat" => V2, 
+	    [] => V1 }),
+    #{ [] := V3, [] := V3 } = id(#{ [] => V1, [] => V3 }),
+    ok.
+
+t_build_and_match_structure(Config) when is_list(Config) ->
+    V2 = id("it"),
+    S = id([42,{"hi", "=)", #{ "a" => 42, any => any, val => "get_" ++ V2}}]),
+
+    %% match deep map values
+    V2 = case S of
+	[42,{"hi",_, #{ "a" := 42, val := "get_" ++ V1, any := _ }}] -> V1
+    end,
+    %% match deep map
+    ok = case S of
+	[42,{"hi",_, #{ }}] -> ok
+    end,
+    ok.
 
 %% Use this function to avoid compile-time evaluation of an expression.
 id(I) -> I.
diff --git a/lib/compiler/test/warnings_SUITE.erl b/lib/compiler/test/warnings_SUITE.erl
index c3b02819f9..ad4ad91f74 100644
--- a/lib/compiler/test/warnings_SUITE.erl
+++ b/lib/compiler/test/warnings_SUITE.erl
@@ -573,7 +573,48 @@ maps(Config) when is_list(Config) ->
            ">>,
            [],
            {warnings,[{3,sys_core_fold,no_clause_match},
-                      {9,sys_core_fold,nomatch_clause_type}]}}],
+                      {9,sys_core_fold,nomatch_clause_type}]}},
+	   {bad_map_src1,
+           <<"
+             t() ->
+		 M = {a,[]},
+		 {'EXIT',{badarg,_}} = (catch(M#{ a => 1})),
+		 ok.
+           ">>,
+           [],
+	   {warnings,[{4,v3_kernel,bad_map}]}},
+	   {bad_map_src2,
+           <<"
+             t() ->
+		 M = id({a,[]}),
+		 {'EXIT',{badarg,_}} = (catch(M#{ a => 1})),
+		 ok.
+	     id(I) -> I.
+           ">>,
+	   [inline],
+	   {warnings,[{4,v3_kernel,bad_map}]}},
+	   {bad_map_src3,
+           <<"
+             t() ->
+		 {'EXIT',{badarg,_}} = (catch <<>>#{ a := 1}),
+		 ok.
+           ">>,
+           [],
+	   {warnings,[{3,v3_core,bad_map}]}},
+	   {bad_map_literal_key,
+           <<"
+             t() ->
+		 V = id(1),
+		 M = id(#{ <<$h,$i>> => V }),
+		 V = case M of
+		    #{ <<0:257>> := Val } -> Val;
+		    #{ <<$h,$i>> := Val } -> Val
+		 end,
+		 ok.
+	     id(I) -> I.
+           ">>,
+           [],
+	   {warnings,[{6,v3_core,nomatch}]}}],
     run(Config, Ts),
     ok.