Discussion:
[erlang-questions] Temporarily violating record type constraints annoys dialyzer
Roger Lipscombe
2018-11-12 10:58:36 UTC
Permalink
I've got a record defined as follows (e.g., and very simplified):

-record widget {
id :: binary(),
name :: binary(),
size :: integer()
}.

I parse that from (e.g.) a proplist:

parse_widget(Props) ->
parse_widget(Props, #widget{}).

parse_widget([{name, Name} | Rest], Acc) ->
parse_widget(Rest, Acc#widget { name = Name });
% etc.

Dialyzer isn't happy that my fields are initially set to 'undefined',
even though this only occurs during the parsing step, and isn't a big
deal.

What can I do to deal with this? Either re-structuring my code or
persuading dialyzer that it's OK would both be acceptable.
Brujo Benavides
2018-11-12 11:11:31 UTC
Permalink
Hi Roger,

According to how you use your record, its spec should actually be


-record widget {
id :: undefined | binary(),
name :: undefined | binary(),
size :: undefined | integer()
}.

That will silence dialyzer or, putting it in the right perspective: That will be a spec that matches how your code actually treats that record.
And that’s because: What happens if Props doesn’t have a tuple for name?

If not having a {name, Name} tuple in Props is an invalid scenario, you should raise some sort of error.
In that case I would recommend you to keep the record definition as-is, but fill the whole record at once:

parse_widget(Props) ->
#widget{
id = get_value(id, Props),
name = get_value(name, Props),


}.

get_value(Key, Props) ->
{Key, Value} = lists:keyfind(Key, 1, Props),
Value.

That way, get_value/2 will raise an error if a property is missing.

If not having a {name, Name} tuple in Props is a valid scenario but you still don’t want that record property to be undefined, you will need a sane default for it.
In that case, you should amend your record definition as


-record widget {
id = <<>> :: binary(),
name = <<>> :: binary(),
size = 0 :: integer()
}.

Hope this helps :)

Brujo Benavides <http://about.me/elbrujohalcon>
Post by Roger Lipscombe
-record widget {
id :: binary(),
name :: binary(),
size :: integer()
}.
parse_widget(Props) ->
parse_widget(Props, #widget{}).
parse_widget([{name, Name} | Rest], Acc) ->
parse_widget(Rest, Acc#widget { name = Name });
% etc.
Dialyzer isn't happy that my fields are initially set to 'undefined',
even though this only occurs during the parsing step, and isn't a big
deal.
What can I do to deal with this? Either re-structuring my code or
persuading dialyzer that it's OK would both be acceptable.
_______________________________________________
erlang-questions mailing list
http://erlang.org/mailman/listinfo/erlang-questions
Roger Lipscombe
2018-11-12 11:35:02 UTC
Permalink
Post by Brujo Benavides
Hi Roger,
According to how you use your record, its spec should actually be…
-record widget {
id :: undefined | binary(),
name :: undefined | binary(),
size :: undefined | integer()
}.
That will silence dialyzer or, putting it in the right perspective: That
will be a spec that matches how your code actually treats that record.
And that’s because: What happens if Props doesn’t have a tuple for name?
I figured someone would say that. The answer is that Props *always*
has a tuple for name -- it was written to a data store with that
invariant. But I can see that it's hard to tell dialyzer this.
Post by Brujo Benavides
If not having a {name, Name} tuple in Props is an invalid scenario, you
should raise some sort of error.
In that case I would recommend you to keep the record definition as-is, but
parse_widget(Props) ->
#widget{
id = get_value(id, Props),
name = get_value(name, Props),

}.
get_value(Key, Props) ->
{Key, Value} = lists:keyfind(Key, 1, Props),
Value.
I was kinda hoping to avoid that -- I've got a vague feeling that we
did this for performance reasons. Of course, those assumptions are
probably no longer valid and need re-checking.
Post by Brujo Benavides
Hope this helps :)
Only in the "you're doing it wrong" sense :)
Brujo Benavides
2018-11-12 11:37:55 UTC
Permalink
And what about adding sane defaults to the fields, then?

Brujo Benavides <http://about.me/elbrujohalcon>
Post by Roger Lipscombe
Post by Brujo Benavides
Hi Roger,
According to how you use your record, its spec should actually be

-record widget {
id :: undefined | binary(),
name :: undefined | binary(),
size :: undefined | integer()
}.
That will silence dialyzer or, putting it in the right perspective: That
will be a spec that matches how your code actually treats that record.
And that’s because: What happens if Props doesn’t have a tuple for name?
I figured someone would say that. The answer is that Props *always*
has a tuple for name -- it was written to a data store with that
invariant. But I can see that it's hard to tell dialyzer this.
Post by Brujo Benavides
If not having a {name, Name} tuple in Props is an invalid scenario, you
should raise some sort of error.
In that case I would recommend you to keep the record definition as-is, but
parse_widget(Props) ->
#widget{
id = get_value(id, Props),
name = get_value(name, Props),


}.
get_value(Key, Props) ->
{Key, Value} = lists:keyfind(Key, 1, Props),
Value.
I was kinda hoping to avoid that -- I've got a vague feeling that we
did this for performance reasons. Of course, those assumptions are
probably no longer valid and need re-checking.
Post by Brujo Benavides
Hope this helps :)
Only in the "you're doing it wrong" sense :)
Jesper Louis Andersen
2018-11-13 14:49:06 UTC
Permalink
Post by Roger Lipscombe
I was kinda hoping to avoid that -- I've got a vague feeling that we
did this for performance reasons. Of course, those assumptions are
probably no longer valid and need re-checking.
A lists:keyfind on a small list is going to be fast, especially because you
are only reading and thus not putting any GC pressure in there. Your
parsing variant constructs a number of intermediate tuples and I don't
think the Erlang compiler currently detects it can unroll that loop, and
strength reduce the tuple creation so it has higher GC pressure.
Alternatively, put it into a map and then transform the map. It is at most
one write more.

I'd measure this carefully.
z***@zxq9.com
2018-11-12 11:35:42 UTC
Permalink
Post by Roger Lipscombe
What can I do to deal with this? Either re-structuring my code or
persuading dialyzer that it's OK would both be acceptable.
Providing a more complete typespec can work:

-record(widget
{id :: undefined | binary(),
name :: undefined | binary(),
size :: undefined | integer()}.

Or providing typed defaults that you know are bottom types:

-record(widget
{id = <<>> :: binary(),
name = <<>> :: binary(),
size = 0 :: integer()}.


Depending on the rest of the code, sometimes a type definition to mask
the record is an easy way to sidestep a few wierd Dialyzer corner cases:

-type widget() :: #widget{}.

And sometimes cutting straight through with line-by-line code conceals
the problem more directly (which allows you to ignore extra args at a
negligble performance cost:

parse_widget(Props) ->
#widget{id = proplists:get_value(id, Props),
name = proplists:get_value(name, Props),
size = proplists:get_value(size, Props)}.


-Craig
z***@zxq9.com
2018-11-12 11:37:59 UTC
Permalink
Post by Roger Lipscombe
parse_widget(Props) ->
#widget{id = proplists:get_value(id, Props),
name = proplists:get_value(name, Props),
size = proplists:get_value(size, Props)}.
Holy tabs!
Krzysztof Jurewicz
2018-11-12 12:49:09 UTC
Permalink
A similar problem arises if we try to use record syntax to construct generators for property testing:

-module(foo_tests).

-include_lib("triq/include/triq.hrl").

-record(
project,
{name :: binary(),
budget :: non_neg_integer(),
successful :: boolean()}).

prop_successful_projects_are_succesful() ->
?FORALL(
Project,
#project{
name=binary(),
budget=non_neg_integer(),
successful=true},
Project#project.successful).

The tests pass, but Dialyzer complains. To silence it, we could rewrite the property as:

prop_successful_projects_are_succesful() ->
?FORALL(
Project,
?LET(
{Name, Budget},
{binary(), non_neg_integer()},
#project{
name=Name,
budget=Budget,
successful=true}),
Project#project.successful).

This is however more verbose.

We may want to put a generator into a function:

project() ->
#project{
name=binary(),
budget=non_neg_integer(),
successful=boolean()}.

The property may then be written as:

prop_successful_projects_are_succesful() ->
?FORALL(
Project,
(project())#project{successful=true},
Project#project.successful).

But again Dialyzer complains. If we wanted to rewrite project() in a ?LET form, then we wouldn’t be able to write (project())#project{successful=true}, as ?LET results in a different data structure.

The root problem here is that by writing #project{name=binary(), budget=non_neg_integer(), successful=boolean()} we don’t really want to create a project record. Instead, we want to create a tuple with a structure resembling the project record which will then be used to create a concrete project record. Dialyzer however doesn’t know about that.

In this particular case one way to silence Dialyzer is to write the project record as:

-record(
project,
{name :: binary() | triq_dom:domain(binary()),
budget :: non_neg_integer() | triq_dom:domain(non_neg_integer()),
successful :: boolean() | triq_dom:domain(boolean())}).

This is however repetitive and also superfluous in production environment.

There is a parse transform named dynarec ( https://github.com/dieswaytoofast/dynarec ) “that automaticaly generates and exports accessors for all records declared within a module”. Theoretically it could be expanded to generate a function named from_map/2 which would look like this:

from_map(
project,
#{name := Name,
budget := Budget,
successful := Successful}) ->
#project{
name=Name,
budget=Budget,
successful=Successful}.

We could then expand the project/0 generator as:

project(FieldMap) ->
?LET(
ProjectMap,
maps:merge(
#{name => binary(),
budget => non_neg_integer(),
successful => bool()},
FieldMap),
from_map(project, ProjectMap)).

It would allow us to leave the project record in its original form and rewrite the property as:

prop_successful_projects_are_succesful() ->
?FORALL(
Project,
project(#{successful => true}),
Project#project.successful).

The parse_widget/1 function from the original post could be written as:

parse_widget(Props) ->
from_map(widget, maps:from_list(Props)).

Generation of from_map/2 is however currently not implemented in dynarec.
Krzysztof Jurewicz
2018-11-12 22:36:39 UTC
Permalink
Post by Krzysztof Jurewicz
Generation of from_map/2 is however currently not implemented in dynarec.
I’ve implemented generation of from_map/2 and to_map/1; see this pull request: https://github.com/dieswaytoofast/dynarec/pull/7 . It should now be possible to convert records to maps and vice versa without writing boilerplate code.
Roger Lipscombe
2018-11-13 17:49:40 UTC
Permalink
On 12 November 2018 at 22:36, Krzysztof Jurewicz
Post by Krzysztof Jurewicz
Post by Krzysztof Jurewicz
Generation of from_map/2 is however currently not implemented in dynarec.
I’ve implemented generation of from_map/2 and to_map/1; see this pull request: https://github.com/dieswaytoofast/dynarec/pull/7 . It should now be possible to convert records to maps and vice versa without writing boilerplate code.
That's *almost* perfect ;-)

Except that we load the code from JSON, using jiffy, which means that
the keys are binary strings. I had a quick look at doing a PR for
this, but I'm a bit sketchy when it comes to parse transforms, and I
got side tracked.
Krzysztof Jurewicz
2018-11-14 13:54:56 UTC
Permalink
Post by Roger Lipscombe
Post by Krzysztof Jurewicz
I’ve implemented generation of from_map/2 and to_map/1; see this pull request: https://github.com/dieswaytoofast/dynarec/pull/7 . It should now be possible to convert records to maps and vice versa without writing boilerplate code.
That's *almost* perfect ;-)
Except that we load the code from JSON, using jiffy, which means that
the keys are binary strings. I had a quick look at doing a PR for
this, but I'm a bit sketchy when it comes to parse transforms, and I
got side tracked.
A workaround (though likely not most efficient) may be to write the following function:

record_from_binary_proplist(RecordName, Props) ->
BinaryFields = [list_to_binary(atom_to_list(Field)) || Field <- fields(RecordName)],
from_map(
RecordName,
maps:from_list(
[{list_to_atom(binary_to_list(Key)), Value}
|| {Key, Value} <- Props, lists:member(Key, BinaryFields)])).

Alternatively:

record_from_binary_proplist(RecordName, Props) ->
FieldsByBinary = maps:from_list([{list_to_binary(atom_to_list(Field)), Field} || Field <- fields(RecordName)]),
from_map(
RecordName,
lists:foldl(
fun ({Key, Value}, MapAcc) ->
case maps:find(Key, FieldsByBinary) of
{ok, KeyAtom} ->
maps:put(KeyAtom, Value, MapAcc);
error ->
MapAcc
end
end,
#{},
Props)).
Fred Hebert
2018-11-13 12:23:26 UTC
Permalink
...
...
That's interesting. I pretty much never run Dialyzer against test suites
simply because there are cases where I want my tests to trigger failures
and validate in/out of boundary conditions are being checked, sometimes
to know if the right kind of exceptions is raised (or that the right
logs are produced as side-effects)

Every time I purposefully send in data that breaks boundaries and raises
exceptions which end up part of my program's contract even if it isn't
part of its type annotations, Dialyzer is guaranteed to never pass.

One of the small unspoken "rules" or practices I have is to never run
Dialyzer on test code because it never works super well for plenty of
cases, particularly when your tests explicitly try to break things.
Brujo Benavides
2018-11-13 12:31:04 UTC
Permalink
Funny
 I do exactly the opposite thing.

Since many of my projects are libraries and dialyzer is mostly clueless on the correctness of exported function specs when it has no other code using them, I use the tests to let dialyzer know how the functions are meant to be used. That way it can tell me if my specs are wrong or if something I meant to do with those functions is actually not possible.

That’s why, more often than not, instead of `rebar3 dialyzer`, I do `rebar3 as test dialyzer`.

BTW, I don’t think Krzysztof was saying that he run dialyzer against tests, just that (on one hand) tests pass but (on the other hand) when he checked his code with dialyzer, it complained. Mostly because I can see how dialyzer would complain with just his code, and no tests :)

Cheers

Brujo Benavides <http://about.me/elbrujohalcon>
...
...
That's interesting. I pretty much never run Dialyzer against test suites simply because there are cases where I want my tests to trigger failures and validate in/out of boundary conditions are being checked, sometimes to know if the right kind of exceptions is raised (or that the right logs are produced as side-effects)
Every time I purposefully send in data that breaks boundaries and raises exceptions which end up part of my program's contract even if it isn't part of its type annotations, Dialyzer is guaranteed to never pass.
One of the small unspoken "rules" or practices I have is to never run Dialyzer on test code because it never works super well for plenty of cases, particularly when your tests explicitly try to break things.
_______________________________________________
erlang-questions mailing list
http://erlang.org/mailman/listinfo/erlang-questions
Fred Hebert
2018-11-13 12:44:14 UTC
Permalink
Funny… I do exactly the opposite thing.
Since many of my projects are libraries and dialyzer is mostly clueless on the correctness of exported function specs when it has no other code using them, I use the tests to let dialyzer know how the functions are meant to be used. That way it can tell me if my specs are wrong or if something I meant to do with those functions is actually not possible.
That’s why, more often than not, instead of `rebar3 dialyzer`, I do `rebar3 as test dialyzer`.
BTW, I don’t think Krzysztof was saying that he run dialyzer against tests, just that (on one hand) tests pass but (on the other hand) when he checked his code with dialyzer, it complained. Mostly because I can see how dialyzer would complain with just his code, and no tests :)
Cheers
Brujo Benavides <http://about.me/elbrujohalcon>
Do you just not do negative tests or you silence each of them
individually?
Attila Rajmund Nohl
2018-11-13 12:48:39 UTC
Permalink
Fred Hebert <***@ferd.ca> ezt írta (időpont: 2018. nov. 13., K, 13:44):
[...]
Post by Fred Hebert
Do you just not do negative tests or you silence each of them
individually?
If the library returns {error, Reason}, not simply crashes, then
dialyzer should not complain.
Brujo Benavides
2018-11-13 12:49:17 UTC
Permalink
I generally tend to follow the rule that people (users of my libs, in particular) should use dialyzer themselves.
So, for instance, if the the spec says this function should be called with a positive_integer(), I don’t test it with 0.
If anybody tries to use the function with the wrong input
 something that will trigger a warning from dialyzer
 they should detect it when running dialyzer on their code.

So, no
 I don’t do negative tests, in that sense.

On the other hand, if I have a function like


my_fun(X) when X > 0 -> work_with(X);
my_fun(X) -> error({invalid_input, X}).

I don’t spec it as

-spec my_fun(pos_integer()) -> 
.

I spec it as

-spec my_fun(pos_integer()) -> 

; my_fun(X) -> no_return().

(or something similar, I typed that off-the-top-of-my-head)

Brujo Benavides <http://about.me/elbrujohalcon>
Post by Brujo Benavides
Funny
 I do exactly the opposite thing.
Since many of my projects are libraries and dialyzer is mostly clueless on the correctness of exported function specs when it has no other code using them, I use the tests to let dialyzer know how the functions are meant to be used. That way it can tell me if my specs are wrong or if something I meant to do with those functions is actually not possible.
That’s why, more often than not, instead of `rebar3 dialyzer`, I do `rebar3 as test dialyzer`.
BTW, I don’t think Krzysztof was saying that he run dialyzer against tests, just that (on one hand) tests pass but (on the other hand) when he checked his code with dialyzer, it complained. Mostly because I can see how dialyzer would complain with just his code, and no tests :)
Cheers
Brujo Benavides <http://about.me/elbrujohalcon>
Do you just not do negative tests or you silence each of them individually?
Brujo Benavides
2018-11-13 12:52:27 UTC
Permalink
Forgot to say that, in the case of my_fun, I do test it with stuff like 0 or -1 or something_that_is_not_even_an_integer
 But dialyzer doesn’t complain, since the spec allows it.

(Sorry for the double-emailing)

Brujo Benavides <http://about.me/elbrujohalcon>
Post by Brujo Benavides
I generally tend to follow the rule that people (users of my libs, in particular) should use dialyzer themselves.
So, for instance, if the the spec says this function should be called with a positive_integer(), I don’t test it with 0.
If anybody tries to use the function with the wrong input
 something that will trigger a warning from dialyzer
 they should detect it when running dialyzer on their code.
So, no
 I don’t do negative tests, in that sense.
On the other hand, if I have a function like

my_fun(X) when X > 0 -> work_with(X);
my_fun(X) -> error({invalid_input, X}).
I don’t spec it as

-spec my_fun(pos_integer()) -> 
.
I spec it as

-spec my_fun(pos_integer()) -> 

; my_fun(X) -> no_return().
(or something similar, I typed that off-the-top-of-my-head)
Brujo Benavides <http://about.me/elbrujohalcon>
Post by Brujo Benavides
Funny
 I do exactly the opposite thing.
Since many of my projects are libraries and dialyzer is mostly clueless on the correctness of exported function specs when it has no other code using them, I use the tests to let dialyzer know how the functions are meant to be used. That way it can tell me if my specs are wrong or if something I meant to do with those functions is actually not possible.
That’s why, more often than not, instead of `rebar3 dialyzer`, I do `rebar3 as test dialyzer`.
BTW, I don’t think Krzysztof was saying that he run dialyzer against tests, just that (on one hand) tests pass but (on the other hand) when he checked his code with dialyzer, it complained. Mostly because I can see how dialyzer would complain with just his code, and no tests :)
Cheers
Brujo Benavides <http://about.me/elbrujohalcon <http://about.me/elbrujohalcon>>
Do you just not do negative tests or you silence each of them individually?
Loïc Hoguin
2018-11-13 12:59:19 UTC
Permalink
Funny… I do exactly the opposite thing.
Since many of my projects are libraries and dialyzer is mostly
clueless on the correctness of exported function specs when it has no
other code using them, I use the tests to let dialyzer know how the
functions are meant to be used. That way it can tell me if my specs
are wrong or if something I meant to do with those functions is
actually not possible.
That’s why, more often than not, instead of `rebar3 dialyzer`, I do
`rebar3 as test dialyzer`.
BTW, I don’t think Krzysztof was saying that he run dialyzer against
tests, just that (on one hand) tests pass but (on the other hand) when
he checked his code with dialyzer, it complained. Mostly because I can
see how dialyzer would complain with just his code, and no tests :)
Cheers
Brujo Benavides <http://about.me/elbrujohalcon>
Do you just not do negative tests or you silence each of them individually?
I personally silence both Dialyzer warnings in expected test failures
(for example Cowboy handler crashes) and any error/crash logs that would
be produced, if any. I've just a few weeks ago enabled Dialyzer against
the Cowboy tests by default (whereas I was running it manually from time
to time before) and this helped me fix a few bad types.

Sometimes Dialyzer needs to be silenced via disabling a warning on a
specific function, sometimes adding a spec with no_return() is enough.
Either way you can't really mix positive and negative tests in the same
test function.

Cheers,
--
Loïc Hoguin
https://ninenines.eu
Krzysztof Jurewicz
2018-11-13 19:57:57 UTC
Permalink
BTW, I don’t think Krzysztof was saying that he run dialyzer against tests, just that (on one hand) tests pass but (on the other hand) when he checked his code with dialyzer, it complained. Mostly because I can see how dialyzer would complain with just his code, and no tests :)
Actually I do run Dialyzer against tests, not only occasionally but routinely. Test code is also code and it can benefit from being checked by Dialyzer. Here is how it looks in the Ercoin repository (ABCI server):

• TEST_DIR is added to DIALYZER_DIRS: https://gitlab.com/Ercoin/ercoin/blob/0c6cedda6bdae870a70655524e008a80d1c1f170/Makefile#L46 . Therefore every invocation of make check and make dialyze (and also CI) covers test code.
• If ABCI client (Tendermint) sends malformed data, then it’s a bug on their side, so we can avoid blame if we crash.
• If end user sends invalid data (via ABCI client), then it is supported and we return a relevant error code. It is also tested; for example, there is a lengthy generator for creating invalid transaction binaries: https://gitlab.com/Ercoin/ercoin/blob/0c6cedda6bdae870a70655524e008a80d1c1f170/test/ercoin_tx_gen.erl#L266 .
• No throw expressions are used. If we parse data from a non-trustworthy data source and there is a possibility of early exit, nested case expressions or monads may be employed (though the latter not without issues, see https://github.com/fogfish/datum/issues/54 ).
Dániel Szoboszlay
2018-12-07 11:34:52 UTC
Permalink
Hi,

I'm a bit late to this party, but I have a suggestion I haven't seen coming
up in the thread. So maybe it could be still useful for you or someone else:

Consider moving type specs out from your record definition completely! Use
a custom type instead:

-record(widget, {id, name, size}).
-type widget() :: #widget{id :: binary(), name :: binary(), size ::
binary()}.

This way you can explicitly tell in the function specs whether you're
dealing with a properly typed widget() or not. You may even define a
different type for the parser, and have Dialyzer check against that:

-type partially_parsed_widget() :: #widget{id :: binary()|undefined, name
:: binary()|undefined, size :: binary()|undefined}.
-spec parse_widget(proplists:proplist()) -> widget().
-spec parse_widget(proplists:proplist(), partially_parsed_widget()) ->
widget().

There are lots of cases when you want to use a record's structure with
different type constraints than the "usual" use of that record. Generating
property based tests and negative tests were already mentioned. But using
records in match specifications is also very common. And in case of some
simple records you may also use tuples that are not meant to be a record,
but look like one (consider someone defining -record(error, {err_no ::
integer(), msg :: string()}). for example).

An additional benefit of separating record definitions from type specs is
that Dialyzer would actually generate much more useful error messages on
type violations. With types in records you may get something like this:

foo.erl:10: Function test/0 has no local return
foo.erl:11: Record construction #foo{bar::0} violates the declared type of
field bar::pos_integer()

The problem is that the "has no local return" error is poisonous: it will
propagate to all other functions calling foo:test/0, and from there to
their callers etc. You'll get a ton of error messages and it will be very
hard to figure out the root cause.

On the other hand when using a separate foo() type the error message would
become:

foo.erl:7: Invalid type specification for function foo:test/0. The success
typing is () -> #foo{bar::0}

This error will not propagate and will be easy to debug.

Just my two cents. Cheers,

Daniel
Post by Roger Lipscombe
-record widget {
id :: binary(),
name :: binary(),
size :: integer()
}.
parse_widget(Props) ->
parse_widget(Props, #widget{}).
parse_widget([{name, Name} | Rest], Acc) ->
parse_widget(Rest, Acc#widget { name = Name });
% etc.
Dialyzer isn't happy that my fields are initially set to 'undefined',
even though this only occurs during the parsing step, and isn't a big
deal.
What can I do to deal with this? Either re-structuring my code or
persuading dialyzer that it's OK would both be acceptable.
_______________________________________________
erlang-questions mailing list
http://erlang.org/mailman/listinfo/erlang-questions
Boroska András
2018-12-07 14:04:23 UTC
Permalink
Hi,

When defining records the way Daniel suggests dialyzer seems to find
problems with record field constraints better. I find it helpful and
changed our record definitions everywhere.

On the other hand when 'warn_untyped_record' compiler option is turned on,
it warns us that "record foo has field(s) without type information".
(OTP-21.1.3) Shall I open a bug report/feature request on dialyzer?

Andras
Boroska András
2018-12-07 14:06:56 UTC
Permalink
Sorry, I meant bug report on the _compiler_ (not on dialyzer).
Post by Boroska András
Hi,
When defining records the way Daniel suggests dialyzer seems to find
problems with record field constraints better. I find it helpful and
changed our record definitions everywhere.
On the other hand when 'warn_untyped_record' compiler option is turned on,
it warns us that "record foo has field(s) without type information".
(OTP-21.1.3) Shall I open a bug report/feature request on dialyzer?
Andras
Kostis Sagonas
2018-12-07 14:39:18 UTC
Permalink
Post by Boroska András
Sorry, I meant bug report on the _compiler_ (not on dialyzer).
Hi,
When defining records the way Daniel suggests dialyzer seems to find
problems with record field constraints better. I find it helpful and
changed our record definitions everywhere.
On the other hand when 'warn_untyped_record' compiler option is
turned on, it warns us that "record foo has field(s) without type
information". (OTP-21.1.3) Shall I open a bug report/feature request
on dialyzer?
If, for whatever reason, you find that having records without types
works better for your setting / code base, what's the reason why you
also set the `warn_untyped_record' compiler option?

It's off by default, isn't it? (And its name explicitly suggests that
it warns when you have record definitions without types, doesn't it?)

Kostis
Dániel Szoboszlay
2018-12-07 15:05:10 UTC
Permalink
I agree that the warn_untyped_record warning does exactly what its name
suggests.

However, when working with records without types, a different warning would
be useful, something that could be called maybe warn_untyped_record_in_type and
what would give you a warning when a -type or -spec declaration contains a
record without type specification for all of its fields.

For example:
-record(foo, {a :: integer(), b}).
-type a() :: #foo{b :: boolean()}. % this is OK, both fields of #foo have a
type
-type b() :: #foo{}. % this is wrong, #foo.b has no explicit
type

Daniel
Post by Kostis Sagonas
Post by Boroska András
Sorry, I meant bug report on the _compiler_ (not on dialyzer).
Hi,
When defining records the way Daniel suggests dialyzer seems to find
problems with record field constraints better. I find it helpful and
changed our record definitions everywhere.
On the other hand when 'warn_untyped_record' compiler option is
turned on, it warns us that "record foo has field(s) without type
information". (OTP-21.1.3) Shall I open a bug report/feature request
on dialyzer?
If, for whatever reason, you find that having records without types
works better for your setting / code base, what's the reason why you
also set the `warn_untyped_record' compiler option?
It's off by default, isn't it? (And its name explicitly suggests that
it warns when you have record definitions without types, doesn't it?)
Kostis
_______________________________________________
erlang-questions mailing list
http://erlang.org/mailman/listinfo/erlang-questions
Kostis Sagonas
2018-12-07 21:17:07 UTC
Permalink
Post by Dániel Szoboszlay
I agree that the warn_untyped_record warning does exactly what its name
suggests.
However, when working with records without types, a different warning
would be useful, something that could be called maybe
warn_untyped_record_in_type and what would give you a warning when a
-type or -spec declaration contains a record without type specification
for all of its fields.
Please feel free to provide a pull request that implements this. (Or
whatever else you think should be appropriate for the approach to typing
-- and at the same time not typing -- records you advocate.)

Kostis

Loading...