aaron.ballman added a comment.
You should also add Sema tests that ensure the attribute applies to the
expected AST nodes, is diagnosed appropriately when applied to something it
shouldn't be applied to, accepts no args, etc. Basically, all of the semantic
places we could warn on should have test coverage, as well as demonstrations of
correct usage.
================
Comment at: include/clang/Basic/Attr.td:1798
+def ObjCClassStub: Attr {
+ let Spellings = [Clang<"objc_class_stub">];
----------------
Formatting is a smidge off here -- should be `ObjCClassStub : Attr` with the
extra whitespace.
================
Comment at: include/clang/Basic/Attr.td:1802
+ let Documentation = [ObjCClassStubDocs];
+}
+
----------------
Does this attribute make sense outside of ObjC? e.g., should it require an ObjC
compiland? If it should only be used in ObjC, then it should have a `LangOpts`
field.
================
Comment at: include/clang/Basic/AttrDocs.td:1116
+def ObjCClassStubDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
----------------
This seems like the wrong category -- the attribute doesn't apply to functions.
================
Comment at: include/clang/Basic/AttrDocs.td:1118
+ let Content = [{
+This attribute specifies that the Objective-C class to which it applies has
dynamically-allocated metadata. Classes annotated with this attribute cannot be
subclassed.
+ }];
----------------
rjmccall wrote:
> You should probably check that the user doesn't try to subclass classes
> annotated with this attribute, then. :)
Try to keep the docs wrapped to the usual 80-col limit.
I think this could use a bit more exposition, or links to explain stuff. Based
on the small amount here, I still have no idea why I would use this attribute.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59628/new/
https://reviews.llvm.org/D59628
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
