[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

[Aaron Ballman via Phabricator via cfe-commits]

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
+}
+ at 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