This revision was automatically updated to reflect the committed changes.
Closed by commit rL319098: add new check to find OSSpinlock usage (authored by
Wizard).
Repository:
rL LLVM
https://reviews.llvm.org/D40325
Files:
clang-tools-extra/trunk/clang-tidy/objc/AvoidSpinlockCheck.cpp
clang
Wizard updated this revision to Diff 124456.
Wizard added a comment.
Herald added a subscriber: klimek.
fix conflict
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40325
Files:
clang-tidy/objc/AvoidSpinlockCheck.cpp
clang-tidy/objc/AvoidSpinlockCheck.h
clang-tidy/objc/CMa
benhamilton added inline comments.
Comment at: test/clang-tidy/objc-avoid-spinlock.m:4
+typedef int OSSpinLock;
+void OSSpinlockTry(OSSpinLock *__lock) {}
+
Wizard wrote:
> benhamilton wrote:
> > Not sure why you declared (and defined?) this one but not the other
Wizard updated this revision to Diff 124448.
Wizard added a comment.
fix test
https://reviews.llvm.org/D40325
Files:
clang-tidy/objc/AvoidSpinlockCheck.cpp
clang-tidy/objc/AvoidSpinlockCheck.h
clang-tidy/objc/CMakeLists.txt
clang-tidy/objc/ObjCTidyModule.cpp
docs/ReleaseNotes.rst
do
Wizard marked an inline comment as done.
Wizard added inline comments.
Comment at: test/clang-tidy/objc-avoid-spinlock.m:4
+typedef int OSSpinLock;
+void OSSpinlockTry(OSSpinLock *__lock) {}
+
benhamilton wrote:
> Not sure why you declared (and defined?) this one
benhamilton added a comment.
Thanks, looks good. Just one question about the test.
Comment at: test/clang-tidy/objc-avoid-spinlock.m:4
+typedef int OSSpinLock;
+void OSSpinlockTry(OSSpinLock *__lock) {}
+
Not sure why you declared (and defined?) this one but no
Wizard updated this revision to Diff 124434.
Wizard marked 5 inline comments as done.
Wizard added a comment.
fix doc
https://reviews.llvm.org/D40325
Files:
clang-tidy/objc/AvoidSpinlockCheck.cpp
clang-tidy/objc/AvoidSpinlockCheck.h
clang-tidy/objc/CMakeLists.txt
clang-tidy/objc/ObjCTid
benhamilton accepted this revision.
benhamilton added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tidy/objc/AvoidSpinlockCheck.cpp:31-32
+ diag(MatchedExpr->getLocStart(),
+ "deprecated usage of OSSpinlock; Please use other locks or
Wizard marked 4 inline comments as done.
Wizard added inline comments.
Comment at: clang-tidy/objc/CMakeLists.txt:4
add_clang_library(clangTidyObjCModule
+ AvoidSpinlockCheck.cpp
ForbiddenSubclassingCheck.cpp
benhamilton wrote:
> IMHO this is really a check
Wizard updated this revision to Diff 124427.
Wizard added a comment.
address comments
https://reviews.llvm.org/D40325
Files:
clang-tidy/objc/AvoidSpinlockCheck.cpp
clang-tidy/objc/AvoidSpinlockCheck.h
clang-tidy/objc/CMakeLists.txt
clang-tidy/objc/ObjCTidyModule.cpp
docs/ReleaseNotes.
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.
=
Wizard updated this revision to Diff 124011.
Wizard marked an inline comment as done.
Wizard added a comment.
fix nit
https://reviews.llvm.org/D40325
Files:
clang-tidy/objc/AvoidSpinlockCheck.cpp
clang-tidy/objc/AvoidSpinlockCheck.h
clang-tidy/objc/CMakeLists.txt
clang-tidy/objc/ObjCTid
hokein added inline comments.
Comment at: clang-tidy/objc/AvoidSpinlockCheck.cpp:35
+ diag(MatchedExpr->getLocStart(),
+ "OSSpinlock is deprecated on iOS. Please use other locks or dispatch "
+ "queue.");
hokein wrote:
> nit: clang-tidy message is no
hokein added a comment.
The implementation looks good, I will let Ben to do the Object-C specific
review since he probably knows more about it.
Comment at: clang-tidy/objc/AvoidSpinlockCheck.cpp:35
+ diag(MatchedExpr->getLocStart(),
+ "OSSpinlock is deprecated on iOS. P
Wizard updated this revision to Diff 123860.
Wizard added a comment.
add doc to header file
https://reviews.llvm.org/D40325
Files:
clang-tidy/objc/AvoidSpinlockCheck.cpp
clang-tidy/objc/AvoidSpinlockCheck.h
clang-tidy/objc/CMakeLists.txt
clang-tidy/objc/ObjCTidyModule.cpp
docs/Release
15 matches
Mail list logo
