NoQ created this revision.
NoQ added reviewers: alexfh, gribozavr2, vsavchenko.
NoQ added a project: clang-tools-extra.
Herald added subscribers: martong, Charusso, xazax.hun.
NoQ requested review of this revision.
This is basically standard Objective-C assert. While we could always pass it as
an option, i believe it makes perfect sense to warn on it by default.
The same applies to `NSCAssert` which drops additional support for displaying
Objective-C method information so it acts exactly like C `assert`.
I tested it on live code with actual Foundation headers and it seems to work
correctly out of the box.
I'm also interested in making an even more agressive step in this direction and
adding an option to accept any macro that ends with "...assert" suffix,
case-insensitively. This would probably go under an off-by-default flag. Would
this be acceptable?
https://reviews.llvm.org/D95519
Files:
clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.m
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.m
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.m
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t
+
+int abort();
+
+@interface NSObject
+@end
+
+@interface NSString
+@end
+
+@interface NSAssertionHandler
++ (NSAssertionHandler *)currentHandler;
+- handleFailureInMethod:(SEL)cmd object:(NSObject *)obj desc:(NSString *)desc;
+- handleFailureInFunction:(NSString *)desc;
+@end
+
+#ifndef NDEBUG
+#define NSAssert(condition, description, ...)
\
+ do {
\
+ if (__builtin_expect(!(condition), 0)) {
\
+ [[NSAssertionHandler currentHandler] handleFailureInMethod:_cmd
\
+ object:self
\
+
desc:(description)]; \
+ }
\
+ } while (0);
+#define NSCAssert(condition, description, ...)
\
+ do {
\
+ if (__builtin_expect(!(condition), 0)) {
\
+ [[NSAssertionHandler currentHandler]
handleFailureInFunction:(description)]; \
+ }
\
+ } while (0);
+#else
+#define NSAssert(condition, description, ...) do {} while (0)
+#define NSCAssert(condition, description, ...) do {} while (0)
+#endif
+
+@interface I : NSObject
+- (void)foo;
+@end
+
+@implementation I
+- (void)foo {
+ int x = 0;
+ NSAssert((++x) == 1, @"Ugh.");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in NSAssert()
condition discarded in release builds [bugprone-assert-side-effect]
+}
+@end
+
+void foo() {
+ int x = 0;
+ NSCAssert((++x) == 1, @"Ugh.");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in NSCAssert()
condition discarded in release builds [bugprone-assert-side-effect]
+}
Index: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -72,7 +72,8 @@
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
CheckFunctionCalls(Options.get("CheckFunctionCalls", false)),
- RawAssertList(Options.get("AssertMacros", "assert")) {
+ RawAssertList(Options.get("AssertMacros",
+ "assert,NSAssert,NSCAssert")) {
StringRef(RawAssertList).split(AssertMacros, ",", -1, false);
}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.m
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.m
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t
+
+int abort();
+
+@interface NSObject
+@end
+
+@interface NSString
+@end
+
+@interface NSAssertionHandler
++ (NSAssertionHandler *)currentHandler;
+- handleFailureInMethod:(SEL)cmd object:(NSObject *)obj desc:(NSString *)desc;
+- handleFailureInFunction:(NSString *)desc;
+@end
+
+#ifndef NDEBUG
+#define NSAssert(condition, description, ...) \
+ do { \
+ if (__builtin_expect(!(condition), 0)) { \
+ [[NSAssertionHandler currentHandler] handleFailureInMethod:_cmd \
+ object:self \
+ desc:(description)]; \
+ } \
+ } while (0);
+#define NSCAssert(condition, description, ...) \
+ do { \
+ if (__builtin_expect(!(condition), 0)) { \
+ [[NSAssertionHandler currentHandler] handleFailureInFunction:(description)]; \
+ } \
+ } while (0);
+#else
+#define NSAssert(condition, description, ...) do {} while (0)
+#define NSCAssert(condition, description, ...) do {} while (0)
+#endif
+
+@interface I : NSObject
+- (void)foo;
+@end
+
+@implementation I
+- (void)foo {
+ int x = 0;
+ NSAssert((++x) == 1, @"Ugh.");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in NSAssert() condition discarded in release builds [bugprone-assert-side-effect]
+}
+@end
+
+void foo() {
+ int x = 0;
+ NSCAssert((++x) == 1, @"Ugh.");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in NSCAssert() condition discarded in release builds [bugprone-assert-side-effect]
+}
Index: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -72,7 +72,8 @@
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
CheckFunctionCalls(Options.get("CheckFunctionCalls", false)),
- RawAssertList(Options.get("AssertMacros", "assert")) {
+ RawAssertList(Options.get("AssertMacros",
+ "assert,NSAssert,NSCAssert")) {
StringRef(RawAssertList).split(AssertMacros, ",", -1, false);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
