Allow lists:member/2 in guard clause when List is a literal list

Erlang/OTP Proposals Proposals: Ideas
, ,
1

What:
I suggest allowing lists:member/2 (or a new erlang:is_list_member/2) to be allowed in guard clauses when the list is a literal term similar to how you can use erlang:is_integer:3 but for a set of distinct known-at-compile-time values.

Why:
It makes it easier to read/write clauses which applies when a variable matches a distinct set of values.

% today, many overloads:
old_foo({const0, Value}) ->
    % func_body...
    Ret;
old_foo({const1, Value}) -> ...;
old_foo({const2, Value}) -> ...;
% ...
foo({constN, Value}) -> ...;

% today, chain of 'orelse':
old_foo({Key, Value}) when Key =:= const0 orelse Key =:= const1 orelse ... ->
    % func_body...
    Ret.

% proposed:
new_foo({Key, Value}) when lists:member(Key, [const0, const1, const2, ... constN]) ->
    % func_body...
    Ret.

This is arguably already possible today by using parse_transform, but it does feel a bit hacky to do it that way.

==IN EXPR==
in_foo(I) when lists:member(I, [1, 2, 3]) -> I + 1.

==IN FORMS==
{function,0,in_foo,1,
    [{clause,
         {6,1},
         [{var,{6,5},'I'}],
         [[{call,
               {6,13},
               {remote,{6,18},{atom,{6,13},lists},{atom,{6,19},member}},
               [{var,{6,26},'I'},
                {cons,
                    {6,29},
                    {integer,{6,30},1},
                    {cons,
                        {6,32},
                        {integer,{6,32},2},
                        {cons,{6,34},{integer,{6,34},3},{nil,0}}}}]}]],
         [{op,{7,7},'+',{var,{7,5},'I'},{integer,{7,9},1}}]}]}

==OUT EXPR==
out_foo(I) when I =:= 1 orelse I =:= 2 orelse I =:= 3 ->
    I + 1.

==OUT FORMS==
{function,0,out_foo,1,
          [{clause,0,
                   [{var,{6,5},'I'}],
                   [[{op,0,'orelse',
                         {op,0,'=:=',{var,{6,26},'I'},{integer,{6,30},1}},
                         {op,0,'orelse',
                             {op,0,'=:=',{var,{6,26},'I'},{integer,{6,32},2}},
                             {op,0,'=:=',
                                 {var,{6,26},'I'},
                                 {integer,{6,34},3}}}}]],
                   [{op,{7,7},'+',{var,{7,5},'I'},{integer,{7,9},1}}]}]}
2

How often is this a problem?
Suppose there were a header file defining
?ANYOF(X,A1,A2) (X =:= (A1) orelse X =:= (A2))
…
?ANYOF(X,A1,…,An) (X =:= (A1) orelse … orelse X =:= (An))

new_foo({Key,Value})
when ?ANYOF(Key, const1, …, constn) →
…

How large would n have to be? And how often would it be used?

I just have the gut feeling that code that would use this is missing an important
absstraction in the first place, that it should really be something like

new_foo({magic,Subkey,Value}) →
…

A real example would be useful. Sometimes pain means you’re doing it wrong.

1 Like
3

I do feel the pain of writing an 8-element orelse chain in 8 lines - on the other hand with this lists:member guard it would be still 8 lines because the 8 element literal list would be too long for a single line and if I had to break into multiple lines, one line for one element would be the most readable. On the other hand it would be really confusing for people (especially newcomers) that some lists:member calls would work from a guard but others not, so I don’t think it’s a good idea.

1 Like
4

@nzok I agree that there is always ways around it, I used parse_transform as an example because I implemented one which rewrote the guards, and -define(ANYOF(X,A1,A2,..AN), …) or “magic tuple” is generally a better/more realistic alternatives.

As for how often it’s a problem, honestly I don’t know; I encounter it often enough that I’d appreciate some syntactic sugar for it. But I’m both a sample size of one and someone who enjoys deliberately digging into weird corners of tech so I don’t think extrapolating from my own experiences gives a very accurate picture.

@NAR I see your point, the reason I proposed this option was because it didn’t introduce any new syntax or operators, “foo(Var) when lists:member(Var, [a, b, c]) → ok.” isn’t valid erlang code but it is syntactically valid, or at least valid enough that it gets to the parse_transform before the compiler complains. I think elixirs approach with “when var in [a,b,c]” is cleaner, but that would require introducing a new operator, because I don’t think any of the existing operators are either syntactically valid and/or appropriate to use there.

5

I was always told that guards should run in constant time.

For this reason I am always suspicious of when length(L).

This is discussed in common caveats.

Recently I was wondering whether anyone now considers it a mistake to have allowed length/1 in guard expressions in the beginning?

1 Like
6

Yes, that is why I included the literal list condition, otherwise it wouldn’t be possible to de-sugar it to a chain of ‘orelse’-es at compile time. I guess it’s possible to argue that a chain of short circuiting boolean operators isn’t strictly constant, but at least it has a clearly defined upper bound.

It does make me wonder a bit about how structural comparisons between ‘container types’ fit in here as they would have to recursively walk the structure until they found some difference.

7

I am still of the opinion that a real example would be very instructive. I was under the impression that only specific guard tests were allowed in guards and that none of them involved module: prefixes. As for length,while length(Xs) >= 3 is not constant time, ‘length >=‘(Xs, 3) would be, and I have proposed just such guard tests in the past. (Yes, the meaning is subtly different but that is the point.)

LET’S HAVE A REAL EXAMPLE.

2 Likes
8

Sure, I cleaned up and modified some stuff I’m tinkering with to make it a less cluttered example.

For context suppose you were receiving some logical replication messages from postgres and wanted to extract the transaction id(s), (spec: PostgreSQL: Documentation: 18: 54.9. Logical Replication Message Formats )

tid_overload, tid_chain, and tid_member is well function overloads, orelse chains and lists:member/2 respectively.

-module(pg_wal).

-export([tid_overload/1, tid_chain/1, tid_member/1]).

tid_overload(<<$A, TransactionId:32/big-signed, SubTransactionId:32/big-signed, _/binary>>) ->
    [TransactionId, SubTransactionId];
tid_overload(<<$M, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$R, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$Y, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$I, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$U, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$D, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$T, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$S, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$c, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$B, _:128, TransactionId:32/big-signed>>) ->
    TransactionId;
tid_overload(<<$b, _:192, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$P, _:200, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$K, _:200, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$p, _:200, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$r, _:264, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_overload(<<$C, _/binary>>) ->
    undefined;
tid_overload(<<$O, _/binary>>) ->
    undefined;
tid_overload(<<$E>>) ->
    undefined.

tid_chain(<<$A, TransactionId:32/big-signed, SubTransactionId:32/big-signed, _/binary>>) ->
    [TransactionId, SubTransactionId];
tid_chain(<<$B, _:128, TransactionId:32/big-signed>>) ->
    TransactionId;
tid_chain(<<$b, _:192, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_chain(<<$r, _:264, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_chain(<<T:8, TransactionId:32/big-signed, _/binary>>) when
    T =:= $M orelse
        T =:= $R orelse
        T =:= $Y orelse
        T =:= $I orelse
        T =:= $U orelse
        T =:= $D orelse
        T =:= $T orelse
        T =:= $S orelse
        T =:= $c
->
    TransactionId;
tid_chain(<<T:8, _:200, TransactionId:32/big-signed, _/binary>>) when
    T =:= $P orelse
        T =:= $K orelse
        T =:= $p
->
    TransactionId;
tid_chain(<<T:8, _/binary>>) when
    T =:= $C orelse
        T =:= $O orelse
        T =:= $E
->
    undefined.

tid_member(<<$A, TransactionId:32/big-signed, SubTransactionId:32/big-signed, _/binary>>) ->
    [TransactionId, SubTransactionId];
tid_member(<<$B, _:128, TransactionId:32/big-signed>>) ->
    TransactionId;
tid_member(<<$b, _:192, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_member(<<$r, _:264, TransactionId:32/big-signed, _/binary>>) ->
    TransactionId;
tid_member(<<T:8, TransactionId:32/big-signed, _/binary>>) when
    lists:member(T, [$M, $R, $Y, $I, $U, $D, $T, $S, $c])
->
    TransactionId;
tid_member(<<T:8, _:200, TransactionId:32/big-signed, _/binary>>) when
    lists:member(T, [$P, $K, $p])
->
    TransactionId;
tid_member(<<T:8, _/binary>>) when lists:member(T, [$C, $O, $E]) -> undefined.
9

Yes, that is because guard BIFs didn’t count reductions, so scheduling would become unfair if a process would execute long-running guard BIFs.

I don’t think it was a mistake at the time Erlang was born. To set the stage: computers were generally single-core; memory was measured in megabytes, not gigabytes; there were no literal pools or persistent terms, meaning that a long list necessarily had to be on an Erlang heap. A list long enough to cause problems in a length/1 guard would certainly cause worse problems when that process was garbage-collected.

It is another thing in the modern runtime system with multi-core support, huge amounts of memory, persistent terms, and garbage collection on a dirty schedulers for huge Erlang processes. That is, huge lists are definitely feasible in a real system. If one process spends a long time executing a guard BIF, that can now prevent schedulers on other CPU cores from doing useful work.

That’s why we changed length/1 in OTP 22 to count reductions and do a context switch after a certain number of reductions:

This is not a general solution for yielding in guard BIFs, but a tricky solution specifically handling length/1, so we don’t want to introduce any new guard BIFs that don’t run in constant time.

More recently we started counting reductions in arithmetic operations. There will not be an immediate rescheduling after a heavy arithmetic operation, but because reductions are counted there will be a context switch the next time a function is called:

4 Likes
10

You could possibly use a predefined map with the match values as keys and use the is_map_key guard which effectively behaves like lists:member/2 would.

1 Like
11

Some preliminary remarks on the example.
Here we have a data structure (PostgresQL message) which is defined by a semiformal document and several chunks of code which take it apart differently, with the connection between the Erlang code and the semiformal document not really being obvious. My preference would be to give the responsibility of parsing that data structure in ONE function, converting it to an Erlang data structure from which the existing functions would be trivial extractors.

To be perfectly honest, I’d probably write an AWK script to read the semiformal document and generate the master parsing function from that.

12

While what you say is true for this admittedly somewhat contrived example (which was modified to not clutter the example with unrelated code) I’m not really convinced that “give the responsibility of parsing that data structure in ONE function” is a general solution to “check for set membership in guard tests”.

The is_map_key option that LeonardB proposes is a general solution, but personally I find it very un-ergonomic to use something like my_func(E) when is_map_key(E, #{a=>[],b=>[],c=>[]}) -> ok. , which admittedly is the whole reason I posted this proposal. This isn’t something we can’t do today it’s just really awkward to do.

13

You’re not convinced that the approach I suggested for this one example “is a general solution”? Neither am I. Never said it was. What I did suggest is that
** wanting lists:member/2 in a guard is a CODE SMELL

  • It means the code needs refactoring
    Of course different source code doing different things will need different refactorings.

The issue at hand is whether Erlang needs to be changed.
The ?ANYOF approach is a “general” bandaid that avoids refactoring.
To decide whether Erlang needs to be changed here,

** we need some examples that do NOT cry out “refactor me”.

14

** we need some examples that do NOT cry out “refactor me”.

I don’t believe it will be productive to continue this reply-chain.

There are however two things I’d like to point out here:

  • That is explicitly not what you asked for, I did provide an example in the original post but you insisted in all caps that you wanted a real example
  • You refused to engage with the example you explicitly requested by making assumptions about the context and using that as an argument to invalidate it
2 Likes
15

I think using a map is slower. I did some measurements because I was writing code that gets executed 10000 times a second and there was measurable difference between the andalso code and the maps:get.

16

What assumption did I make about the context?
I carefully read the example.
I carefully read the documentation for the protocol it related to.
I noted that the same packet was parsed three different ways.
I also noted that this obscured the connection between the code
and the specification.
I realised that separating the problem into

  • one parsing function with a clause per item in the specification
    building an Erlang term
  • three accessing functions picking stuff out of that term
    would make the connection between the code and the specification
    far more obvious AND would eliminate the “need” for lists:member/2
    in any guards.
    Context didn’t enter into any of this. It doesn’t matter what the
    context is (assuming that these functions are ever used).

Yes, I wanted an example. A real example.
I’m glad I got one. It was an excellent example of why feeling
a need for lists:member/2 in a guard is a warning sign that a
refactoring is a better choice.
And now I want a real example that doesn’t suggest a need for refactoring.

Here’s the basic principle.
Having a lot of cases that should be handled the same way
suggests that either they aren’t in fact a lot of cases,
but one case that should have been represented as obviously
the same thing (with subcases) in the first place, or
that they probably shouldn’t be handled exactly the same
way (if not now, then in the future), or that you need
to ‘factor’ the process into separate classification and
dispatch parts.

The classic example of the last is a lexical analyser,
where you want all white space characters to be handled
one way, all letters to be handled another, all digits
to be handled a third, and so on. That made the code
simpler, faster, and (in the old days when we had to
worry about such things) easier to adapt to different
character codes.

2 Likes