[PATCH] D150071: [clang-tidy] Fix bugprone-assert-side-effect to actually give warnings

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h:1
+int abort() { return 0; }
+
----------------
There should be no need to define the function (the real header wouldn't either).


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp:1
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions
-
-//===--- assert definition block ------------------------------------------===//
-int abort() { return 0; }
-
-#ifdef NDEBUG
-#define assert(x) 1
-#else
-#define assert(x)                                                              \
-  if (!(x))                                                                    \
-  (void)abort()
-#endif
-
-void print(...);
-#define assert2(e) (__builtin_expect(!(e), 0) ?                                \
-                       print (#e, __FILE__, __LINE__) : (void)0)
-
-#ifdef NDEBUG
-#define my_assert(x) 1
-#else
-#define my_assert(x)                                                           \
-  ((void)((x) ? 1 : abort()))
-#endif
-
-#ifdef NDEBUG
-#define not_my_assert(x) 1
-#else
-#define not_my_assert(x)                                                       \
-  if (!(x))                                                                    \
-  (void)abort()
-#endif
-
-#define real_assert(x) ((void)((x) ? 1 : abort()))
-#define wrap1(x) real_assert(x)
-#define wrap2(x) wrap1(x)
-#define convoluted_assert(x) wrap2(x)
-
-#define msvc_assert(expression) (void)(                                        \
-            (!!(expression)) ||                                                \
-            (abort(), 0)                                                       \
-        )
-
-
-//===----------------------------------------------------------------------===//
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions -isystem %S/Inputs/assert-side-effect
+#include <assert.h>
----------------
Perhaps a more reliable (or a "belt and suspenders") way would be to add the following pragma to the header:

```
#pragma clang system_header
```

This way I believe it would not matter whether the header is found through `-isystem` or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150071