https://github.com/isuckatcs updated https://github.com/llvm/llvm-project/pull/128265
>From c13cf10fe9f63c4fa361985388ab1ab6c7e55514 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Mon, 17 Feb 2025 22:50:49 +0100 Subject: [PATCH 1/3] add new check --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../clang-tidy/bugprone/TrueMacroCheck.cpp | 97 +++++++++++++++++++ .../clang-tidy/bugprone/TrueMacroCheck.h | 29 ++++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../clang-tidy/checks/bugprone/true-macro.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../clang-tidy/checkers/bugprone/true-macro.c | 14 +++ 8 files changed, 156 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro.c diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index c5f0b5b28418f..2893d7871a710 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -85,6 +85,7 @@ #include "TerminatingContinueCheck.h" #include "ThrowKeywordMissingCheck.h" #include "TooSmallLoopVariableCheck.h" +#include "TrueMacroCheck.h" #include "UncheckedOptionalAccessCheck.h" #include "UndefinedMemoryManipulationCheck.h" #include "UndelegatedConstructorCheck.h" @@ -247,6 +248,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-throw-keyword-missing"); CheckFactories.registerCheck<TooSmallLoopVariableCheck>( "bugprone-too-small-loop-variable"); + CheckFactories.registerCheck<TrueMacroCheck>( + "bugprone-true-macro"); CheckFactories.registerCheck<UncheckedOptionalAccessCheck>( "bugprone-unchecked-optional-access"); CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index e8309c68b7fca..e479c3c137809 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -86,6 +86,7 @@ add_clang_library(clangTidyBugproneModule STATIC TerminatingContinueCheck.cpp ThrowKeywordMissingCheck.cpp TooSmallLoopVariableCheck.cpp + TrueMacroCheck.cpp UncheckedOptionalAccessCheck.cpp UndefinedMemoryManipulationCheck.cpp UndelegatedConstructorCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp new file mode 100644 index 0000000000000..a1940a1015af1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp @@ -0,0 +1,97 @@ +//===--- TrueMacroCheck.cpp - clang-tidy ----------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "TrueMacroCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/MacroInfo.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include <iostream> + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { +namespace { + +class MacroCallback : public PPCallbacks { + static constexpr const char *TrueMacroSpelling = "true"; + +public: + MacroCallback(TrueMacroCheck *Check, const SourceManager &SM, + Preprocessor *PP) + : Check(Check), SM(&SM), PP(PP) {} + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override { + if (PP->getSpelling(MacroNameTok) == TrueMacroSpelling) + TrueDefined = true; + } + + virtual void MacroUndefined(const Token &MacroNameTok, + const MacroDefinition &MD, + const MacroDirective *Undef) override { + if (PP->getSpelling(MacroNameTok) == TrueMacroSpelling) + TrueDefined = false; + } + + virtual void If(SourceLocation Loc, SourceRange ConditionRange, + ConditionValueKind ConditionValue) override { + StringRef Condition = + Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), + PP->getSourceManager(), PP->getLangOpts()); + + for (auto &&Identifier : identifiersInCondition(Condition)) + std::cout << Identifier.str() << ' ' << Identifier.size() << '\n'; + } + +private: + void emitDiagnostic() {} + + std::vector<StringRef> identifiersInCondition(StringRef Condition) { + const static auto Start = [](char C) { + return C == '_' || ('a' <= C && C <= 'z') || ('A' <= C && C <= 'Z'); + }; + + const static auto Continue = [](char C) { + return ('0' <= C && C <= '9') || Start(C); + }; + + std::vector<StringRef> Identifiers; + const char *Str = nullptr; + + for (size_t I = 0; I < Condition.size(); ++I) { + char C = Condition[I]; + + if (!Str && Start(C)) { + Str = Condition.begin() + I; + } else if (Str && !Continue(C)) { + Identifiers.emplace_back(Str, Condition.begin() + I - Str); + Str = nullptr; + } + } + + if (Str) + Identifiers.emplace_back(Str, Condition.end() - Str); + + return Identifiers; + } + + bool TrueDefined = false; + + TrueMacroCheck *Check; + const SourceManager *SM; + Preprocessor *PP; +}; +} // namespace + +void TrueMacroCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks(std::make_unique<MacroCallback>(this, SM, PP)); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h new file mode 100644 index 0000000000000..ca6e5a7bfa64b --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h @@ -0,0 +1,29 @@ +//===--- TrueMacroCheck.h - clang-tidy --------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_TRUEMACROCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_TRUEMACROCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +class TrueMacroCheck : public ClangTidyCheck { +public: + TrueMacroCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return !LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_TRUEMACROCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6b8fe22242417..49220fbb39836 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -91,6 +91,11 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-true-macro + <clang-tidy/checks/bugprone/true-macro>` check. + + FIXME: Write a short description. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst new file mode 100644 index 0000000000000..70b313774ef15 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - bugprone-true-macro + +bugprone-true-macro +=================== + +FIXME: Describe what patterns does the check detect and why. Give examples. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 7b9b905ef7671..2940e8ee7248a 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -153,6 +153,7 @@ Clang-Tidy Checks :doc:`bugprone-terminating-continue <bugprone/terminating-continue>`, "Yes" :doc:`bugprone-throw-keyword-missing <bugprone/throw-keyword-missing>`, :doc:`bugprone-too-small-loop-variable <bugprone/too-small-loop-variable>`, + :doc:`bugprone-true-macro <bugprone/true-macro>`, "Yes" :doc:`bugprone-unchecked-optional-access <bugprone/unchecked-optional-access>`, :doc:`bugprone-undefined-memory-manipulation <bugprone/undefined-memory-manipulation>`, :doc:`bugprone-undelegated-constructor <bugprone/undelegated-constructor>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro.c new file mode 100644 index 0000000000000..25ce8b63bb4cd --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro.c @@ -0,0 +1,14 @@ +// RUN: %check_clang_tidy -std=c99-or-later %s bugprone-true-macro %t + +// FIXME: Add something that triggers the check here. +void f(); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [bugprone-true-macro] + +// FIXME: Verify the applied fix. +// * Make the CHECK patterns specific enough and try to make verified lines +// unique to avoid incorrect matches. +// * Use {{}} for regular expressions. +// CHECK-FIXES: {{^}}void awesome_f();{{$}} + +// FIXME: Add something that doesn't trigger the check here. +void awesome_f2(); >From 92170548bd83e4f396854452c9a16220e224a22c Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Tue, 18 Feb 2025 23:04:44 +0100 Subject: [PATCH 2/3] implement check --- .../clang-tidy/bugprone/TrueMacroCheck.cpp | 46 +++++++++++++++---- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp index a1940a1015af1..4b1375db34039 100644 --- a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp @@ -7,11 +7,9 @@ //===----------------------------------------------------------------------===// #include "TrueMacroCheck.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/MacroInfo.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" -#include <iostream> using namespace clang::ast_matchers; @@ -22,11 +20,20 @@ class MacroCallback : public PPCallbacks { static constexpr const char *TrueMacroSpelling = "true"; public: - MacroCallback(TrueMacroCheck *Check, const SourceManager &SM, - Preprocessor *PP) - : Check(Check), SM(&SM), PP(PP) {} + MacroCallback(TrueMacroCheck *Check, Preprocessor *PP) + : Check(Check), PP(PP) {} void MacroDefined(const Token &MacroNameTok, const MacroDirective *MD) override { + if (TrueDefined) + return; + + const MacroInfo *MI = MD->getMacroInfo(); + for (const Token &Tok : MI->tokens()) { + if (PP->getSpelling(Tok) == TrueMacroSpelling) + emitDiagnostics(Tok.getLocation(), + {Tok.getLocation(), Tok.getEndLoc()}); + } + if (PP->getSpelling(MacroNameTok) == TrueMacroSpelling) TrueDefined = true; } @@ -44,12 +51,32 @@ class MacroCallback : public PPCallbacks { Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), PP->getSourceManager(), PP->getLangOpts()); - for (auto &&Identifier : identifiersInCondition(Condition)) - std::cout << Identifier.str() << ' ' << Identifier.size() << '\n'; + if (!TrueDefined && Condition == TrueMacroSpelling) { + emitDiagnostics(ConditionRange.getBegin(), ConditionRange); + return; + } + + for (auto &&Identifier : identifiersInCondition(Condition)) { + if (!TrueDefined && Identifier == TrueMacroSpelling) { + emitDiagnostics(Loc, {}, true); + break; + } + } } private: - void emitDiagnostic() {} + void emitDiagnostics(SourceLocation Loc, SourceRange ReplaceRange, + bool InCondition = false) { + DiagnosticBuilder Builder = + Check->diag(Loc, "in C 'true'%select{| in the condition}0 is treated " + "as an undefined " + "macro and evaluates to a falsy value; " + "consider replacing it with '1'") + << InCondition; + + if (!InCondition) + Builder << FixItHint::CreateReplacement(ReplaceRange, "1"); + } std::vector<StringRef> identifiersInCondition(StringRef Condition) { const static auto Start = [](char C) { @@ -83,7 +110,6 @@ class MacroCallback : public PPCallbacks { bool TrueDefined = false; TrueMacroCheck *Check; - const SourceManager *SM; Preprocessor *PP; }; } // namespace @@ -91,7 +117,7 @@ class MacroCallback : public PPCallbacks { void TrueMacroCheck::registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - PP->addPPCallbacks(std::make_unique<MacroCallback>(this, SM, PP)); + PP->addPPCallbacks(std::make_unique<MacroCallback>(this, PP)); } } // namespace clang::tidy::bugprone >From f6aefcb282e7129a668bb64ebc56c4863efdae59 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Sat, 22 Feb 2025 03:14:04 +0100 Subject: [PATCH 3/3] started testing and writing docs --- .../clang-tidy/bugprone/TrueMacroCheck.h | 5 +++++ clang-tools-extra/docs/ReleaseNotes.rst | 2 +- .../clang-tidy/checks/bugprone/true-macro.rst | 16 +++++++++++++++- .../checkers/bugprone/true-macro-condition.c | 10 ++++++++++ .../checkers/bugprone/true-macro-defintion.c | 5 +++++ .../clang-tidy/checkers/bugprone/true-macro.c | 14 -------------- 6 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-condition.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-defintion.c delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro.c diff --git a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h index ca6e5a7bfa64b..24116dfcbeb4e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h @@ -13,6 +13,11 @@ namespace clang::tidy::bugprone { +/// This check gives a warning if `true` is used in the preprocessor when not +/// defined in C. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/true-macro.html class TrueMacroCheck : public ClangTidyCheck { public: TrueMacroCheck(StringRef Name, ClangTidyContext *Context) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 49220fbb39836..51683093e5d23 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -94,7 +94,7 @@ New checks - New :doc:`bugprone-true-macro <clang-tidy/checks/bugprone/true-macro>` check. - FIXME: Write a short description. + This check gives a warning if ``true`` is used in the preprocessor when not defined in C. New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst index 70b313774ef15..a5551ed9d9f2c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst @@ -3,4 +3,18 @@ bugprone-true-macro =================== -FIXME: Describe what patterns does the check detect and why. Give examples. +In C++, ``true`` is considered a keyword by the preprocessor so an ``#if true`` enters the true branch, +while in C, ``true`` is not treated as a special keyword by the preprocessor, so the false branch is entered. + +The following snippet returns ``1`` in C++, but ``0`` in C. + +.. code-block:: c++ + int main() { + #if true + return 1; + #else + return 0; + #endif + } + +The check also offers fix-it hints. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-condition.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-condition.c new file mode 100644 index 0000000000000..6495c282f34f4 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-condition.c @@ -0,0 +1,10 @@ +// RUN: %check_clang_tidy -std=c99-or-later %s bugprone-true-macro %t + +#if true +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: in C 'true' is treated as an undefined macro and evaluates to a falsy value; consider replacing it with '1' [bugprone-true-macro] +// CHECK-FIXES: 1 +#endif + +#if false || true +// CHECK-MESSAGES: :[[@LINE-1]]:2: warning: in C 'true' in the condition is treated as an undefined macro and evaluates to a falsy value; consider replacing it with '1' [bugprone-true-macro] +#endif diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-defintion.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-defintion.c new file mode 100644 index 0000000000000..981a0e84cc241 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-defintion.c @@ -0,0 +1,5 @@ +// RUN: %check_clang_tidy -std=c99-or-later %s bugprone-true-macro %t + +#define FOO true +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: in C 'true' is treated as an undefined macro and evaluates to a falsy value; consider replacing it with '1' [bugprone-true-macro] +// CHECK-FIXES: 1 diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro.c deleted file mode 100644 index 25ce8b63bb4cd..0000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro.c +++ /dev/null @@ -1,14 +0,0 @@ -// RUN: %check_clang_tidy -std=c99-or-later %s bugprone-true-macro %t - -// FIXME: Add something that triggers the check here. -void f(); -// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [bugprone-true-macro] - -// FIXME: Verify the applied fix. -// * Make the CHECK patterns specific enough and try to make verified lines -// unique to avoid incorrect matches. -// * Use {{}} for regular expressions. -// CHECK-FIXES: {{^}}void awesome_f();{{$}} - -// FIXME: Add something that doesn't trigger the check here. -void awesome_f2(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits