Solved: every record is a tuple, but not every tuple is a record

[tsloughter]

I have use of lists:key* on a list of records that Gradualizer complains about:

opentelemetry-erlang/_build/default/lib/opentelemetry_experimental/ebin/otel_meter_server.beam: The function call on line 252 at column 35 is expected to have type [#reader{}] but it has type [tuple()]

    {noreply,
     State#state{readers =
                     lists:keydelete(Ref, #reader.monitor_ref, Readers)}};

Seems it doesn't know a record is a tuple?

[erszcz]

Thanks for raising this, @tsloughter!

Hmmm, there's code to make any record a subtype of tuple() in place, which leads me to believe that the problem is going in the other direction - what is your definition of #state{}? I imagine #state.readers :: [#reader{}], whereas keydelete(Key, N, [tuple()]) -> [tuple()] and that is the problem, as not every tuple is a valid record.

That's confirmed by the following test:

$ cat record_as_tuple_pass.erl
-module(record_as_tuple_pass).

-record(reader, {monitor_ref}).

-spec f() -> [tuple()].
f() ->
    Ref = 1,
    Readers = [#reader{monitor_ref = 1}],
    lists:keydelete(Ref, #reader.monitor_ref, Readers).
$ ./bin/gradualizer record_as_tuple_pass.erl
# no output means success
$

vs

$ cat record_as_tuple_fail.erl
-module(record_as_tuple_fail).

-record(reader, {monitor_ref}).

-spec f() -> [#reader{}].
f() ->
    Ref = 1,
    Readers = [#reader{monitor_ref = 1}],
    lists:keydelete(Ref, #reader.monitor_ref, Readers).
$ ./bin/gradualizer record_as_tuple_fail.erl
record_as_tuple_fail.erl: The function call on line 9 at column 5 is expected to have type [#reader{}] but it has type [tuple()]

    Ref = 1,
    Readers = [#reader{monitor_ref = 1}],
    lists:keydelete(Ref, #reader.monitor_ref, Readers).
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

It's unfortunate, but this is the classic example of losing information when upcasting in the subtyping hierarchy - from the moment #reader{} is treated as a tuple() we don't know for sure anymore whether it's really a #reader{} or not and the downcast is unsafe.

As of now, if we can't trust the data (for example, it comes from outside the system) then anytime such a downcast from a more generic type is necessary it should be preceded by validation that we're really dealing with the type we want to downcast to. For a simple example in Gradualizer itself, we have the code which converts integers to arities.

In this particular case, since we can trust the data - we only passed it to lists:keydelete and got the result back - we can assert the more specific type by including gradualizer.hrl and using a macro:

?assert_type(lists:keydelete(Ref, #reader.monitor_ref, Readers), [#reader{}])

Or wrap the cast in a type-safe interface - if it accepts only [#reader{}] then the cast must be safe:

-spec remove_reader(reference(), [#reader{}]) -> [#reader{}].
remove_reader(Ref, Readers) ->
    ?assert_type(lists:keydelete(Ref, #reader.monitor_ref, Readers), [#reader{}]).

Let's check that:

19:00:06 erszcz @ x6 : ~/work/erszcz/gradualizer (constraint-solver-1 %)
$ cat record_as_tuple_pass.erl
-module(record_as_tuple_pass).

-record(reader, {monitor_ref}).

-include("gradualizer.hrl").

-spec f() -> [tuple()].
f() ->
    Ref = 1,
    Readers = [#reader{monitor_ref = 1}],
    lists:keydelete(Ref, #reader.monitor_ref, Readers).

-spec remove_reader(reference(), [#reader{}]) -> [#reader{}].
remove_reader(Ref, Readers) ->
    ?assert_type(lists:keydelete(Ref, #reader.monitor_ref, Readers), [#reader{}]).
19:00:36 erszcz @ x6 : ~/work/erszcz/gradualizer (constraint-solver-1 %)
$ ./bin/gradualizer -I include/ -- record_as_tuple_pass.erl
19:00:46 erszcz @ x6 : ~/work/erszcz/gradualizer (constraint-solver-1 %)
$

Indeed, it passes!

[erszcz]

In general, there are type system mechanisms to free the user from such mundane wrapping into type-safe abstractions. One of them would be bounded quantification, which we, at least partially, support. Another would be intersection types.

@zuiderkwast I saw the discussion in https://github.com/josefs/Gradualizer/issues/122, but I'm not sure we can control type bounds and therefore use bounded quantification flexibly enough yet. Do you think it's possible? The following suggests it's not (BTW, I'm using https://github.com/erszcz/Gradualizer/tree/constraint-solver-1, but it doesn't seem to make a difference):

$ cat record_as_tuple_bounded.erl
-module(record_as_tuple_bounded).

-record(reader, {monitor_ref}).

%% Our spec for the sake of the example!
-spec keydelete(Key, N, [Elem]) -> [Elem] when
      Key :: any(),
      N :: pos_integer(),
      %% Elem :: tuple(), even though Elem is used twice, doesn't seem to generate a constraint, but just works as an alias.
      Elem :: tuple().

%% But the impl straight from OTP.
keydelete(K, N, L) when is_integer(N), N > 0 ->
    keydelete3(K, N, L).

keydelete3(Key, N, [H|T]) when element(N, H) == Key -> T;
keydelete3(Key, N, [H|T]) ->
    [H|keydelete3(Key, N, T)];
keydelete3(_, _, []) -> [].

-spec g() -> [#reader{}].
g() ->
    Ref = 1,
    Readers = [#reader{monitor_ref = 1}],
    keydelete(Ref, #reader.monitor_ref, Readers).
$ ./bin/gradualizer -I include/ --solve_constraints -- record_as_tuple_pass.erl
record_as_tuple_pass.erl: The function call on line 22 at column 5 is expected to have type [#reader{}] but it has type [tuple()]

    Ref = 1,
    Readers = [#reader{monitor_ref = 1}],
    keydelete(Ref, #reader.monitor_ref, Readers).
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[zuiderkwast]

@erszcz You mean we should use this spec?

-spec keydelete(Key, N, [A]) -> [A] when A :: tuple().

Currently (since #122), we simply convert such specs to keydelete(Key, N, [tuple()]) -> [tuple()]. Perhaps we can work around that by omitting the constraint:

-spec keydelete(Key, N, [A]) -> [A].

[erszcz]

@zuiderkwast

Perhaps we can work around that by omitting the constraint:

-spec keydelete(Key, N, [A]) -> [A].

I tried, but the constraint A :: tuple() is not picked up from the use of element() in the impl of keydelete3, so there's nothing protecting us from calling keydelete(key, 1, [not_a_tuple]). It's not a surprise, since keydelete3 doesn't have a spec at all so any() is assumed for all its args. I don't think we reliably can or should override the spec for keydelete3, given it's an unexported function in lists :|