[clang-tools-extra] [clang-tidy] warn when `true` is used as a preprocessor keyword in C (PR #128265)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Feb 22 02:48:34 PST 2025
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
More information about the cfe-commits
mailing list