[cfe-dev] [RFC] A new attribute for enums

[Richard Smith via cfe-dev]

On 17 February 2017 at 10:21, Akira Hatanaka via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> I’d like to propose a new attribute for enums.
>
> ### The proposed attribute ###
> The attribute is tentatively named “enum_style” and takes two arguments.
> The first argument determines the style of the enum, either “option” or
> “choice".
>

These seem like a bad choice of names to me, since they are synonyms in
English. Maybe call the flag form "flag"? But I don't actually see why we
need or want to merge these two orthogonal arguments into a single
attribute at all. Why not instead:

__attribute__((open_enum))
__attribute__((closed_enum))
__attribute__((flag_enum, open_enum))
__attribute__((flag_enum, closed_enum))

?

“option” implies that the enum can be used as a one-bit flag, just like
> “flag_enum", and it’s OK to OR the values to create a new value. “choice”
> implies the enum cannot be used like “option” enums. The second argument is
> used to indicate whether or not the enum can be extended. If an enum is
> marked “closed”, clang can assume a variable of the enum type always has a
> value that is in the range determined by the enumerators listed in the enum
> definition.
>

What exactly do you mean by this? The C++ rule for unscoped enums is that
the range of representable values is (roughly) the values that fit in the
smallest bit-field that can contain the enum. I assume that would be the
rule for flag-style enums; for enumeration-style enums, would you restrict
the range further to just lowest-declared-value to highest-declared-value
(inclusive)?


> If it’s marked “open”, it doesn’t have the restriction.
>
There are four possible combinations:
>
> 1. "choice, closed"
> 2. "choice, open"
> 3. "option, closed"
> 4. "option, open"
>
> Attribute “flag_enum” we have today is equivalent to "option,closed".
>
> In addition, I’m considering adding a command line option that specifies
> the default enum-style for unannotated enums.
>

A command-line option amounts to adding a new language dialect; that seems
like a bad idea to me.


> ### Motivation for the new attribute ###
> There are several areas that can be improved using the new attribute and
> command line option.
>
> 1. Warnings
> The new attribute can improve the accuracy of enum-related warnings such
> as -Wassign-enum and give better control over when the warnings are issued.
>
> -Wassign-enum currently warns whenever a value that is out of the range
> determined by the enumerators is assigned to an enum variable. For a
> flag-enum, a value is in range if it can be created by ORing the
> enumerators listed in the enum definition or is a complement of one of the
> in-range values. For enums that are not flag-enums, only the values of the
> enumerators listed in the enum definition are considered to be in range.
> The warning is helpful in catching out-of-range values that are
> unintentionally assigned, but would be too strict if a project
> intentionally extended an enum by defining out-of-range “private” values
> (this does happen often). If the compiler knows an enum is “open”, it can
> choose not to issue warnings.
>
> Another problem with the current approach is that all the enums the
> compiler sees have to be classified into flag-enums or non-flag-enums and
> the flag-enums have to be annotated. This requires a lot of work up front
> to determine whether or not an enum is a flag-enum, and sometimes it’s not
> even possible to annotate the enums if they are defined in a third party
> library that cannot be modified. With the command line option for
> specifying the default enum-style, users can instruct the compiler not to
> issue warnings if the enum is unannotated (the default can be either
> "choice,open” or “option,open”) and add the attributes to the enum
> definitions in an incremental fashion.
>
> 2. Code-completion and debugging
> Code-completion tools can offer better suggestions based on whether the
> enum is a choice or an option. For example, if we had an "option" enum like
> this:
>
> enum __attribute((enum_style(option, closed)))__ MyEnum {
>   E1 = 1, E2 = 2,
> };
>
> and the user requests a code completion for this:
>
> MyEnum = E1 | <esc>
>
> code completion would recognize MyEnum is an “option" enum and could offer
> E2 as a suggestion. If MyEnum were a “choice" enum, it would offer no
> suggestions.
>
> Debugging experience can be improved too. For example, when a MyEnum
> variable is set to 3, the debugger could show “E1 | E2” instead of showing
> the raw value 3.
>
> 3. clang importer
> In order to determine whether a C or ObjC enum maps to an enum or option
> set in swift, swift’s clang importer looks at whether the enum was declared
> using macros such as CF_ENUM or CF_OPTIONS (the macros are explained in the
> link below). With the proposed attribute, this hack can be removed.
>
> https://developer.apple.com/library/content/releasenotes/
> ObjectiveC/ModernizationObjC/AdoptingModernObjective-C/
> AdoptingModernObjective-C.html
>
> 4. Optimization
> There is a command line option named -fstrict-enums, which allows the
> compiler to optimize code using the assumption that the value of an enum
> variable is in range. This option can safely be used only if it is known
> that the values the variables of enum types can take are always in range.
> With the new attribute, the compiler can focus on variables of enum types
> that are marked “closed” and optimize them and leave other variables
> unoptimized.
>
> ### What will change ###
> I plan to add support for the new attribute in Sema and change the code
> that issues warning. I’m not planning to work on the IRGen optimization
> that takes advantage of the information the new attribute provides. It will
> be left as future work.
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170217/1906ae49/attachment.html>