Quantcast

add compiler checked atoms

Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

add compiler checked atoms

Richard Carlsson-5
Sorry, pressed send too soon in the last attempt. Here is the code:

   git fetch git:richcarl/otp.git warn-unknown-atom

Here is a suggested patch to the compiler, that I'm throwing out there
as a request for comments. In any large code base, it can be hard to
maintain atom hygiene - you may have hidden errors due to misspelled
atoms, people may be adding atoms without much thought, and it can be
hard to track which atoms are being used where and for what (e.g., atoms
used for options, error indicators, message tags, callback module and
function names, etc. This allows you to declare your atoms like so:

   -atoms([foo, bar]).

You can have any number of such declarations, and an atom may occur
multiple times and/or in different declarations - it's the union of
known atoms that matters. Note that this declaration is backwards
compatible; the current compiler will accept it as a generic attribute
with no particular meaning.

If you specify the compiler option `warn_unused_atom' when compiling a
module (you can put `-compile([warn_unused_atom]).' in the module if you
want to enable the checking in that module only), you will get a warning
for each atom that has not been explicitly declared. The compiler knows
about standard atoms such as `ok', `true', `false', `undefined' etc.,
and it does not check function and module names in calls, or record
names and fields of records, since these are checked in other ways already.

I don't think this addition warrants an EEP, since it doesn't change the
language as such; it only adds a backwards compatible feature to the
compiler. However, I'd like to get some initial feedback before I bother
to update the documentation and submit it as a full patch.

git:richcarl/otp.git

    /Richard

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

add compiler checked atoms

Richard Carlsson-5
On 02/05/2012 10:15 PM, Richard Carlsson wrote:
> If you specify the compiler option `warn_unused_atom' when compiling a
> module (you can put `-compile([warn_unused_atom]).' in the module if you
> want to enable the checking in that module only), you will get a warning
> for each atom that has not been explicitly declared.

I'm obviously too tired; that should be `warn_unknown_atom'.

     /Richard

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

add compiler checked atoms

Michael Truog-2
In reply to this post by Richard Carlsson-5
This is very cool.  It could help document complex code by making atom usage more transparent within the module, while also preventing errors.  I mention the documentation part, since having the atom declaration gives the opportunity for comments specific to that atom declaration (so you could have atom declarations as logical groupings, similar to what is commonly done for export declarations).  Do atoms used within the spec declarations need to be mentioned within the atom declaration when using this compiler flag?  That might bring in a lot of external module atom dependencies, which might be counter-productive.

On 02/05/2012 01:15 PM, Richard Carlsson wrote:

> Sorry, pressed send too soon in the last attempt. Here is the code:
>
>   git fetch git:richcarl/otp.git warn-unknown-atom
>
> Here is a suggested patch to the compiler, that I'm throwing out there as a request for comments. In any large code base, it can be hard to maintain atom hygiene - you may have hidden errors due to misspelled atoms, people may be adding atoms without much thought, and it can be hard to track which atoms are being used where and for what (e.g., atoms used for options, error indicators, message tags, callback module and function names, etc. This allows you to declare your atoms like so:
>
>   -atoms([foo, bar]).
>
> You can have any number of such declarations, and an atom may occur multiple times and/or in different declarations - it's the union of known atoms that matters. Note that this declaration is backwards compatible; the current compiler will accept it as a generic attribute with no particular meaning.
>
> If you specify the compiler option `warn_unused_atom' when compiling a module (you can put `-compile([warn_unused_atom]).' in the module if you want to enable the checking in that module only), you will get a warning for each atom that has not been explicitly declared. The compiler knows about standard atoms such as `ok', `true', `false', `undefined' etc., and it does not check function and module names in calls, or record names and fields of records, since these are checked in other ways already.
>
> I don't think this addition warrants an EEP, since it doesn't change the language as such; it only adds a backwards compatible feature to the compiler. However, I'd like to get some initial feedback before I bother to update the documentation and submit it as a full patch.
>
> git:richcarl/otp.git
>
>    /Richard
> _______________________________________________
> erlang-patches mailing list
> erlang-patches
> http://erlang.org/mailman/listinfo/erlang-patches
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

add compiler checked atoms

Richard Carlsson-5
On 02/05/2012 10:34 PM, Michael Truog wrote:

> This is very cool.  It could help document complex code by making
> atom usage more transparent within the module, while also preventing
> errors.  I mention the documentation part, since having the atom
> declaration gives the opportunity for comments specific to that atom
> declaration (so you could have atom declarations as logical
> groupings, similar to what is commonly done for export declarations).
> Do atoms used within the spec declarations need to be mentioned
> within the atom declaration when using this compiler flag?  That
> might bring in a lot of external module atom dependencies, which
> might be counter-productive.

No, atoms occurring in -spec and -type declarations are not checked. As
you say, that could force you to declare a lot more atoms than you want.
You'll have to run Dialyzer to check the consistency of specs.

What you mention about groupings of atoms is exactly the sort of thing I
had in mind. Here are some examples from eunit, which I used as a test
bed for this feature:

%% used in options
-atoms([verbose, report, no_tty, event_log, enqueue, eunit_test_suffix,
         eunit_generator_suffix, eunit_export_suffix]).

%% atoms used in test representations
-atoms([module, application, file, dir, generator, with, local, spawn,
         local, remote, timeout, inorder, inparallel, setup, node,
         foreach, foreachx]).

%% used by erlang:fun_info/2
-atoms([module, name, type, local, external, arity]).

%% used in I/O stream messages
-atoms([io_request, io_reply, put_chars, get_chars, get_line, get_until,
         eof, getopts, setopts, get_geometry, columns, rows, enotsup,
         requests, request]).

%% used in file_monitor server messages
-atoms([monitor, demonitor, automonitor, set_interval, get_interval,
         stop, poll, enable_poll]).

Some of these, you'd put in some header file shared between several of
your modules, but atoms only used in one module should be declared in
that module, to keep the checking as tight as possible.

The explicit declaration and grouping of atoms makes it easy to find
which modules actually use a particular atom, and for what. This can
otherwise be very hard to figure out. If you've ever tried to use grep
to find all places in stdlib where the atom `gen' occurs in the gen_...
modules (it's hardcoded at various points as a callback module name),
you know the sort of pain I'm talking about.

    /Richard

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

add compiler checked atoms

Kenneth Lundin-4
In reply to this post by Richard Carlsson-5
Hi,

I don't think this is a good way to introduce declared atoms.

We already have the -type and -spec notation for definition of
types and function signatures which are then used by Dialyzer for type
checking.
I don't think we should introduce yet another notation that is not
harmonized with -type and -spec.
It is already possible to declare atoms in -type and -spec. Why not use
this already present notation and add
optional checking of atoms against declared atoms inside -type and -spec
instead.

With the suggested -atom declaration we will probably get the same atoms
declared 2 times with different notations and I think that
will clutter down the code with redundant information to an
unacceptable degree.

/Kenneth, Erlang/OTP Ericsson
On Sun, Feb 5, 2012 at 10:15 PM, Richard Carlsson <
carlsson.richard> wrote:

> Sorry, pressed send too soon in the last attempt. Here is the code:
>
>  git fetch git:richcarl/otp.**git warn-unknown-atom
>
> Here is a suggested patch to the compiler, that I'm throwing out there as
> a request for comments. In any large code base, it can be hard to maintain
> atom hygiene - you may have hidden errors due to misspelled atoms, people
> may be adding atoms without much thought, and it can be hard to track which
> atoms are being used where and for what (e.g., atoms used for options,
> error indicators, message tags, callback module and function names, etc.
> This allows you to declare your atoms like so:
>
>  -atoms([foo, bar]).
>
> You can have any number of such declarations, and an atom may occur
> multiple times and/or in different declarations - it's the union of known
> atoms that matters. Note that this declaration is backwards compatible; the
> current compiler will accept it as a generic attribute with no particular
> meaning.
>
> If you specify the compiler option `warn_unused_atom' when compiling a
> module (you can put `-compile([warn_unused_atom]).**' in the module if
> you want to enable the checking in that module only), you will get a
> warning for each atom that has not been explicitly declared. The compiler
> knows about standard atoms such as `ok', `true', `false', `undefined' etc.,
> and it does not check function and module names in calls, or record names
> and fields of records, since these are checked in other ways already.
>
> I don't think this addition warrants an EEP, since it doesn't change the
> language as such; it only adds a backwards compatible feature to the
> compiler. However, I'd like to get some initial feedback before I bother to
> update the documentation and submit it as a full patch.
>
> git:richcarl/otp.**git
>
>   /Richard
> ______________________________**_________________
> erlang-patches mailing list
> erlang-patches
> http://erlang.org/mailman/**listinfo/erlang-patches<http://erlang.org/mailman/listinfo/erlang-patches>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20120214/c2bb3d68/attachment.html>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

add compiler checked atoms

Richard Carlsson-5
On 02/14/2012 09:44 AM, Kenneth Lundin wrote:

> Hi,
> I don't think this is a good way to introduce declared atoms.
> We already have the -type and -spec notation for definition of
> types and function signatures which are then used by Dialyzer for type
> checking.
> I don't think we should introduce yet another notation that is not
> harmonized with -type and -spec.
> It is already possible to declare atoms in -type and -spec. Why not use
> this already present notation and add
> optional checking of atoms against declared atoms inside -type and -spec
> instead.
> With the suggested -atom declaration we will probably get the same atoms
> declared 2 times with different notations and I think that
> will clutter down the code with redundant information to an
> unacceptable degree.

Yes, that is probably a better idea: that any atom occurring in a -type
and/or -spec declaration is implicitly said to be known, and others are
not. This would be an incentive for people to define types for things
like the set of messages to a server or the set of atoms allowed as
flags to a function (or error codes returned from a function).

The question is, should atoms occurring in -spec declarations be taken
as implicit "exists"-declarations, or should it only be those in -type?
If it's only -type that counts, you could get checking of the atoms in
-spec declarations as well, so you don't spell an atom wrong in the spec
for one of 3 versions of a function and suddenly that atom is also
implicitly allowed. I think -type only is the right way.

I'll see if I can change my patch to do this instead.

    /Richard

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

add compiler checked atoms

Kenneth Lundin-4
I think atoms introduced in both -type and -spec should be respected in
this feature. If you want to
guard yourself against a misspelled atom in one -spec you can use -type but
you are not forced to.

And if you introduce an atom in  a -spec and never use it in the code then
you could maybe issue a warning for that too.

/Kenneth

On Tue, Feb 14, 2012 at 10:47 AM, Richard Carlsson <
carlsson.richard> wrote:

> On 02/14/2012 09:44 AM, Kenneth Lundin wrote:
>
>> Hi,
>> I don't think this is a good way to introduce declared atoms.
>> We already have the -type and -spec notation for definition of
>> types and function signatures which are then used by Dialyzer for type
>> checking.
>> I don't think we should introduce yet another notation that is not
>> harmonized with -type and -spec.
>> It is already possible to declare atoms in -type and -spec. Why not use
>> this already present notation and add
>> optional checking of atoms against declared atoms inside -type and -spec
>> instead.
>> With the suggested -atom declaration we will probably get the same atoms
>> declared 2 times with different notations and I think that
>> will clutter down the code with redundant information to an
>> unacceptable degree.
>>
>
> Yes, that is probably a better idea: that any atom occurring in a -type
> and/or -spec declaration is implicitly said to be known, and others are
> not. This would be an incentive for people to define types for things like
> the set of messages to a server or the set of atoms allowed as flags to a
> function (or error codes returned from a function).
>
> The question is, should atoms occurring in -spec declarations be taken as
> implicit "exists"-declarations, or should it only be those in -type? If
> it's only -type that counts, you could get checking of the atoms in -spec
> declarations as well, so you don't spell an atom wrong in the spec for one
> of 3 versions of a function and suddenly that atom is also implicitly
> allowed. I think -type only is the right way.
>
> I'll see if I can change my patch to do this instead.
>
>   /Richard
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20120214/3e7e3850/attachment.html>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: add compiler checked atoms

Richard Carlsson-3
Wow, time flies... has it been that long? Anyway, I found this old branch and decided to rework it like Kenneth suggested. Here's the PR: https://github.com/erlang/otp/pull/1265


        /Richard

2012-02-14 12:24 GMT+01:00 Kenneth Lundin <[hidden email]>:
I think atoms introduced in both -type and -spec should be respected in this feature. If you want to
guard yourself against a misspelled atom in one -spec you can use -type but you are not forced to.
 
And if you introduce an atom in  a -spec and never use it in the code then you could maybe issue a warning for that too.
 
/Kenneth

On Tue, Feb 14, 2012 at 10:47 AM, Richard Carlsson <[hidden email]> wrote:
On 02/14/2012 09:44 AM, Kenneth Lundin wrote:
Hi,
I don't think this is a good way to introduce declared atoms.
We already have the -type and -spec notation for definition of
types and function signatures which are then used by Dialyzer for type
checking.
I don't think we should introduce yet another notation that is not
harmonized with -type and -spec.
It is already possible to declare atoms in -type and -spec. Why not use
this already present notation and add
optional checking of atoms against declared atoms inside -type and -spec
instead.
With the suggested -atom declaration we will probably get the same atoms
declared 2 times with different notations and I think that
will clutter down the code with redundant information to an
unacceptable degree.

Yes, that is probably a better idea: that any atom occurring in a -type and/or -spec declaration is implicitly said to be known, and others are not. This would be an incentive for people to define types for things like the set of messages to a server or the set of atoms allowed as flags to a function (or error codes returned from a function).

The question is, should atoms occurring in -spec declarations be taken as implicit "exists"-declarations, or should it only be those in -type? If it's only -type that counts, you could get checking of the atoms in -spec declarations as well, so you don't spell an atom wrong in the spec for one of 3 versions of a function and suddenly that atom is also implicitly allowed. I think -type only is the right way.

I'll see if I can change my patch to do this instead.

  /Richard



_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Loading...