aaron.ballman added a comment.
In D92039#2441992 <https://reviews.llvm.org/D92039#2441992>, @vsavchenko wrote:
> @aaron.ballman Can you please take a look at the attribute side of this?
Thank you for your patience, sorry it took me a while to get to this!
================
Comment at: clang/include/clang/Basic/Attr.td:1774
+ let LangOpts = [ObjC];
+ let Documentation = [Undocumented];
+}
----------------
No new undocumented attributes, please.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3623
+static void handleCalledOnceAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (D->isInvalidDecl())
+ return;
----------------
I don't think there's a need for this check -- attaching the attribute to an
invalid declaration doesn't hurt anything downstream (I believe), but failing
to attach the attribute harms AST fidelity for the folks doing work on
retaining erroneous AST nodes.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3630
+ if (!isFunctionLike(*T)) {
+ S.Diag(AL.getLoc(), diag::err_called_once_attribute_wrong_type);
+ return;
----------------
Because there can be multiple parameters in the declaration, passing the range
to the specific problematic parameter is helpful.
================
Comment at: clang/test/SemaObjC/warn-called-once.m:887
+}
+@end
----------------
Can you also add a test that shows this works with lambda or block parameters
that contain the attribute? e.g.,
```
[](void (*fp CALLED_ONCE)()) { ... }(some_fp);
```
Also, you should add some tests that the attribute diagnoses when applied to
something other than a parameter, is given an argument, and a test that the
attribute is rejected unless in ObjC.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92039/new/
https://reviews.llvm.org/D92039
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
