Skip to content

Commit 0d6d8a8

Browse files
committed
[clang-tidy] Fix bugprone-assert-side-effect to actually give warnings
Some time ago a patch was merged to disable all clang-tidy warnings from system macros. This led to bugprone-assert-side-effect silently no longer working, since the warnings came from a system macro. The problem was not detected because the fake assert functions were implemented in the same file as the test, instead of being a system include like it's done in the real world. Move the assert to a proper system header, and fix the code to warn at the correct location. This patch is breakdown from https://reviews.llvm.org/D147081 by PiotrZSL. Fixes #62314 Differential Revision: https://reviews.llvm.org/D150071
1 parent 4d0d295 commit 0d6d8a8

File tree

3 files changed

+43
-45
lines changed

3 files changed

+43
-45
lines changed

clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,13 @@ void AssertSideEffectCheck::check(const MatchFinder::MatchResult &Result) {
117117
StringRef AssertMacroName;
118118
while (Loc.isValid() && Loc.isMacroID()) {
119119
StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LangOpts);
120+
Loc = SM.getImmediateMacroCallerLoc(Loc);
120121

121122
// Check if this macro is an assert.
122123
if (llvm::is_contained(AssertMacros, MacroName)) {
123124
AssertMacroName = MacroName;
124125
break;
125126
}
126-
Loc = SM.getImmediateMacroCallerLoc(Loc);
127127
}
128128
if (AssertMacroName.empty())
129129
return;
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#pragma clang system_header
2+
3+
int abort();
4+
5+
#ifdef NDEBUG
6+
#define assert(x) 1
7+
#else
8+
#define assert(x) \
9+
if (!(x)) \
10+
(void)abort()
11+
#endif
12+
13+
void print(...);
14+
#define assert2(e) (__builtin_expect(!(e), 0) ? \
15+
print (#e, __FILE__, __LINE__) : (void)0)
16+
17+
#ifdef NDEBUG
18+
#define my_assert(x) 1
19+
#else
20+
#define my_assert(x) \
21+
((void)((x) ? 1 : abort()))
22+
#endif
23+
24+
#ifdef NDEBUG
25+
#define not_my_assert(x) 1
26+
#else
27+
#define not_my_assert(x) \
28+
if (!(x)) \
29+
(void)abort()
30+
#endif
31+
32+
#define real_assert(x) ((void)((x) ? 1 : abort()))
33+
#define wrap1(x) real_assert(x)
34+
#define wrap2(x) wrap1(x)
35+
#define convoluted_assert(x) wrap2(x)
36+
37+
#define msvc_assert(expression) (void)( \
38+
(!!(expression)) || \
39+
(abort(), 0) \
40+
)

clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,5 @@
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
2-
3-
//===--- assert definition block ------------------------------------------===//
4-
int abort() { return 0; }
5-
6-
#ifdef NDEBUG
7-
#define assert(x) 1
8-
#else
9-
#define assert(x) \
10-
if (!(x)) \
11-
(void)abort()
12-
#endif
13-
14-
void print(...);
15-
#define assert2(e) (__builtin_expect(!(e), 0) ? \
16-
print (#e, __FILE__, __LINE__) : (void)0)
17-
18-
#ifdef NDEBUG
19-
#define my_assert(x) 1
20-
#else
21-
#define my_assert(x) \
22-
((void)((x) ? 1 : abort()))
23-
#endif
24-
25-
#ifdef NDEBUG
26-
#define not_my_assert(x) 1
27-
#else
28-
#define not_my_assert(x) \
29-
if (!(x)) \
30-
(void)abort()
31-
#endif
32-
33-
#define real_assert(x) ((void)((x) ? 1 : abort()))
34-
#define wrap1(x) real_assert(x)
35-
#define wrap2(x) wrap1(x)
36-
#define convoluted_assert(x) wrap2(x)
37-
38-
#define msvc_assert(expression) (void)( \
39-
(!!(expression)) || \
40-
(abort(), 0) \
41-
)
42-
43-
44-
//===----------------------------------------------------------------------===//
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 -I %S/Inputs/assert-side-effect
2+
#include <assert.h>
453

464
bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
475

0 commit comments

Comments
 (0)