Skip to content

Complete maps.erl take/2 etc.#2355

Open
petermm wants to merge 5 commits into
atomvm:release-0.7from
petermm:add-maps-take-update-with-with-without
Open

Complete maps.erl take/2 etc.#2355
petermm wants to merge 5 commits into
atomvm:release-0.7from
petermm:add-maps-take-update-with-with-without

Conversation

@petermm

@petermm petermm commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

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

Comment thread libs/estdlib/src/maps.erl Outdated
Comment thread libs/estdlib/src/maps.erl Outdated
Comment thread CHANGELOG.md Outdated
Comment thread tests/libs/estdlib/test_maps.erl
@petermm
petermm marked this pull request as draft July 7, 2026 08:51
@petermm
petermm force-pushed the add-maps-take-update-with-with-without branch from 3683d22 to 00b26a2 Compare July 10, 2026 15:16
petermm added 5 commits July 23, 2026 11:12
OTP-compatible implementations of maps:take/2,
maps:update_with/3, maps:update_with/4, maps:with/2 and
maps:without/2 to the estdlib maps module.

Each function preserves OTP error semantics:

- {badmap, Map} when the map argument is not a map (taking
  precedence over badarg when multiple arguments are wrong)
- badarg when Keys is not a list (with/2, without/2) or Fun
  is not a function of arity 1 (update_with/3,4)
- {badkey, Key} from update_with/3 only when Map is a valid
  map, Fun is arity-1 and Key is missing
- update_with/4 inserts Init verbatim without invoking Fun
  when the key is absent

Tests cover the happy paths plus error precedence, duplicate
keys in with/without, a value of the atom 'error' in take/2
(must not be confused with the missing-key result), and the
guarantee that update_with/4 does not call Fun on insert.

Signed-off-by: Peter M <petermm@gmail.com>
…iterator_valid

Add six pure Erlang functions to the estdlib maps module for
improved OTP compatibility:

- filtermap/2: Combined filter and map operation
- intersect/2: Map intersection (values from second map)
- intersect_with/3: Map intersection with value combiner
- groups_from_list/2: Group list elements by key function
- groups_from_list/3: Group with value transformation
- is_iterator_valid/1: Iterator validation (internal, exported)

All functions follow OTP semantics exactly, include comprehensive
error handling ({badmap, Map}, badarg), work with both maps and
iterators where applicable, and have full test coverage.

Signed-off-by: Peter M <petermm@gmail.com>
Signed-off-by: Peter M <petermm@gmail.com>
Align map helper error precedence and iterator handling with current OTP semantics, including ordered and key-value iterator forms.

Harden maps:next/1 against invalid numeric positions and malformed ordered iterators, and expand estdlib regression coverage for callback order, iterator validation, and badarg/badmap behavior.

Signed-off-by: Peter M <petermm@gmail.com>
Signed-off-by: Peter M <petermm@gmail.com>
@petermm
petermm force-pushed the add-maps-take-update-with-with-without branch from 00b26a2 to 0ba30dd Compare July 23, 2026 09:34
@petermm

petermm commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

PR Review: OTP-compatible maps helpers

Scope

Reviewed the following four commits as one change set (HEAD~4..HEAD):

  • 6c712108amaps: Add take, update_with, with, without
  • 213ed8f16maps: Add filtermap, intersect, intersect_with, groups_from_list, is_iterator_valid
  • 33c004c06maps:take/2 nif
  • 7b34dfeeemaps: align helpers with OTP behavior

Files reviewed:

  • libs/estdlib/src/maps.erl
  • src/libAtomVM/nifs.c
  • src/libAtomVM/nifs.gperf
  • tests/libs/estdlib/test_maps.erl
  • CHANGELOG.md

The Erlang behavior was compared with the BEAM source in /Users/petermm/OSScontrib/otp at OTP-29.0.3 (maint-29, commit 40ea544c97aa8492596f35a88286ea80326510ec), including invalid callback precedence, malformed iterators, ordered iterators, callback order, and groups_from_list/2,3 exception behavior. Oracle was also asked to independently review correctness, GC safety, malformed iterator handling, and OTP compatibility.

Verdict

Approve — no actionable correctness findings.

The implementation preserves the intended OTP behavior, and the native maps:take/2 implementation correctly roots and reloads terms across a potentially moving GC. The iterator NIF validates numeric positions and the complete ordered-key list before accessing map storage. Invalid iterator errors are translated to {badmap, OriginalIterator} without masking callback exceptions.

Findings

No blocking or non-blocking defects found.

Areas specifically checked

  • maps:take/2 roots both arguments across GC, repeats the key lookup after GC, reserves enough heap for the copied map and result tuple, and handles allocation failures.
  • maps:next/1 rejects negative and out-of-range numeric positions, improper ordered-key lists, and ordered iterators containing absent keys.
  • filter/2, filtermap/2, fold/3, foreach/2, and map/2 preserve callback order for ordered iterators and only translate iterator badarg failures into badmap errors.
  • intersect/2 and intersect_with/3 preserve the value and combiner argument ordering when the smaller map is the second input.
  • update_with/3,4, with/2, and without/2 implement the tested OTP error precedence.
  • groups_from_list/2,3 validates the full list before invoking callbacks and preserves callback exceptions from the try ... of body.

Applied test suggestion

Added explicit groups_from_list/2,3 regression tests confirming that exceptions from both key and value callbacks propagate unchanged rather than being converted to badarg. This protects the subtle try ... of exception boundary used by the OTP-compatible implementation.

Verification

  • git diff --check HEAD~4..HEAD — passed.
  • Full test_estdlib.avm archive on AtomVM — passed, including test_maps.
  • Added callback-exception regression tests for groups_from_list/2,3 — passed on AtomVM.
  • Source and differential edge-case checks against the local OTP 29.0.3 BEAM checkout — matched the intended behavior.
  • Oracle review — no actionable findings; identified only unverified hardening/coverage risks, not defects.

The full estdlib run emits expected crash reports from tests that intentionally exercise process failure paths and skips unavailable platform facilities; it finishes with every listed test module, including test_maps, returning ok.

@petermm
petermm marked this pull request as ready for review July 23, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants