From 4f7a0552e38ba23997e3c2ae7a507bb821a045f8 Mon Sep 17 00:00:00 2001
From: Paul Guyot <pguyot@kallisys.net>
Date: Tue, 25 Apr 2023 20:31:51 +0200
Subject: [PATCH] Fix API for several map BIFs, with tests

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`

Signed-off-by: Paul Guyot <pguyot@kallisys.net>
---
 src/libAtomVM/bif.c              | 25 +++++++++----------------
 tests/libs/estdlib/test_maps.erl |  6 ++++++
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/libAtomVM/bif.c b/src/libAtomVM/bif.c
index 60bb51f4f..59b3e4eb2 100644
--- a/src/libAtomVM/bif.c
+++ b/src/libAtomVM/bif.c
@@ -176,13 +176,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);
     }
@@ -253,13 +252,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);
     }
@@ -270,30 +268,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);
diff --git a/tests/libs/estdlib/test_maps.erl b/tests/libs/estdlib/test_maps.erl
index 1cc72d2ac..7aebdffff 100644
--- a/tests/libs/estdlib/test_maps.erl
+++ b/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.