Erlang Mailing Lists

Author Message

<  Erlang patches mailing list  ~  Avoiding false Dialyzer warnings for NIFs

Guest
Posted: Fri May 21, 2010 2:56 pm Reply with quote
Guest
I have implemented a new erlang:nif_error/1 BIF that can
be used in the stub for NIFs to avoid false Dialyzer
warnings:

http://github.com/bjorng/otp/commit/826c9574776ae931917e55d441ec0cfcb0acaf84

In a second commit, I have modified the crypto
module to use erlang:nif_error/1.

I have run the crypto test suite, but I have not yet
verified that the false Dialyzer warnings disappear.

Feedback welcome.


The branch can be fetched like this:

git fetch git://github.com/bjorng/otp.git bg/nif_error

--
Björn Gustavsson, Erlang/OTP, Ericsson AB

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist
Guest
Posted: Fri May 21, 2010 2:56 pm Reply with quote
Guest
I have implemented a new erlang:nif_error/1 BIF that can
be used in the stub for NIFs to avoid false Dialyzer
warnings:

http://github.com/bjorng/otp/commit/826c9574776ae931917e55d441ec0cfcb0acaf84

In a second commit, I have modified the crypto
module to use erlang:nif_error/1.

I have run the crypto test suite, but I have not yet
verified that the false Dialyzer warnings disappear.

Feedback welcome.


The branch can be fetched like this:

git fetch git://github.com/bjorng/otp.git bg/nif_error

--
Björn Gustavsson, Erlang/OTP, Ericsson AB

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist
Guest
Posted: Mon May 24, 2010 8:56 am Reply with quote
Guest
Björn Gustavsson wrote:
> I have implemented a new erlang:nif_error/1 BIF that can
> be used in the stub for NIFs to avoid false Dialyzer
> warnings:
>
> http://github.com/bjorng/otp/commit/826c9574776ae931917e55d441ec0cfcb0acaf84
>
> In a second commit, I have modified the crypto
> module to use erlang:nif_error/1.
>
> I have run the crypto test suite, but I have not yet
> verified that the false Dialyzer warnings disappear.
>
> Feedback welcome.

Bjorn,

I've verified that your patch is OK and does what you want it to as far
as dialyzer is concerned.

However, a slightly better addition to the type/4 function of
erl_bif_types is the following:

=========================================================================
type(erlang, nif_error, 1, _) ->
t_unit(); % this BIF and the next one are stubs for NIFs and never return
type(erlang, nif_error, 2, Xs) ->
strict(arg_types(erlang, nif_error, 2), Xs, fun (_) -> t_unit() end);
=========================================================================

I suggest you use it. Make sure you also add t_unit/0 at the end of the
list of imported functions from erl_types (i.e., immediately after
t_tuple_subtypes/1).


FYI, the unit() type is an internal type somewhere between any() and
none(). It's main purpose is to be used for functions that do not
return e.g. because they go into a loop. However, unlike none(), this
return type can be overwritten by a spec that says that the function
returns some other type.

Also, the strict(...) call in the second BIF will make dialyzer catch
accidental wrong uses of this BIF (those where the second argument to
erlang:nif_error/2 is something other than a list). Obviously, the BIF
with only one argument does not need this check since the type of its
argument is any().


For your fix to be complete, I suggest you also add -specs for all
crypto functions that are implemented as NIFs. With this patch, these
can be added directly to the crypto.erl file and dialyzer will handle
them properly.

Kostis

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist
Guest
Posted: Mon May 24, 2010 8:56 am Reply with quote
Guest
Björn Gustavsson wrote:
> I have implemented a new erlang:nif_error/1 BIF that can
> be used in the stub for NIFs to avoid false Dialyzer
> warnings:
>
> http://github.com/bjorng/otp/commit/826c9574776ae931917e55d441ec0cfcb0acaf84
>
> In a second commit, I have modified the crypto
> module to use erlang:nif_error/1.
>
> I have run the crypto test suite, but I have not yet
> verified that the false Dialyzer warnings disappear.
>
> Feedback welcome.

Bjorn,

I've verified that your patch is OK and does what you want it to as far
as dialyzer is concerned.

However, a slightly better addition to the type/4 function of
erl_bif_types is the following:

=========================================================================
type(erlang, nif_error, 1, _) ->
t_unit(); % this BIF and the next one are stubs for NIFs and never return
type(erlang, nif_error, 2, Xs) ->
strict(arg_types(erlang, nif_error, 2), Xs, fun (_) -> t_unit() end);
=========================================================================

I suggest you use it. Make sure you also add t_unit/0 at the end of the
list of imported functions from erl_types (i.e., immediately after
t_tuple_subtypes/1).


FYI, the unit() type is an internal type somewhere between any() and
none(). It's main purpose is to be used for functions that do not
return e.g. because they go into a loop. However, unlike none(), this
return type can be overwritten by a spec that says that the function
returns some other type.

Also, the strict(...) call in the second BIF will make dialyzer catch
accidental wrong uses of this BIF (those where the second argument to
erlang:nif_error/2 is something other than a list). Obviously, the BIF
with only one argument does not need this check since the type of its
argument is any().


For your fix to be complete, I suggest you also add -specs for all
crypto functions that are implemented as NIFs. With this patch, these
can be added directly to the crypto.erl file and dialyzer will handle
them properly.

Kostis

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist
Guest
Posted: Mon May 24, 2010 12:11 pm Reply with quote
Guest
Thanks for the feedback.

2010/5/24 Kostis Sagonas <kostis@cs.ntua.gr>:

>
> =========================================================================
> type(erlang, nif_error, 1, _) ->
> t_unit(); % this BIF and the next one are stubs for NIFs and never
> return
> type(erlang, nif_error, 2, Xs) ->
> strict(arg_types(erlang, nif_error, 2), Xs, fun (_) -> t_unit() end);
> =========================================================================

OK. I have incorporated your change and revised the commit message.

One question, though. Does this mean that you *must* write a type
spec for functions that call erlang:nif_error/1,2?

> For your fix to be complete, I suggest you also add -specs for all crypto
> functions that are implemented as NIFs. With this patch, these can be added
> directly to the crypto.erl file and dialyzer will handle them properly.

Yes, will do that in the next iteration (probably tomorrow).

The (updated) branch can be fetched like this:

git fetch git://github.com/bjorng/otp.git bg/nif_error


--
Björn Gustavsson, Erlang/OTP, Ericsson AB

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist
Guest
Posted: Mon May 24, 2010 12:11 pm Reply with quote
Guest
Thanks for the feedback.

2010/5/24 Kostis Sagonas <kostis@cs.ntua.gr>:

>
> =========================================================================
> type(erlang, nif_error, 1, _) ->
> t_unit(); % this BIF and the next one are stubs for NIFs and never
> return
> type(erlang, nif_error, 2, Xs) ->
> strict(arg_types(erlang, nif_error, 2), Xs, fun (_) -> t_unit() end);
> =========================================================================

OK. I have incorporated your change and revised the commit message.

One question, though. Does this mean that you *must* write a type
spec for functions that call erlang:nif_error/1,2?

> For your fix to be complete, I suggest you also add -specs for all crypto
> functions that are implemented as NIFs. With this patch, these can be added
> directly to the crypto.erl file and dialyzer will handle them properly.

Yes, will do that in the next iteration (probably tomorrow).

The (updated) branch can be fetched like this:

git fetch git://github.com/bjorng/otp.git bg/nif_error


--
Björn Gustavsson, Erlang/OTP, Ericsson AB

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist
Guest
Posted: Mon May 24, 2010 12:46 pm Reply with quote
Guest
Björn Gustavsson wrote:
> Thanks for the feedback.
>
> 2010/5/24 Kostis Sagonas <kostis@cs.ntua.gr>:
>
>> =========================================================================
>> type(erlang, nif_error, 1, _) ->
>> t_unit(); % this BIF and the next one are stubs for NIFs and never
>> return
>> type(erlang, nif_error, 2, Xs) ->
>> strict(arg_types(erlang, nif_error, 2), Xs, fun (_) -> t_unit() end);
>> =========================================================================
>
> OK. I have incorporated your change and revised the commit message.
>
> One question, though. Does this mean that you *must* write a type
> spec for functions that call erlang:nif_error/1,2?

No, there is no such obligation. Things will work smoothly even without
any specs.

Still, specs are recommended because they will give type information to
code that uses these NIFs.

Kostis

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist
Guest
Posted: Mon May 24, 2010 12:47 pm Reply with quote
Guest
Björn Gustavsson wrote:
> Thanks for the feedback.
>
> 2010/5/24 Kostis Sagonas <kostis@cs.ntua.gr>:
>
>> =========================================================================
>> type(erlang, nif_error, 1, _) ->
>> t_unit(); % this BIF and the next one are stubs for NIFs and never
>> return
>> type(erlang, nif_error, 2, Xs) ->
>> strict(arg_types(erlang, nif_error, 2), Xs, fun (_) -> t_unit() end);
>> =========================================================================
>
> OK. I have incorporated your change and revised the commit message.
>
> One question, though. Does this mean that you *must* write a type
> spec for functions that call erlang:nif_error/1,2?

No, there is no such obligation. Things will work smoothly even without
any specs.

Still, specs are recommended because they will give type information to
code that uses these NIFs.

Kostis

________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org

Post received from mailinglist

Display posts from previous:  

All times are GMT
Page 1 of 1
This forum is locked: you cannot post, reply to, or edit topics.

Jump to:  

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You cannot attach files in this forum
You cannot download files in this forum