[PATCH] D40325: add new check to find OSSpinlock usage

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.


================
Comment at: clang-tidy/objc/AvoidSpinlockCheck.cpp:22-24
+  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+    return;
+  }
----------------
Why? `OSSpinLock()` calls should also be avoided in C++.

I think you should remove this.



================
Comment at: clang-tidy/objc/AvoidSpinlockCheck.cpp:27
+      callExpr(
+          callee((functionDecl(matchesName("::OSSpinlock(Lock|Unlock|Try)")))))
+          .bind("spinlock"),
----------------
`matchesName()` uses a regular expression, and is many orders of magnitude slower than `hasAnyName(A, B, C)`.

Can you change to `hasAnyName()`, please?



================
Comment at: clang-tidy/objc/CMakeLists.txt:4
 add_clang_library(clangTidyObjCModule
+  AvoidSpinlockCheck.cpp
   ForbiddenSubclassingCheck.cpp
----------------
IMHO this is really a check which should apply to products built on Apple SDKs, not for Objective-C.

I don't know if that means we should move this to an `apple` submodule or if there is a better solution.

You don't have to move it in this review, but let's open up a discussion to figure out where non-ObjC checks should go.


================
Comment at: docs/clang-tidy/checks/objc-avoid-spinlock.rst:13
+- `OSSpinlockTry`
+- `OSSpinlockUnlcok`
+
----------------
Typo: Unlcok -> Unlock



https://reviews.llvm.org/D40325