gleam icon indicating copy to clipboard operation
gleam copied to clipboard

Add qualifiers for type variables in generated Erlang typespecs

Open dvic opened this issue 1 year ago • 16 comments

Starting from OTP 26 erlang/otp#6915, warnings are emitted for unbound type variables, for example:

 -spec try_call_timeout_test() -> {ok, GGA}

We need to replace this unbound type variable with term() so that no warning is emitted.

originally reported in gleam-lang/erlang#46

dvic avatar May 31 '24 08:05 dvic

@lpil isn't it better to use a bound instead of term directly? Let's say this was the function (from the OTP PR mentioned above):

-spec foo(atom()) -> {ok, X} | {error, X}

Then instead of

-spec foo(atom()) -> {ok, term()} | {error, term()}

I think it's better to have

-spec foo(atom()) -> {ok, X} | {error, X} when X :: term()

In this way, the typespec correctly signals that the type should be the same in the two branches?

dvic avatar May 31 '24 08:05 dvic

@dvic The example you give there doesn't need to be changed, it's only when a parameter is used once that it is invalid. What you've suggested it be changed to is also the same and is redundant as term() offers no constraint at all.

Thank you @dvic !

lpil avatar May 31 '24 09:05 lpil

@dvic The example you give there doesn't need to be changed, it's only when a parameter is used once that it is invalid.

Thank you @dvic !

Are you sure? I just double checked this locally with the example from erlang/otp#6915:

-module(foo).

-export([foo/1]).

-spec foo(atom()) -> {ok, X} | {error, X}.
foo(X) ->
    {ok, X}.
❯ erlc foo.erl
foo.erl:5:27: Warning: type variable 'X' is only used once (is unbound)
%    5| -spec foo(atom()) -> {ok, X} | {error, X}.
%     |                           ^

While

-module(foo).

-export([foo/1]).

-spec foo(atom()) -> {ok, X} | {error, X} when X :: term().
foo(X) ->
    {ok, X}.

does not emit an warning (this is on OTP 26.2.5).

dvic avatar May 31 '24 09:05 dvic

Huh. OK I don't understand what's happening here. It says "once" but it's clearly used twice. Is this a bug in erlc?

lpil avatar May 31 '24 09:05 lpil

Huh. OK I don't understand what's happening here. It says "once" but it's clearly used twice. Is this a bug in erlc?

Yeah the message is a bit confusing, I think it's more about the (is unbound) part. Apparently for the compiler X is being used twice in the following example, once in the return type and once in the type guard.

-spec foo(atom()) -> {ok, X} | {error, X} when X :: term().

dvic avatar May 31 '24 10:05 dvic

I've opened an issue with Erlang/OTP to find out if this warning is emitted incorrectly, or if the text is misleading.

lpil avatar May 31 '24 10:05 lpil

@lpil I may have confused you with the following comment image

OTP 26.2.5 also emits a warning without the type guard, it's because of the added type guard that no warning is emitted.

I posted a clarification on the issue as well.

dvic avatar May 31 '24 10:05 dvic

Thank you.

This is all very confusing to me. I guess we add seemingly redundant qualifiers to everything!

lpil avatar May 31 '24 15:05 lpil

Thank you.

This is all very confusing to me.

Yeah it is, I guess we're all spoiled by the errors in Gleam ;)

dvic avatar May 31 '24 19:05 dvic

Looks like I'm totally wrong about how they work. https://github.com/erlang/otp/issues/8533

lpil avatar Jun 05 '24 10:06 lpil

I saw the conversation :) So for now we only add X :: term() when a type parameter is not used in the input? That should cover it, right?

dvic avatar Jun 05 '24 10:06 dvic

I think so? It's still incorrect though unfortunately.

lpil avatar Jun 05 '24 10:06 lpil

I think so? It's still incorrect though unfortunately.

Yeah, it's the best we can do for now.

I was busy with playing around with https://github.com/gleam-lang/erlang/pull/45 but I can pick this one up, wanted to get familiar with the Rust codebase anyways :)

dvic avatar Jun 05 '24 10:06 dvic

@lpil any idea how I can trigger the issue in a isolated test? I tried this but any() is generated:

image

It makes me think: isn't that what should have been generated in the example in this issue?

dvic avatar Jun 06 '24 20:06 dvic

Never mind, I have it reproduced now with:

image image

dvic avatar Jun 06 '24 20:06 dvic

PR is submitted!

dvic avatar Jun 06 '24 21:06 dvic