Merge pull request #521 from pguyot/w17/fix-map-api

Fix API for several map BIFs

This PR is a successor of #516

Fix the following APIs, using new `memory_ensure_free_opt` roots arg:
- `erlang:is_map_key/2`
- `erlang:map_size/1`
- `erlang:map_get/2`

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

src/libAtomVM/bif.c

@@ -186,13 +186,12 @@ term bif_erlang_is_map_1(Context *ctx, term arg1)
 term bif_erlang_is_map_key_2(Context *ctx, term arg1, term arg2)
 {
     if (UNLIKELY(!term_is_map(arg2))) {
-        if (UNLIKELY(memory_ensure_free_opt(ctx, 3, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
+        if (UNLIKELY(memory_ensure_free_with_roots(ctx, 3, 1, &arg2, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
             RAISE_ERROR(OUT_OF_MEMORY_ATOM);
         }
         term err = term_alloc_tuple(2, ctx);
         term_put_tuple_element(err, 0, BADMAP_ATOM);
-        // TODO elt(2) of err term is supposed to be arg2 but may be invalidated by GC
-        term_put_tuple_element(err, 1, UNSUPPORTED_ATOM);
+        term_put_tuple_element(err, 1, arg2);
 
         RAISE_ERROR(err);
     }
@@ -263,13 +262,12 @@ term bif_erlang_map_size_1(Context *ctx, int live, term arg1)
     UNUSED(live);
 
     if (!UNLIKELY(term_is_map(arg1))) {
-        if (UNLIKELY(memory_ensure_free_opt(ctx, 3, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
+        if (UNLIKELY(memory_ensure_free_with_roots(ctx, 3, 1, &arg1, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
             RAISE_ERROR(OUT_OF_MEMORY_ATOM);
         }
         term err = term_alloc_tuple(2, ctx);
         term_put_tuple_element(err, 0, BADMAP_ATOM);
-        // TODO elt(2) of err term is supposed to be arg1 but may be invalidated by GC
-        term_put_tuple_element(err, 1, UNSUPPORTED_ATOM);
+        term_put_tuple_element(err, 1, arg1);
 
         RAISE_ERROR(err);
     }
@@ -280,30 +278,25 @@ term bif_erlang_map_size_1(Context *ctx, int live, term arg1)
 term bif_erlang_map_get_2(Context *ctx, term arg1, term arg2)
 {
     if (!UNLIKELY(term_is_map(arg2))) {
-        if (UNLIKELY(memory_ensure_free_opt(ctx, 3, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
+        if (UNLIKELY(memory_ensure_free_with_roots(ctx, 3, 1, &arg2, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
             RAISE_ERROR(OUT_OF_MEMORY_ATOM);
         }
         term err = term_alloc_tuple(2, ctx);
         term_put_tuple_element(err, 0, BADMAP_ATOM);
-        // TODO elt(2) of err term is supposed to be arg2 but may be invalidated by GC
-        term_put_tuple_element(err, 1, UNSUPPORTED_ATOM);
+        term_put_tuple_element(err, 1, arg2);
 
         RAISE_ERROR(err);
     }
 
     int pos = term_find_map_pos(arg2, arg1, ctx->global);
     if (pos == TERM_MAP_NOT_FOUND) {
-        if (UNLIKELY(memory_ensure_free_opt(ctx, 3, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
+        if (UNLIKELY(memory_ensure_free_with_roots(ctx, 3, 1, &arg1, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
             RAISE_ERROR(OUT_OF_MEMORY_ATOM);
         }
         term err = term_alloc_tuple(2, ctx);
         term_put_tuple_element(err, 0, BADKEY_ATOM);
-        if (term_is_atom(arg1)) {
-            term_put_tuple_element(err, 1, arg1);
-        } else {
-            // TODO elt(2) of err term is supposed to be arg1 but may be invalidated by GC
-            term_put_tuple_element(err, 1, UNSUPPORTED_ATOM);
-        }
+        term_put_tuple_element(err, 1, arg1);
+
         RAISE_ERROR(err);
     } else if (UNLIKELY(pos == TERM_MAP_MEMORY_ALLOC_FAIL)) {
         RAISE_ERROR(OUT_OF_MEMORY_ATOM);

tests/libs/estdlib/test_maps.erl

@@ -49,12 +49,15 @@ test_get() ->
     ok = check_bad_key(fun() -> maps:get(bar, id(#{foo => bar})) end, bar),
 
     ?ASSERT_MATCH(maps:get(gnu, id(#{foo => bar}), gnat), gnat),
+    ?ASSERT_FAILURE(maps:get({hello}, id(#{foo => bar})), {badkey, {hello}}),
+    ?ASSERT_FAILURE(maps:get(gnu, id({hello})), {badmap, {hello}}),
     ok.
 
 test_is_key() ->
     ?ASSERT_MATCH(maps:is_key(foo, id(#{foo => bar})), true),
     ok = check_bad_map(fun() -> maps:is_key(bar, id(not_a_map)) end),
     ?ASSERT_MATCH(maps:is_key(bar, id(#{foo => bar})), false),
+    ?ASSERT_FAILURE(maps:is_key(gnu, id({hello})), {badmap, {hello}}),
     ok.
 
 test_put() ->
@@ -108,6 +111,7 @@ test_from_list() ->
 test_size() ->
     ?ASSERT_MATCH(maps:size(maps:new()), 0),
     ?ASSERT_MATCH(maps:size(#{a => 1, b => 2, c => 3}), 3),
+    ?ASSERT_FAILURE(maps:size({hello}), {badmap, {hello}}),
     ok = check_bad_map(fun() -> maps:size(id(not_a_map)) end),
     ok.
 
@@ -176,6 +180,8 @@ test_update() ->
     ?ASSERT_EQUALS(maps:update(b, 20, #{a => 1, b => 2, c => 3}), #{a => 1, b => 20, c => 3}),
     ?ASSERT_EQUALS(maps:update(c, 30, #{a => 1, b => 2, c => 3}), #{a => 1, b => 2, c => 30}),
     ?ASSERT_FAILURE(maps:update(d, 40, #{a => 1, b => 2, c => 3}), {badkey, d}),
+    ?ASSERT_FAILURE(maps:update({hello}, 40, #{a => 1, b => 2, c => 3}), {badkey, {hello}}),
+    ?ASSERT_FAILURE(maps:update(a, 40, {hello}), {badmap, {hello}}),
     ok = check_bad_map(fun() -> maps:update(foo, bar, id(not_a_map)) end),
     ok.