Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dialyzer should consider the use for an improper list #5937

Closed
dotsimon opened this issue Apr 27, 2022 · 5 comments
Closed

dialyzer should consider the use for an improper list #5937

dotsimon opened this issue Apr 27, 2022 · 5 comments
Assignees
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM

Comments

@dotsimon
Copy link
Contributor

Describe the bug
Dialyzer produces an improper list warning even when that list is constructed for a function call that specs an improper list .e.g. iodata()

Consider erlang:port_control/3

-spec port_control(Port, Operation, Data) -> iodata() | binary() when
      Port :: port() | atom(),
      Operation :: integer(),
      Data :: iodata().

And this code snippet:

cmd(P, flush) ->
    port_control(P, ?FLUSH, [0 | <<1:32/native>>]);

Causes this dialyzer output:
x.erl:10: Cons will produce an improper list since its 2nd argument is <<_:32>>

To Reproduce
Put the example above into a module and dialyze

Expected behaviour
If the success typing for a constructed list is an improper list - and especially iodata() - then no warning should be emitted.

Affected versions
All?

@dotsimon dotsimon added the bug Issue is reported as a bug label Apr 27, 2022
@jhogberg jhogberg added the team:VM Assigned to OTP team VM label Apr 28, 2022
@jhogberg
Copy link
Contributor

Thanks for your report!

This warning is not for the use of, but rather the construction of improper lists, which the original authors considered awful. We're leaning towards disabling these warnings by default in a future version, but for now you can pass -Wno_improper_lists to suppress it.

@dgud
Copy link
Contributor

dgud commented Apr 28, 2022

Or suppress it:
-dialyzer({no_improper_lists, [stack/2, length_b/3]}).

@dotsimon
Copy link
Contributor Author

Ok, the warning being about construction of an improper list makes a subtle difference.
So then maybe this should be a feature request rather than PR because I still think that given dialyzer tracks types especially within a module, that it could be smarter about this.
I personally don't know that disabling all improper list warning would be a good idea. I would assume most lists are used as lists (eg. could be used as an argument to lists:* or in a comprehension) and therefore improper lists should be viewed with some suspicion.
Or is this now an "old fashioned" view - I notice the OTP team made a design decision that gen tags could be an improper list!

@dgud Yes! That is what I have been using to make these go away.

@jhogberg
Copy link
Contributor

jhogberg commented May 3, 2022

I personally don't know that disabling all improper list warning would be a good idea. I would assume most lists are used as lists (eg. could be used as an argument to lists:* or in a comprehension) and therefore improper lists should be viewed with some suspicion.

no_improper_lists only affects the construction warnings, so you'll still get a type warning for passing [foo | bar] to a list function, for example.

Or is this now an "old fashioned" view - I notice the OTP team made a design decision that gen tags could be an improper list!

That's just the opinion of the original authors, at least as it were back then. Many of us at OTP disagree with the warning because it throws the baby out with the bathwater, there's nothing wrong with using a single cons cell as a more compact alternative to 2-tuples in internal code like gen.

@dotsimon
Copy link
Contributor Author

dotsimon commented May 3, 2022

Thanks for the additional context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

4 participants