[PATCH] D30766: Add support for attribute "enum_extensibility"

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/AttrDocs.td:1969
+  let Content = [{
+Attribute ``enum_extensibility`` is used to distinguish between enum definitions that are extensible and those that are not. The attribute can take either ``closed`` or ``open`` as an argument. ``closed`` indicates a variable of the enum type takes a value that corresponds to one of the enumerators listed in the enum definition or, when the enum is annotated with ``flag_enum``, a value that can be constructed using values corresponding to the enumerators. ``open`` indicates a variable of the enum type can take any values allowed by the standard and instructs clang to be more lenient when issuing warnings.
+  }];
----------------
Breaking this up into multiple lines would be appreciated (we aren't strict about the 80 col limit in this file, but it should be readable still).

Also, adding some examples would be appreciated.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2383-2384
   "%0 attribute requires integer constant between %1 and %2 inclusive">;
+def err_attribute_argument_not_supported : Error<
+  "%0 attribute argument not supported: %1">;
 def err_init_priority_object_attr : Error<
----------------
I'd rather see this defined next to `warn_attribute_type_not_supported` as `def err_attribute_type_not_supported : Error<warn_attribute_type_not_supported.Text>;`


================
Comment at: test/Sema/enum-attr.c:27
+
+enum __attribute__((enum_extensibility(arg1))) EnumInvalidArg { // expected-warning{{'enum_extensibility' attribute argument not supported: 'arg1'}}
+  G
----------------
ahatanak wrote:
> arphaman wrote:
> > Should this be an error instead?
> Yes, I agree.
I'm not opposed to it, but we do treat it as a warning for every other attribute (and just ignore the attribute), FWIW.


================
Comment at: test/SemaCXX/enum-attr.cpp:108
+  }
+}
----------------
Missing tests for the attribute being applied to something other than an enum, or taking too few/many arguments.


https://reviews.llvm.org/D30766