Dialyzer: Cons will produce an improper list since its 2nd argument is none()

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Jesper Eskilson-2
Hi,

When dialyzer analyzes this program:

-module(foo).
-export([main/0]).
-record(foo, {x :: integer()}).
main() ->
  [ #foo{x = 0},
    #foo{x = false} ].

it says:

foo.erl:7: Function main/0 has no local return
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()

It seems like dialyzer assumes that any cons with second argument not being a list will produce an improper list, but shouldn't it treat "none()" differently?

Is this a bug in dialyzer, or a feature whose usefulness I am unable to grasp?

--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com

Reply | Threaded
Open this post in threaded view
|

Re: Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Kostis Sagonas-2
On 2/14/20 1:44 PM, Jesper Eskilson wrote:

> Hi,
>
> When dialyzer analyzes this program:
>
>
>     -module(foo).
>     -export([main/0]).
>     -record(foo, {x :: integer()}).
>     main() ->
>        [ #foo{x = 0},
>          #foo{x = false} ].
>
>
> it says:
>
>     foo.erl:7: Function main/0 has no local return
>     foo.erl:8: Cons will produce an improper list since its 2nd argument is
>                none()
>     foo.erl:9: Record construction
>                #foo{x :: false} violates the declared type of field x ::
>                integer()
>
>
> It seems like dialyzer assumes that any cons with second argument not
> being a list will produce an improper list, but shouldn't it treat
> "none()" differently?
>
> Is this a bug in dialyzer, or a feature whose usefulness I am unable to
> grasp?

It's a dialyzer feature, which we have purposedly sneaked into the tool
to discover all users who choose to stop reading all warnings that the
tool spits out and are not experienced enough to realize that once they
fix the real culprit, which may not be the first warning that that the
tool produces, the other warnings will also disappear.

Cheers,
Kostis
Reply | Threaded
Open this post in threaded view
|

Re: Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Brujo Benavides-3
In reply to this post by Jesper Eskilson-2
I think what's happening in this case is this…
Dialyzer sees your code (using cons for the list) as…

main() -> [ #foo{x = 0} | [#foo{x = false} | []] ].

It then detects the third warning from your email (namely, that you're using false for something that should be an integer):
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()
Therefore, the type of that second part of the list ([#foo{x = false} | []]) will be none().
And that leads dialyzer to complain with the second warning that you see…
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
Which, in time, produces the first warning…
foo.erl:7: Function main/0 has no local return

Hope this helps. 

On Fri, Feb 14, 2020 at 1:45 PM Jesper Eskilson <[hidden email]> wrote:
Hi,

When dialyzer analyzes this program:

-module(foo).
-export([main/0]).
-record(foo, {x :: integer()}).
main() ->
  [ #foo{x = 0},
    #foo{x = false} ].

it says:

foo.erl:7: Function main/0 has no local return
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()

It seems like dialyzer assumes that any cons with second argument not being a list will produce an improper list, but shouldn't it treat "none()" differently?

Is this a bug in dialyzer, or a feature whose usefulness I am unable to grasp?

--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com



--
Brujo Benavides
about.me/elbrujohalcon
Reply | Threaded
Open this post in threaded view
|

Re: Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Jesper Eskilson-2
In reply to this post by Kostis Sagonas-2
Awesome reply! Thanks, 

On Fri, Feb 14, 2020 at 1:53 PM Kostis Sagonas <[hidden email]> wrote:
On 2/14/20 1:44 PM, Jesper Eskilson wrote:
> Hi,
>
> When dialyzer analyzes this program:
>
>
>     -module(foo).
>     -export([main/0]).
>     -record(foo, {x :: integer()}).
>     main() ->
>        [ #foo{x = 0},
>          #foo{x = false} ].
>
>
> it says:
>
>     foo.erl:7: Function main/0 has no local return
>     foo.erl:8: Cons will produce an improper list since its 2nd argument is
>                none()
>     foo.erl:9: Record construction
>                #foo{x :: false} violates the declared type of field x ::
>                integer()
>
>
> It seems like dialyzer assumes that any cons with second argument not
> being a list will produce an improper list, but shouldn't it treat
> "none()" differently?
>
> Is this a bug in dialyzer, or a feature whose usefulness I am unable to
> grasp?

It's a dialyzer feature, which we have purposedly sneaked into the tool
to discover all users who choose to stop reading all warnings that the
tool spits out and are not experienced enough to realize that once they
fix the real culprit, which may not be the first warning that that the
tool produces, the other warnings will also disappear.

Cheers,
Kostis


--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com

Reply | Threaded
Open this post in threaded view
|

Re: Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Jesper Eskilson-2
In reply to this post by Brujo Benavides-3
Ah, my question wasn't why the error occurs in the first place, but rather an inquiry if dialyzer oughtn't formulate the error a bit differently. :)


On Fri, Feb 14, 2020 at 1:54 PM Fernando Benavides <[hidden email]> wrote:
I think what's happening in this case is this…
Dialyzer sees your code (using cons for the list) as…

main() -> [ #foo{x = 0} | [#foo{x = false} | []] ].

It then detects the third warning from your email (namely, that you're using false for something that should be an integer):
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()
Therefore, the type of that second part of the list ([#foo{x = false} | []]) will be none().
And that leads dialyzer to complain with the second warning that you see…
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
Which, in time, produces the first warning…
foo.erl:7: Function main/0 has no local return

Hope this helps. 

On Fri, Feb 14, 2020 at 1:45 PM Jesper Eskilson <[hidden email]> wrote:
Hi,

When dialyzer analyzes this program:

-module(foo).
-export([main/0]).
-record(foo, {x :: integer()}).
main() ->
  [ #foo{x = 0},
    #foo{x = false} ].

it says:

foo.erl:7: Function main/0 has no local return
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()

It seems like dialyzer assumes that any cons with second argument not being a list will produce an improper list, but shouldn't it treat "none()" differently?

Is this a bug in dialyzer, or a feature whose usefulness I am unable to grasp?

--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com



--
Brujo Benavides
about.me/elbrujohalcon


--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com

Reply | Threaded
Open this post in threaded view
|

Re: Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Dmitry Belyaev-3
In reply to this post by Kostis Sagonas-2
When a tool does something incorrect, and I think everybody agree that first and second warnings are incorrect, those are called bugs, not features.

And when something incorrect is purposefully sneaked in, then one may ask a question what else undesirable may be purposefully added. It could be a routine sending private code to professional human code review to ensure the high quality and absence of bugs.

On 14 February 2020 11:53:23 pm AEDT, Kostis Sagonas <[hidden email]> wrote:
On 2/14/20 1:44 PM, Jesper Eskilson wrote:
Hi,

When dialyzer analyzes this program:


-module(foo).
-export([main/0]).
-record(foo, {x :: integer()}).
main() ->
  [ #foo{x = 0},
    #foo{x = false} ].


it says:

foo.erl:7: Function main/0 has no local return
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()


It seems like dialyzer assumes that any cons with second argument not
being a list will produce an improper list, but shouldn't it treat
"none()" differently?

Is this a bug in dialyzer, or a feature whose usefulness I am unable to
grasp?

It's a dialyzer feature, which we have purposedly sneaked into the tool
to discover all users who choose to stop reading all warnings that the
tool spits out and are not experienced enough to realize that once they
fix the real culprit, which may not be the first warning that that the
tool produces, the other warnings will also disappear.

Cheers,
Kostis

--
Kind regards,
Dmitry Belyaev
Reply | Threaded
Open this post in threaded view
|

Re: Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Jesper Eskilson-2
I'm sure dialyzer is just a frontend to a mail program, and that all it does is to send the code to Kostis for analysis. We're running dialyzer several times a day on a code base in the MLOC ballpark, so I apologise for making Kostis read so much code.

Joke aside, I presume Kostis was being a bit ironic when claiming that this was something sneaked in on purpose.

/Jesper
Reply | Threaded
Open this post in threaded view
|

Re: Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Richard Carlsson-3
In reply to this post by Jesper Eskilson-2
Nice answer, Brujo! I think the only confusion here is the warning about the improper list; without that one, everything should have been clear. The check for improper lists ought to ignore the case when the tail is none(), since it means that the result will also be none() anyway.

        /Richard


Den fre 14 feb. 2020 kl 14:05 skrev Jesper Eskilson <[hidden email]>:
Ah, my question wasn't why the error occurs in the first place, but rather an inquiry if dialyzer oughtn't formulate the error a bit differently. :)


On Fri, Feb 14, 2020 at 1:54 PM Fernando Benavides <[hidden email]> wrote:
I think what's happening in this case is this…
Dialyzer sees your code (using cons for the list) as…

main() -> [ #foo{x = 0} | [#foo{x = false} | []] ].

It then detects the third warning from your email (namely, that you're using false for something that should be an integer):
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()
Therefore, the type of that second part of the list ([#foo{x = false} | []]) will be none().
And that leads dialyzer to complain with the second warning that you see…
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
Which, in time, produces the first warning…
foo.erl:7: Function main/0 has no local return

Hope this helps. 

On Fri, Feb 14, 2020 at 1:45 PM Jesper Eskilson <[hidden email]> wrote:
Hi,

When dialyzer analyzes this program:

-module(foo).
-export([main/0]).
-record(foo, {x :: integer()}).
main() ->
  [ #foo{x = 0},
    #foo{x = false} ].

it says:

foo.erl:7: Function main/0 has no local return
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()

It seems like dialyzer assumes that any cons with second argument not being a list will produce an improper list, but shouldn't it treat "none()" differently?

Is this a bug in dialyzer, or a feature whose usefulness I am unable to grasp?

--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com



--
Brujo Benavides
about.me/elbrujohalcon


--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com

Reply | Threaded
Open this post in threaded view
|

Re: Dialyzer: Cons will produce an improper list since its 2nd argument is none()

Dániel Szoboszlay
I was playing a bit with Jesper's example, and I feel Dialyzer behaves very surprisingly and somewhat inconsistently with reporting this kind of problems. Here's my extended test program to illustrate the issues:

-module(foo).
-export([foo/0, err/0, errs/0, bar/0, bars/0]).
-record(foo, {x :: integer()}).
-record(bar, {x}).
-type bar() :: #bar{x :: integer()}.
foo() ->
  [ #foo{x = 0},
    #foo{x = false} ].
err() ->
  error(badarg).
errs() ->
  [ #foo{x = 0},
    error(badarg) ].
-spec bar() -> bar().
bar() ->
  #bar{x = false}.
-spec bars() -> [bar()].
bars() ->
  [ #bar{x = 0},
    #bar{x = false} ].

And these are the warnings Dialyzer reports:

foo.erl:6: Function foo/0 has no local return
foo.erl:7: Cons will produce an improper list since its 2nd argument is
          none()
foo.erl:8: Record construction
          #foo{x :: 'false'} violates the declared type of field x ::
          integer()
foo.erl:11: Function errs/0 has no local return
foo.erl:14: Invalid type specification for function foo:bar/0. The success typing is
          () -> #bar{x :: 'false'}

I find the following things problematic with these warnings:
  • "Function foo/0 has no local return": what the message says is not true. There are some type spec issues with foo/0, but it won't crash, because the emulator doesn't care about your type specs at runtime.
  • "Cons will produce an improper list": what the message says is not true. If the 2nd argument is none(), that is: evaluating that expression would raise an exception instead of returning a value, cons will not produce anything, the execution will never reach that far.
  • There is no "Function err/0 has no local return" warning, even though err/0 has no local return for real. This also feels very inconsistent with the fact that there is such a warning for the errs/0 function, which does basically the same thing (when considering types).
  • There is no "Cons will produce an improper list" warning for errs/0, even though it is exactly the same situation as in case of foo/0. Dialyzer feels inconsistent here.
  • There is no "Invalid type specification for function foo:bars/0: warning, even though this function clearly violates its spec, and the same problem in bar/0 is properly reported. (Note: this one in particular makes me very sad, because I prefer keeping type specs outside of record declarations precisely to avoid the "Function has no local return" warnings. In my experience Dialyzer produces better warnings this way, but now I found an example where an obvious problem is not detected by Dialyzer due to the field types being declared outside of the record definition.)
  • A minor inconsistency, but some warnings refer to the function in which they occur via its local name within the module (foo/0), while others use the qualified name (foo:bar/0).
Would it be possible to make Dialyzer more consistent and less surprising in these cases?

Cheers,
Daniel

On Fri, 14 Feb 2020 at 15:48, Richard Carlsson <[hidden email]> wrote:
Nice answer, Brujo! I think the only confusion here is the warning about the improper list; without that one, everything should have been clear. The check for improper lists ought to ignore the case when the tail is none(), since it means that the result will also be none() anyway.

        /Richard


Den fre 14 feb. 2020 kl 14:05 skrev Jesper Eskilson <[hidden email]>:
Ah, my question wasn't why the error occurs in the first place, but rather an inquiry if dialyzer oughtn't formulate the error a bit differently. :)


On Fri, Feb 14, 2020 at 1:54 PM Fernando Benavides <[hidden email]> wrote:
I think what's happening in this case is this…
Dialyzer sees your code (using cons for the list) as…

main() -> [ #foo{x = 0} | [#foo{x = false} | []] ].

It then detects the third warning from your email (namely, that you're using false for something that should be an integer):
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()
Therefore, the type of that second part of the list ([#foo{x = false} | []]) will be none().
And that leads dialyzer to complain with the second warning that you see…
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
Which, in time, produces the first warning…
foo.erl:7: Function main/0 has no local return

Hope this helps. 

On Fri, Feb 14, 2020 at 1:45 PM Jesper Eskilson <[hidden email]> wrote:
Hi,

When dialyzer analyzes this program:

-module(foo).
-export([main/0]).
-record(foo, {x :: integer()}).
main() ->
  [ #foo{x = 0},
    #foo{x = false} ].

it says:

foo.erl:7: Function main/0 has no local return
foo.erl:8: Cons will produce an improper list since its 2nd argument is
          none()
foo.erl:9: Record construction
          #foo{x :: false} violates the declared type of field x ::
          integer()

It seems like dialyzer assumes that any cons with second argument not being a list will produce an improper list, but shouldn't it treat "none()" differently?

Is this a bug in dialyzer, or a feature whose usefulness I am unable to grasp?

--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com



--
Brujo Benavides
about.me/elbrujohalcon


--

Jesper Eskilson
Senior Software Engineer
Kred Core
+46-72-855-8421

Klarna Bank AB (publ)
Sveavägen 46, 111 34 Stockholm
Tel: <a href="tel:+46812012000" style="color:rgb(130,130,137)" target="_blank">+46 8 120 120 00
Reg no: 556737-0431
klarna.com