llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: None (isuckatcs) <details> <summary>Changes</summary> 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. ```c++ int main() { #if true return 1; #else return 0; #endif } ``` The check identifies such cases, when `true` is used without being defined first and also offers fix-its in some cases. --- Full diff: https://github.com/llvm/llvm-project/pull/128265.diff 8 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+2) - (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1) - (added) clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp (+123) - (added) clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h (+34) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst (+22) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-defintion.c (+43) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index c5f0b5b28418f..6d993669f2303 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,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<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..4b1375db34039 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.cpp @@ -0,0 +1,123 @@ +//===--- 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/Lex/MacroInfo.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { +namespace { + +class MacroCallback : public PPCallbacks { + static constexpr const char *TrueMacroSpelling = "true"; + +public: + 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; + } + + 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()); + + if (!TrueDefined && Condition == TrueMacroSpelling) { + emitDiagnostics(ConditionRange.getBegin(), ConditionRange); + return; + } + + for (auto &&Identifier : identifiersInCondition(Condition)) { + if (!TrueDefined && Identifier == TrueMacroSpelling) { + emitDiagnostics(Loc, {}, true); + break; + } + } + } + +private: + 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) { + 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; + Preprocessor *PP; +}; +} // namespace + +void TrueMacroCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks(std::make_unique<MacroCallback>(this, 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..24116dfcbeb4e --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/TrueMacroCheck.h @@ -0,0 +1,34 @@ +//===--- 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 { + +/// 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) + : 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..51683093e5d23 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. + + 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 new file mode 100644 index 0000000000000..89ec804130b37 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/true-macro.rst @@ -0,0 +1,22 @@ +.. title:: clang-tidy - bugprone-true-macro + +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. + +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 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/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-defintion.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-defintion.c new file mode 100644 index 0000000000000..060f42c4aee0f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/true-macro-defintion.c @@ -0,0 +1,43 @@ +// 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 + +#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 `````````` </details> https://github.com/llvm/llvm-project/pull/128265 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits