Introduce guards in MapSet

127 views
Skip to first unread message

eksperimental

unread,
Dec 21, 2020, 10:36:51 PM12/21/20
to elixir-l...@googlegroups.com
Hi list,
I would like to propose to convert two existing functions to guards in
the MapSet module.

1. Is MapSet.size/1. I found myself not being able to use this function
in guards, and having to port my code, unless I want to get into
the bad habit of reading the internal structure of the struct which is
an opaque type.
This guard can be implemented with :erlang.map_get/2

2. Deprecate MapSet.member?/2 in favor of MapSet.is_member/2

Thank you for reading

eksperimental

unread,
Dec 23, 2020, 10:48:11 PM12/23/20
to elixir-l...@googlegroups.com
Actually, why are we having MapSet.member?/2 when we have
Enum.member?/2. It is redudant.

José Valim

unread,
Dec 24, 2020, 2:37:40 AM12/24/20
to elixir-l...@googlegroups.com
Because if I have a MapSet, I would prefer to call MapSet API to make the intent clearer.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/5fe40f76.1c69fb81.f8161.5ba3SMTPIN_ADDED_MISSING%40gmr-mx.google.com.

Christopher Keele

unread,
Mar 1, 2024, 6:30:49 PMMar 1
to elixir-lang-core
MapSets are very useful constant time data structures; and I keep on finding myself wishing there was a future-proof approved way to use them in guards. Consider this my +1.

I believe eksperimental's original rationale still holds:

1. :erlang.is_map_key/2, as well as :erlang.map_get/2, are awkward to use guards (as of writing, by design, to encourage destructuring)
2. a MapSet's .map is a private field and should not be accessed/destructured directly (see: various pain points when the internal implementation of Range fields changed)

I'd like to re-open a proposal for a guard-safe MapSet.is_member/2, with the implementation:

defmodule MapSet do
defguard is_member(map_set, value)
when is_struct(map_set, __MODULE__) and
:erlang.is_map_key(value, map_set.map)
end

This would bring the core conceit of MapSets (constant-time set membership) into where they are very valuable (constant-time guard operations for pattern matching against function heads).

I am not convinced that deprecating MapSet.member?/2 adds much value if it would force developers to require MapSet before using its core functionality, so I'm content having a bit of duplication. Similarly, checking the size of a MapSet is less core to its utility, and again I wouldn't want to developers to have to require the module just to check the size—so I'm proposing that addition. Just the single guard!

José Valim

unread,
Mar 2, 2024, 3:21:29 AMMar 2
to elixir-l...@googlegroups.com
Please do submit a PR for MapSet.is_member. Since then, we have added map related guards to Kernel, which also do duplicate existing functions in Map, So I think some of my original concerns are no longer relevant. There is also no need to deprecate any of the existing functions.

Christopher Keele

unread,
Mar 2, 2024, 5:28:38 PMMar 2
to elixir-lang-core

PR opened here!

Bruce Tate

unread,
Mar 3, 2024, 8:37:05 AMMar 3
to elixir-l...@googlegroups.com
I'm excited for this one, Chris. Thanks for taking it on. 

-bt



--

Regards,
Bruce Tate
CEO

Aleksei Matiushkin

unread,
Mar 3, 2024, 9:13:55 AMMar 3
to elixir-l...@googlegroups.com
Hi all,

while we are on this, I’d like to share the `mapset/1 macro we use to simplify MapSet matching

 @doc """
 Mapsets in matches are matched the same way as maps, that said the following would be matched

 ```elixir
 fn mapset() -> :ok end.(MapSet.new())
 fn mapset() -> :ok end.(MapSet.new([1, 2]))
 fn mapset([1]) -> :ok end.(MapSet.new([1, 2]))
 ```

 and the following would not

 ```elixir
 fn mapset([1]) -> :ok end.(MapSet.new())
 fn mapset([2]) -> :ok end.(MapSet.new([1]))
 ```

 """
 @doc guard: false
 @spec mapset(list()) :: Macro.t()
 defmacro mapset(list) do
   case __CALLER__.context do
     :guard ->
       raise_opts = fn description ->
         [
           file: Path.relative_to_cwd(__CALLER__.file),
           line: __CALLER__.line,
           description: description
         ]
       end

       raise CompileError, raise_opts.("`mapset/1` macro cannot be used in guards")

     :match ->
       list_ast =
         Enum.map(list, fn
           {:^, meta, [{_, _, _} = elem]} -> {{:^, meta, [elem]}, []}
           # to raise properly, we need the following line
           {_var_name, meta, _} = var -> {{:var!, meta, [var]}, []}
           elem -> {elem, []}
         end)

       map_ast = {:%{}, Macro.Env.location(__CALLER__), list_ast}

       quote do: %MapSet{map: unquote(map_ast)}

     nil ->
       quote do: MapSet.new(unquote(list))
   end
 end

It could be a great addition to the guard, specifically in tests.


Reply all
Reply to author
Forward
0 new messages