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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
