otp icon indicating copy to clipboard operation
otp copied to clipboard

Add matchspec BIFs maps_put and maps_remove

Open sverker opened this issue 2 years ago • 6 comments

corresponding to maps:put/3 and maps:remove/2.

This is a quick attempt to fix #7309.

The map syntax to update keys cannot simply be used as Map#{key => value} would be translated by fun2ms to something like '$1'#{key => value} which is not valid Erlang syntax. Instead use the match spec syntax to call BIFs.

1> T = ets:new(x,[]).

2> ets:insert(T, {key, #{a=>1, b=>2}}).

3> MS = ets:fun2ms(fun({Key, Map}) when is_map(Map) -> {Key, maps_put(c, 3, Map)} end).
[{{'$1','$2'},
  [{is_map,'$2'}],
  [{{'$1',{maps_put,c,3,'$2'}}}]}]

4> ets:select_replace(T, MS).
1
5> ets:tab2list(T).
[{key,#{c => 3,a => 1,b => 2}}]

ToDo: Test and documentation.

sverker avatar May 30 '23 12:05 sverker

CT Test Results

No tests were run for this PR. This is either because the build failed, or the PR is based on a branch without GH actions tests configured.

Results for commit e3cdf6c90432db826cb855dda8837bd626ee8162

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

github-actions[bot] avatar May 30 '23 12:05 github-actions[bot]

Could it be that fun2ms translates the map update syntax into map_put calls?

michalmuskala avatar May 31 '23 12:05 michalmuskala

Could it be that fun2ms translates the map update syntax into map_put calls?

It could I suppose. I'm not familiar with the fun2ms code though. You would still need to call maps_remove directly.

sverker avatar May 31 '23 12:05 sverker

Vote for

fun2ms translates the map update syntax into map_put calls

yushli avatar May 31 '23 17:05 yushli

A couple of thoughts:

  • Wouldn't it be better to have this be map_put and map_remove?

    Today's map_get and map_size equivalents do not pluralize maps.

  • Should these be added as general auto-imported BIFs?

    Today's map_get and map_size are named the same as auto-imported BIFs map_get/2 and map_size/2. Do we want this to be internally consistent or just a secret capability of match specs? Or are there equivalent auto-imported BIFS in master and I'm just behind the times?

  • My own (extreme) ignorance showing here in how this works under the hood, but the match spec docs suggest that action functions are only allowed in dbg-dialect ms's. Would this implementation only allow them in dbg-dialects? Or would this be the first set of "action functions" allowed in the ets dialect?

christhekeele avatar Feb 12 '24 18:02 christhekeele

  • Wouldn't it be better to have this be map_put and map_remove?

Yes maybe. I called them maps_put and maps_remove to refer to maps:put and maps:remove.

  • Should these be added as general auto-imported BIFs? Today's map_get and map_size are named the same as auto-imported BIFs [map_get/2]

Maybe. I did this as a quick proof of concept without too much thought of the naming.

  • My own (extreme) ignorance showing here in how this works under the hood, but the match spec docs suggest that action functions are only allowed in dbg-dialect ms's. Would this implementation only allow them in dbg-dialects? Or would this be the first set of "action functions" allowed in the ets dialect?

I don't remember why I added them to ms_transform:action_function/1. They are not action functions in the sense of side effects. maps_put and maps_remove are pure functions returning new maps and will fit in with the existing ets match spec paradigm.

sverker avatar Feb 13 '24 10:02 sverker