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/5] 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/5] 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/5] 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(); >From 44a5541f25a2627cd43a3bf4e2e192deff67cc74 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Sat, 22 Feb 2025 11:43:42 +0100 Subject: [PATCH 4/5] fix errors --- .../bugprone/BugproneTidyModule.cpp | 3 +- .../clang-tidy/checks/bugprone/true-macro.rst | 4 +- .../checkers/bugprone/true-macro-condition.c | 10 ----- .../checkers/bugprone/true-macro-defintion.c | 38 +++++++++++++++++++ 4 files changed, 42 insertions(+), 13 deletions(-) delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-condition.c diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 2893d7871a710..6d993669f2303 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -248,8 +248,7 @@ class BugproneModule : public ClangTidyModule { "bugprone-throw-keyword-missing"); CheckFactories.registerCheck<TooSmallLoopVariableCheck>( "bugprone-too-small-loop-variable"); - CheckFactories.registerCheck<TrueMacroCheck>( - "bugprone-true-macro"); + CheckFactories.registerCheck<TrueMacroCheck>("bugprone-true-macro"); CheckFactories.registerCheck<UncheckedOptionalAccessCheck>( "bugprone-unchecked-optional-access"); CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>( 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 a5551ed9d9f2c..89ec804130b37 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 @@ -9,6 +9,7 @@ while in C, ``true`` is not treated as a special keyword by the preprocessor, so The following snippet returns ``1`` in C++, but ``0`` in C. .. code-block:: c++ + int main() { #if true return 1; @@ -17,4 +18,5 @@ The following snippet returns ``1`` in C++, but ``0`` in C. #endif } -The check also offers fix-it hints. +The check identifies such cases, when ``true`` is used without being defined first and also offers fix-its in +some cases. 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 deleted file mode 100644 index 6495c282f34f4..0000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-condition.c +++ /dev/null @@ -1,10 +0,0 @@ -// 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 index 981a0e84cc241..060f42c4aee0f 100644 --- 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 @@ -3,3 +3,41 @@ #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 + +#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 + +#define true 1 + +#define FOO true + +#if true +#endif + +#if false || true +#endif + +#undef true + +#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 + +#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 + +#define true true +// CHECK-MESSAGES: :[[@LINE-1]]:14: 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 >From 46af06bac6fad5f0c16aaefec6c32695ebc3b266 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isucka...@users.noreply.github.com> Date: Sun, 23 Feb 2025 00:56:10 +0100 Subject: [PATCH 5/5] addressed comments --- .../clang-tidy/bugprone/TrueMacroCheck.cpp | 4 ++-- clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h | 10 +++++++--- clang-tools-extra/docs/ReleaseNotes.rst | 7 ++++++- .../docs/clang-tidy/checks/bugprone/true-macro.rst | 9 +++++---- .../checkers/bugprone/true-macro-defintion.c | 4 +++- 5 files changed, 23 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp index 4b1375db34039..20634f8a460c6 100644 --- a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp @@ -47,7 +47,7 @@ class MacroCallback : public PPCallbacks { virtual void If(SourceLocation Loc, SourceRange ConditionRange, ConditionValueKind ConditionValue) override { - StringRef Condition = + const StringRef Condition = Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), PP->getSourceManager(), PP->getLangOpts()); @@ -91,7 +91,7 @@ class MacroCallback : public PPCallbacks { const char *Str = nullptr; for (size_t I = 0; I < Condition.size(); ++I) { - char C = Condition[I]; + const char C = Condition[I]; if (!Str && Start(C)) { Str = Condition.begin() + I; diff --git a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h index 24116dfcbeb4e..f018bb46e0442 100644 --- a/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h @@ -13,8 +13,12 @@ namespace clang::tidy::bugprone { -/// This check gives a warning if `true` is used in the preprocessor when not -/// defined in C. +/// 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 check identifies such cases, when ``true`` is used without being +/// defined first and also offers fix-its in some cases. /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/true-macro.html @@ -25,7 +29,7 @@ class TrueMacroCheck : public ClangTidyCheck { void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { - return !LangOpts.CPlusPlus; + return LangOpts.C99 || LangOpts.C11 || LangOpts.C17; } }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 51683093e5d23..2bbd62517980e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -94,7 +94,12 @@ New checks - New :doc:`bugprone-true-macro <clang-tidy/checks/bugprone/true-macro>` check. - This check gives a warning if ``true`` is used in the preprocessor when not defined in C. + 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 check identifies such cases, when ``true`` is used without being defined + first and also offers fix-its in some cases. 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 89ec804130b37..6dbe964270dcc 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,8 +3,9 @@ bugprone-true-macro =================== -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. +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. @@ -18,5 +19,5 @@ The following snippet returns ``1`` in C++, but ``0`` in C. #endif } -The check identifies such cases, when ``true`` is used without being defined first and also offers fix-its in -some cases. +The check identifies such cases, when ``true`` is used without being defined +first and also offers fix-its in some cases. 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 index 060f42c4aee0f..82f2e6eb20cb8 100644 --- 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 @@ -1,4 +1,6 @@ -// RUN: %check_clang_tidy -std=c99-or-later %s bugprone-true-macro %t +// RUN: %check_clang_tidy -std=c99 %s bugprone-true-macro %t +// RUN: %check_clang_tidy -std=c11 %s bugprone-true-macro %t +// RUN: %check_clang_tidy -std=c17 %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] _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits