[PATCH] D67567: [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Looks good! A couple nits/questions.

Does the check allow variables in anonymous namespaces?

  namespace {
  dispatch_once_t onceToken;
  }

I //think// such variables should satisfy initialization requirements.

If not, can we update the check to not trigger on variables? Either way, can we add a test to verify behavior for variables in anonymous namespaces?



================
Comment at: clang-tools-extra/clang-tidy/darwin/CMakeLists.txt:13
+  clangLex
+  clangSerialization
+  clangTidy
----------------
Is this library necessary?


================
Comment at: clang-tools-extra/clang-tidy/darwin/CMakeLists.txt:16
+  clangTidyUtils
+  clangTooling
+  )
----------------
Is this library necessary?


================
Comment at: clang-tools-extra/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp:44
+      diag(PD->getTypeSpecStartLoc(),
+           "dispatch_once_ts must have static or global storage duration; "
+           "function parameters should be pointer references");
----------------
"dispatch_once_t variables" to be consistent.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst:15
+
+Programmers have also been known to make ``dispatch_once_t``s be members of
+structs/classes, with the intent to lazily perform some expensive struct or
----------------
Maybe rephrase to this:
```
``dispatch_once_t`` variables
```


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst:16
+Programmers have also been known to make ``dispatch_once_t``s be members of
+structs/classes, with the intent to lazily perform some expensive struct or
+class member initialization only once; however, this violates the libdispatch
----------------
"structs or classes"


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67567/new/

https://reviews.llvm.org/D67567