Author: Carlos Galvez Date: 2023-05-09T16:45:02Z New Revision: 0d6d8a853a6ea29b5f461a475a8f8eb7e7ba18e2
URL: https://github.com/llvm/llvm-project/commit/0d6d8a853a6ea29b5f461a475a8f8eb7e7ba18e2 DIFF: https://github.com/llvm/llvm-project/commit/0d6d8a853a6ea29b5f461a475a8f8eb7e7ba18e2.diff LOG: [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 https://github.com/llvm/llvm-project/issues/62314 Differential Revision: https://reviews.llvm.org/D150071 Added: clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h Modified: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp index 600a923b211cf..07a987359d4d8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp @@ -117,13 +117,13 @@ void AssertSideEffectCheck::check(const MatchFinder::MatchResult &Result) { StringRef AssertMacroName; while (Loc.isValid() && Loc.isMacroID()) { StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LangOpts); + Loc = SM.getImmediateMacroCallerLoc(Loc); // Check if this macro is an assert. if (llvm::is_contained(AssertMacros, MacroName)) { AssertMacroName = MacroName; break; } - Loc = SM.getImmediateMacroCallerLoc(Loc); } if (AssertMacroName.empty()) return; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h new file mode 100644 index 0000000000000..904597ff2184e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/assert-side-effect/assert.h @@ -0,0 +1,40 @@ +#pragma clang system_header + +int abort(); + +#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) \ + ) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp index c327007651d4c..ccafeb4b7f3b1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp @@ -1,47 +1,5 @@ -// 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 -I %S/Inputs/assert-side-effect +#include <assert.h> bool badButIgnoredFunc(int a, int b) { return a * b > 0; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
