Correct error behavior of is_map_key/2 in guards

Consider the following functions:

    foo() -> bar(not_a_map).

    bar(M) when not is_map_key(a, M) -> ok;
    bar(_) -> error.

What will `foo/0` return? It depends. If the module is compiled
with the default compiler options, the return value will be
`ok`. If the module is compiled with the `inline` option,
the return value will be `error`.

The correct value is `error`, because the call to `is_map_key/2`
when the second argument is not a map should fail the entire
guard. That is the way other failing guards BIFs are handled.
For example:

    foo() -> bar(not_a_tuple).

    bar(T) when not element(1, T) -> ok;
    bar(_) -> error.

`foo/0` always returns `error` (whether the code is inlined
or not).

This bug can be fixed by changing the classification of `is_map_key/2`
in the `erl_internal` module. It is now classified as a type test,
which is incorrect because type tests should not fail. Reclassifying
it as a plain guard BIF corrects the bug.

This correction also fixes the internal consistency check
failure which was reported in:

https://bugs.erlang.org/browse/ERL-699

2 files changed, 45 insertions, 6 deletions

diff --git a/lib/compiler/src/v3_codegen.erl b/lib/compiler/src/v3_codegen.erl
index 6cd114abf7..e9152ba88f 100644
--- a/lib/compiler/src/v3_codegen.erl
+++ b/lib/compiler/src/v3_codegen.erl
@@ -1621,11 +1621,6 @@ test_cg(is_boolean, [#k_atom{val=Val}], Fail, I, Vdb, Bef, St) ->
 	     false -> [{jump,{f,Fail}}]
 	 end,
     {Is,Aft,St};
-test_cg(is_map_key, As, Fail, I, Vdb, Bef, St) ->
-    [Key,Map] = cg_reg_args(As, Bef),
-    Aft = clear_dead(Bef, I, Vdb),
-    F = {f,Fail},
-    {[{test,is_map,F,[Map]},{test,has_map_fields,F,Map,{list,[Key]}}],Aft,St};
 test_cg(Test, As, Fail, I, Vdb, Bef, St) ->
     Args = cg_reg_args(As, Bef),
     Aft = clear_dead(Bef, I, Vdb),
diff --git a/lib/compiler/test/map_SUITE.erl b/lib/compiler/test/map_SUITE.erl
index 9182c1b5ed..c004dca834 100644
--- a/lib/compiler/test/map_SUITE.erl
+++ b/lib/compiler/test/map_SUITE.erl
@@ -1212,10 +1212,25 @@ t_guard_bifs(Config) when is_list(Config) ->
     true   = map_guard_empty_2(),
     true   = map_guard_head(#{a=>1}),
     false  = map_guard_head([]),
+
     true   = map_get_head(#{a=>1}),
+    false  = map_get_head(#{}),
+    false  = map_get_head([]),
+
+    true   = map_get_head_not(#{a=>false}),
+    false  = map_get_head_not(#{a=>true}),
+    false  = map_get_head(#{}),
     false  = map_get_head([]),
+
     true   = map_is_key_head(#{a=>1}),
     false  = map_is_key_head(#{}),
+    false  = map_is_key_head(not_a_map),
+
+    false  = map_is_key_head_not(#{a=>1}),
+    true   = map_is_key_head_not(#{b=>1}),
+    true   = map_is_key_head_not(#{}),
+    false  = map_is_key_head_not(not_a_map),
+
     true   = map_guard_body(#{a=>1}),
     false  = map_guard_body({}),
     true   = map_guard_pattern(#{a=>1, <<"hi">> => "hi" }),
@@ -1224,6 +1239,25 @@ t_guard_bifs(Config) when is_list(Config) ->
     true   = map_guard_ill_map_size(),
     true   = map_field_check_sequence(#{a=>1}),
     false  = map_field_check_sequence(#{}),
+
+    %% The guard BIFs used in a body.
+
+    v = map_get(a, id(#{a=>v})),
+    {'EXIT',{{badkey,a},_}} =
+        (catch map_get(a, id(#{}))),
+    {'EXIT',{{badmap,not_a_map},_}} =
+        (catch map_get(a, id(not_a_map))),
+
+    true   = is_map_key(a, id(#{a=>1})),
+    false  = is_map_key(b, id(#{a=>1})),
+    false  = is_map_key(b, id(#{})),
+    {'EXIT',{{badmap,not_a_map},_}} =
+        (catch is_map_key(b, id(not_a_map))),
+
+    {true,v} = erl_699(#{k=>v}),
+    {'EXIT',{{badkey,k},_}} = (catch erl_699(#{})),
+    {'EXIT',{{badmap,not_a_map},_}} = (catch erl_699(not_a_map)),
+
     ok.
 
 map_guard_empty() when is_map(#{}); false -> true.
@@ -1236,8 +1270,14 @@ map_guard_head(_) -> false.
 map_get_head(M) when map_get(a, M) =:= 1 -> true;
 map_get_head(_) -> false.
 
+map_get_head_not(M) when not map_get(a, M) -> true;
+map_get_head_not(_) -> false.
+
 map_is_key_head(M) when is_map_key(a, M) -> true;
-map_is_key_head(M) -> false.
+map_is_key_head(_) -> false.
+
+map_is_key_head_not(M) when not is_map_key(a, M) -> true;
+map_is_key_head_not(_) -> false.
 
 map_guard_body(M) -> is_map(M).
 
@@ -1254,6 +1294,10 @@ map_field_check_sequence(M)
 map_field_check_sequence(_) ->
     false.
 
+erl_699(M) ->
+    %% Used to cause an internal consistency failure.
+    {is_map_key(k, M),maps:get(k, M)}.
+
 t_guard_sequence(Config) when is_list(Config) ->
 	{1, "a"} = map_guard_sequence_1(#{seq=>1,val=>id("a")}),
 	{2, "b"} = map_guard_sequence_1(#{seq=>2,val=>id("b")}),