[clang-tools-extra] [clang-tidy] Add bugprone-macro-condition check (PR #210768)
Richard Thomson via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 2 09:49:26 PDT 2026
https://github.com/LegalizeAdulthood updated https://github.com/llvm/llvm-project/pull/210768
>From e9682613d5d89be767e7c608d2c69c8215a65e2a Mon Sep 17 00:00:00 2001
From: Richard <legalize at xmission.com>
Date: Thu, 20 Jan 2022 01:19:19 -0700
Subject: [PATCH] [clang-tidy] Add bugprone-macro-condition check
Warns about inconsistent macro usage in preprocessor conditions.
#define USE_FOO 0
#ifdef USE_FOO
// ...no preprocessor directives testing the value of USE_FOO
#endif
Here USE_FOO is defined to a value (and furthermore defined to
evaluate to false) but the preprocessor condition only checks for
the macro being defined.
Fixes #27438
---
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
.../bugprone/MacroConditionCheck.cpp | 444 ++++++++++++++++++
.../clang-tidy/bugprone/MacroConditionCheck.h | 30 ++
clang-tools-extra/docs/ReleaseNotes.md | 5 +
.../checks/bugprone/macro-condition.md | 67 +++
.../docs/clang-tidy/checks/list.md | 1 +
.../Inputs/macro-condition-cross-file.h | 3 +
.../bugprone/macro-condition-command-line.cpp | 19 +
.../bugprone/macro-condition-cross-file.cpp | 8 +
.../checkers/bugprone/macro-condition.cpp | 180 +++++++
11 files changed, 761 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/macro-condition.md
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/macro-condition-cross-file.h
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition-command-line.cpp
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition-cross-file.cpp
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 3aa39d10ceb5dc..e39d90a1177dac 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -46,6 +46,7 @@
#include "IntegerDivisionCheck.h"
#include "InvalidEnumDefaultInitializationCheck.h"
#include "LambdaFunctionNameCheck.h"
+#include "MacroConditionCheck.h"
#include "MacroParenthesesCheck.h"
#include "MacroRepeatedSideEffectsCheck.h"
#include "MisleadingSetterOfReferenceCheck.h"
@@ -203,6 +204,8 @@ class BugproneModule : public ClangTidyModule {
"bugprone-invalid-enum-default-initialization");
CheckFactories.registerCheck<LambdaFunctionNameCheck>(
"bugprone-lambda-function-name");
+ CheckFactories.registerCheck<MacroConditionCheck>(
+ "bugprone-macro-condition");
CheckFactories.registerCheck<MacroParenthesesCheck>(
"bugprone-macro-parentheses");
CheckFactories.registerCheck<MacroRepeatedSideEffectsCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 43e85b1407f21a..7c023e7458cfd0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -50,6 +50,7 @@ add_clang_library(clangTidyBugproneModule STATIC
InfiniteLoopCheck.cpp
IntegerDivisionCheck.cpp
LambdaFunctionNameCheck.cpp
+ MacroConditionCheck.cpp
MacroParenthesesCheck.cpp
MacroRepeatedSideEffectsCheck.cpp
MisleadingSetterOfReferenceCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.cpp
new file mode 100644
index 00000000000000..2c429fcf690faa
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.cpp
@@ -0,0 +1,444 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "MacroConditionCheck.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
+#include <memory>
+#include <optional>
+#include <string>
+#include <utility>
+
+namespace clang::tidy::bugprone {
+
+namespace {
+class MacroConditionCallbacks : public PPCallbacks {
+public:
+ MacroConditionCallbacks(MacroConditionCheck *Check, const SourceManager &SM,
+ Preprocessor &PP)
+ : Check(Check), SM(SM), PP(PP) {}
+
+ void If(SourceLocation Loc, SourceRange ConditionRange,
+ ConditionValueKind ConditionValue) override;
+ void Ifdef(SourceLocation Loc, const Token &MacroNameTok,
+ const MacroDefinition &MD) override;
+ void Ifndef(SourceLocation Loc, const Token &MacroNameTok,
+ const MacroDefinition &MD) override;
+ void Elif(SourceLocation Loc, SourceRange ConditionRange,
+ ConditionValueKind ConditionValue, SourceLocation IfLoc) override;
+ void Elifdef(SourceLocation Loc, const Token &MacroNameTok,
+ const MacroDefinition &MD) override;
+ void Elifdef(SourceLocation Loc, SourceRange ConditionRange,
+ SourceLocation IfLoc) override;
+ void Elifndef(SourceLocation Loc, const Token &MacroNameTok,
+ const MacroDefinition &MD) override;
+ void Elifndef(SourceLocation Loc, SourceRange ConditionRange,
+ SourceLocation IfLoc) override;
+ void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override;
+
+private:
+ enum class ReferenceKind { Definition, NegatedDefinition, Value };
+
+ struct MacroReference {
+ std::string Name;
+ SourceLocation Loc;
+ ReferenceKind Kind;
+ };
+
+ struct MacroUsage {
+ SourceLocation DefinitionTestLoc;
+ SourceLocation ValueTestLoc;
+ bool Diagnosed = false;
+ };
+
+ using ConditionReferences = SmallVector<MacroReference, 4>;
+
+ ConditionReferences referencesInCondition(SourceRange ConditionRange) const;
+ std::optional<std::string>
+ defaultedMacroInSkippedRange(SourceRange Range) const;
+ MacroReference referenceFromRange(SourceRange Range,
+ SourceLocation Loc) const;
+ bool isIgnoredIdentifier(StringRef Name) const;
+ void processReferences(const ConditionReferences &References);
+ void processDefinitionReference(StringRef Name, SourceLocation Loc);
+ void checkReference(const MacroReference &Reference);
+
+ using FileUsages = llvm::DenseMap<FileID, MacroUsage>;
+ llvm::DenseMap<const MacroInfo *, FileUsages> MacroUsages;
+ MacroConditionCheck *Check;
+ const SourceManager &SM;
+ Preprocessor &PP;
+};
+
+} // namespace
+
+static StringRef getTokenName(const Token &Tok) {
+ if (Tok.is(tok::raw_identifier))
+ return Tok.getRawIdentifier();
+ if (const IdentifierInfo *Info = Tok.getIdentifierInfo())
+ return Info->getName();
+ return {};
+}
+
+static bool skipFunctionLikeInvocation(ArrayRef<Token> Tokens, size_t &Index) {
+ if (Index + 1 >= Tokens.size() || Tokens[Index + 1].isNot(tok::l_paren))
+ return false;
+
+ unsigned ParenthesisDepth = 0;
+ do {
+ ++Index;
+ if (Tokens[Index].is(tok::l_paren))
+ ++ParenthesisDepth;
+ else if (Tokens[Index].is(tok::r_paren))
+ --ParenthesisDepth;
+ } while (Index + 1 < Tokens.size() && ParenthesisDepth != 0);
+ return true;
+}
+
+static bool isNegatedDefined(ArrayRef<Token> Tokens, size_t Index) {
+ unsigned Negations = 0;
+ while (Index > 0) {
+ while (Index > 0 && Tokens[Index - 1].is(tok::l_paren))
+ --Index;
+ if (Index == 0 || Tokens[Index - 1].isNot(tok::exclaim))
+ break;
+ --Index;
+ ++Negations;
+ }
+ return Negations % 2 != 0;
+}
+
+static bool isStandardPredefinedMacro(StringRef Name) {
+ return Name == "__cplusplus" || Name == "__DATE__" || Name == "__FILE__" ||
+ Name == "__LINE__" || Name == "__TIME__" ||
+ Name.starts_with("__cpp_") || Name.starts_with("__STDC_") ||
+ Name.starts_with("__STDCPP_");
+}
+
+MacroConditionCallbacks::ConditionReferences
+MacroConditionCallbacks::referencesInCondition(
+ SourceRange ConditionRange) const {
+ ConditionReferences References;
+ const SourceLocation BeginLoc = SM.getExpansionLoc(ConditionRange.getBegin());
+ if (BeginLoc.isInvalid())
+ return References;
+
+ const std::pair<FileID, unsigned> Decomposed = SM.getDecomposedLoc(BeginLoc);
+ bool Invalid = false;
+ StringRef Buffer = SM.getBufferData(Decomposed.first, &Invalid);
+ if (Invalid || Decomposed.second >= Buffer.size())
+ return References;
+
+ size_t End = Decomposed.second;
+ while (End < Buffer.size()) {
+ if (Buffer[End] != '\r' && Buffer[End] != '\n') {
+ ++End;
+ continue;
+ }
+
+ const size_t Newline = End;
+ if (Newline > Decomposed.second && Buffer[Newline - 1] == '\\') {
+ if (Buffer[End] == '\r' && End + 1 < Buffer.size() &&
+ Buffer[End + 1] == '\n')
+ ++End;
+ ++End;
+ continue;
+ }
+ break;
+ }
+
+ std::string Text = Buffer.slice(Decomposed.second, End).str();
+ Lexer Lex(BeginLoc, PP.getLangOpts(), Text.data(), Text.data(),
+ Text.data() + Text.size());
+ SmallVector<Token, 16> Tokens;
+ Token Tok;
+ bool AtEnd = false;
+ do {
+ AtEnd = Lex.LexFromRawLexer(Tok);
+ if (Tok.isNot(tok::eof))
+ Tokens.push_back(Tok);
+ } while (!AtEnd);
+
+ for (size_t Index = 0; Index < Tokens.size(); ++Index) {
+ const Token &Current = Tokens[Index];
+ if (!Current.is(tok::raw_identifier))
+ continue;
+
+ StringRef Name = Current.getRawIdentifier();
+ if (Name != "defined") {
+ if (skipFunctionLikeInvocation(Tokens, Index))
+ continue;
+ if (!isIgnoredIdentifier(Name))
+ References.push_back(
+ {Name.str(), Current.getLocation(), ReferenceKind::Value});
+ continue;
+ }
+
+ const SourceLocation DefinedLoc = Current.getLocation();
+ const bool IsNegated = isNegatedDefined(Tokens, Index);
+ ++Index;
+ if (Index < Tokens.size() && Tokens[Index].is(tok::l_paren))
+ ++Index;
+ if (Index < Tokens.size() && Tokens[Index].is(tok::raw_identifier))
+ References.push_back({Tokens[Index].getRawIdentifier().str(), DefinedLoc,
+ IsNegated ? ReferenceKind::NegatedDefinition
+ : ReferenceKind::Definition});
+ }
+ return References;
+}
+
+std::optional<std::string>
+MacroConditionCallbacks::defaultedMacroInSkippedRange(SourceRange Range) const {
+ const SourceLocation BeginLoc = SM.getExpansionLoc(Range.getBegin());
+ const SourceLocation EndLoc = SM.getExpansionLoc(Range.getEnd());
+ if (BeginLoc.isInvalid() || EndLoc.isInvalid())
+ return std::nullopt;
+
+ const std::pair<FileID, unsigned> Begin = SM.getDecomposedLoc(BeginLoc);
+ const std::pair<FileID, unsigned> End = SM.getDecomposedLoc(EndLoc);
+ if (Begin.first != End.first || Begin.second >= End.second)
+ return std::nullopt;
+
+ bool Invalid = false;
+ StringRef Buffer = SM.getBufferData(Begin.first, &Invalid);
+ if (Invalid || End.second > Buffer.size())
+ return std::nullopt;
+
+ std::string Text = Buffer.slice(Begin.second, End.second).str();
+ Lexer Lex(BeginLoc, PP.getLangOpts(), Text.data(), Text.data(),
+ Text.data() + Text.size());
+ SmallVector<Token, 32> Tokens;
+ Token Tok;
+ bool AtEnd = false;
+ do {
+ AtEnd = Lex.LexFromRawLexer(Tok);
+ if (Tok.isNot(tok::eof))
+ Tokens.push_back(Tok);
+ } while (!AtEnd);
+
+ if (Tokens.size() < 3 || Tokens[0].isNot(tok::hash) ||
+ !Tokens[0].isAtStartOfLine())
+ return std::nullopt;
+
+ size_t Index = 2;
+ std::string Name;
+ StringRef Directive = getTokenName(Tokens[1]);
+ if (Directive == "ifndef") {
+ if (!Tokens[Index].is(tok::raw_identifier))
+ return std::nullopt;
+ Name = Tokens[Index++].getRawIdentifier().str();
+ } else if (Directive == "if") {
+ if (Tokens[Index++].isNot(tok::exclaim) || Index >= Tokens.size() ||
+ getTokenName(Tokens[Index++]) != "defined")
+ return std::nullopt;
+ if (Index < Tokens.size() && Tokens[Index].is(tok::l_paren))
+ ++Index;
+ if (Index >= Tokens.size() || Tokens[Index].isNot(tok::raw_identifier))
+ return std::nullopt;
+ Name = Tokens[Index++].getRawIdentifier().str();
+ if (Index < Tokens.size() && Tokens[Index].is(tok::r_paren))
+ ++Index;
+ } else {
+ return std::nullopt;
+ }
+
+ if (Index < Tokens.size() && !Tokens[Index].isAtStartOfLine())
+ return std::nullopt;
+
+ unsigned Depth = 1;
+ for (; Index + 1 < Tokens.size(); ++Index) {
+ if (Tokens[Index].isNot(tok::hash) || !Tokens[Index].isAtStartOfLine())
+ continue;
+
+ StringRef NestedDirective = getTokenName(Tokens[++Index]);
+ if (NestedDirective == "if" || NestedDirective == "ifdef" ||
+ NestedDirective == "ifndef") {
+ ++Depth;
+ continue;
+ }
+ if (NestedDirective == "endif") {
+ if (--Depth == 0)
+ break;
+ continue;
+ }
+ if (Depth != 1)
+ continue;
+ if (NestedDirective == "else" || NestedDirective.starts_with("elif"))
+ break;
+ if (NestedDirective != "define" || Index + 2 >= Tokens.size() ||
+ getTokenName(Tokens[Index + 1]) != Name ||
+ Tokens[Index + 2].isAtStartOfLine())
+ continue;
+
+ const Token &MacroName = Tokens[Index + 1];
+ const Token &Replacement = Tokens[Index + 2];
+ const SourceLocation MacroNameEnd = Lexer::getLocForEndOfToken(
+ MacroName.getLocation(), 0, SM, PP.getLangOpts());
+ if (Replacement.is(tok::l_paren) &&
+ MacroNameEnd == Replacement.getLocation())
+ continue;
+ return Name;
+ }
+ return std::nullopt;
+}
+
+MacroConditionCallbacks::MacroReference
+MacroConditionCallbacks::referenceFromRange(SourceRange Range,
+ SourceLocation Loc) const {
+ ConditionReferences References = referencesInCondition(Range);
+ if (!References.empty()) {
+ References.front().Loc = Loc;
+ References.front().Kind = ReferenceKind::Definition;
+ return std::move(References.front());
+ }
+ return {{}, Loc, ReferenceKind::Definition};
+}
+
+bool MacroConditionCallbacks::isIgnoredIdentifier(StringRef Name) const {
+ const IdentifierInfo *Info = PP.getIdentifierInfo(Name);
+ return Name == "true" || Name == "false" ||
+ Info->isCPlusPlusOperatorKeyword();
+}
+
+void MacroConditionCallbacks::processReferences(
+ const ConditionReferences &References) {
+ for (const MacroReference &Reference : References) {
+ bool IsCompoundReference = false;
+ for (const MacroReference &Other : References) {
+ if (Reference.Name == Other.Name && Reference.Kind != Other.Kind) {
+ IsCompoundReference = true;
+ break;
+ }
+ }
+ if (!IsCompoundReference)
+ checkReference(Reference);
+ }
+}
+
+void MacroConditionCallbacks::processDefinitionReference(StringRef Name,
+ SourceLocation Loc) {
+ if (!Name.empty())
+ checkReference({Name.str(), Loc, ReferenceKind::Definition});
+}
+
+void MacroConditionCallbacks::checkReference(const MacroReference &Reference) {
+ if (Reference.Kind == ReferenceKind::NegatedDefinition ||
+ isStandardPredefinedMacro(Reference.Name))
+ return;
+
+ const IdentifierInfo *Info = PP.getIdentifierInfo(Reference.Name);
+ const MacroInfo *Macro = PP.getMacroDefinition(Info).getMacroInfo();
+ if (!Macro || Macro->isBuiltinMacro() || Macro->isFunctionLike() ||
+ Macro->tokens().empty())
+ return;
+
+ const SourceLocation SpellingLoc = SM.getSpellingLoc(Reference.Loc);
+ if (SpellingLoc.isInvalid())
+ return;
+
+ MacroUsage &Usage = MacroUsages[Macro][SM.getFileID(SpellingLoc)];
+ const bool IsDefinition = Reference.Kind == ReferenceKind::Definition;
+ SourceLocation &CurrentLoc =
+ IsDefinition ? Usage.DefinitionTestLoc : Usage.ValueTestLoc;
+ const SourceLocation OtherLoc =
+ IsDefinition ? Usage.ValueTestLoc : Usage.DefinitionTestLoc;
+ if (CurrentLoc.isInvalid())
+ CurrentLoc = Reference.Loc;
+ if (Usage.Diagnosed || OtherLoc.isInvalid())
+ return;
+
+ const unsigned Kind = IsDefinition ? 0 : 1;
+ Check->diag(Reference.Loc,
+ "Macro '%0' checked here for %select{definition|value}1 after "
+ "being checked for %select{value|definition}1")
+ << Reference.Name << Kind;
+ Check->diag(OtherLoc,
+ "Macro '%0' first checked here for "
+ "%select{value|definition}1",
+ DiagnosticIDs::Note)
+ << Reference.Name << Kind;
+ Usage.Diagnosed = true;
+}
+
+void MacroConditionCallbacks::If(SourceLocation Loc, SourceRange ConditionRange,
+ ConditionValueKind ConditionValue) {
+ processReferences(referencesInCondition(ConditionRange));
+}
+
+void MacroConditionCallbacks::Ifdef(SourceLocation Loc,
+ const Token &MacroNameTok,
+ const MacroDefinition &MD) {
+ processDefinitionReference(getTokenName(MacroNameTok), Loc);
+}
+
+void MacroConditionCallbacks::Ifndef(SourceLocation, const Token &,
+ const MacroDefinition &) {}
+
+void MacroConditionCallbacks::Elif(SourceLocation Loc,
+ SourceRange ConditionRange,
+ ConditionValueKind ConditionValue,
+ SourceLocation IfLoc) {
+ processReferences(referencesInCondition(ConditionRange));
+}
+
+void MacroConditionCallbacks::Elifdef(SourceLocation Loc,
+ const Token &MacroNameTok,
+ const MacroDefinition &MD) {
+ processDefinitionReference(getTokenName(MacroNameTok), Loc);
+}
+
+void MacroConditionCallbacks::Elifdef(SourceLocation Loc,
+ SourceRange ConditionRange,
+ SourceLocation IfLoc) {
+ MacroReference Reference = referenceFromRange(ConditionRange, Loc);
+ if (!Reference.Name.empty())
+ checkReference(Reference);
+}
+
+void MacroConditionCallbacks::Elifndef(SourceLocation, const Token &,
+ const MacroDefinition &) {}
+
+void MacroConditionCallbacks::Elifndef(SourceLocation, SourceRange,
+ SourceLocation) {}
+
+void MacroConditionCallbacks::SourceRangeSkipped(SourceRange Range,
+ SourceLocation EndifLoc) {
+ std::optional<std::string> Name = defaultedMacroInSkippedRange(Range);
+ if (!Name)
+ return;
+
+ const IdentifierInfo *Info = PP.getIdentifierInfo(*Name);
+ const MacroInfo *Macro = PP.getMacroDefinition(Info).getMacroInfo();
+ auto MacroIt = MacroUsages.find(Macro);
+ if (MacroIt == MacroUsages.end())
+ return;
+
+ const SourceLocation GuardLoc = SM.getSpellingLoc(Range.getBegin());
+ const FileID GuardFile = SM.getFileID(GuardLoc);
+ auto FileIt = MacroIt->second.find(GuardFile);
+ if (FileIt == MacroIt->second.end())
+ return;
+
+ SourceLocation &DefinitionLoc = FileIt->second.DefinitionTestLoc;
+ if (DefinitionLoc.isValid() && SM.getSpellingLineNumber(DefinitionLoc) ==
+ SM.getSpellingLineNumber(GuardLoc))
+ DefinitionLoc = {};
+}
+
+void MacroConditionCheck::registerPPCallbacks(const SourceManager &SM,
+ Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) {
+ PP->addPPCallbacks(std::make_unique<MacroConditionCallbacks>(this, SM, *PP));
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.h b/clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.h
new file mode 100644
index 00000000000000..ab4fb90490268f
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.h
@@ -0,0 +1,30 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_MACROCONDITIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MACROCONDITIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Warns about inconsistent macro usage in preprocessor conditions.
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone-macro-condition.html
+class MacroConditionCheck : public ClangTidyCheck {
+public:
+ using ClangTidyCheck::ClangTidyCheck;
+
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MACROCONDITIONCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 41aa783cabd248..6fbb87b0f431c5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -129,6 +129,11 @@ infrastructure are described first, followed by tool-specific sections.
#### New checks
+- New {doc}`bugprone-macro-condition
+ <clang-tidy/checks/bugprone/macro-condition>` check.
+
+ Warns about inconsistent macro usage in preprocessor conditions.
+
- New {doc}`llvm-invalid-regex-pattern
<clang-tidy/checks/llvm/invalid-regex-pattern>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/macro-condition.md b/clang-tools-extra/docs/clang-tidy/checks/bugprone/macro-condition.md
new file mode 100644
index 00000000000000..042e598755fc74
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/macro-condition.md
@@ -0,0 +1,67 @@
+```{title} clang-tidy - bugprone-macro-condition
+```
+
+# bugprone-macro-condition
+
+Warns about inconsistent macro usage in preprocessor conditions.
+
+Given the following code:
+
+```c++
+#define USE_FOO 0
+// ...
+#if defined(USE_FOO)
+ // ...
+#endif
+// ...
+#if USE_FOO
+ // ...
+#endif
+```
+
+`USE_FOO` is checked for definition in one condition and for value in
+another. Was the intention to evaluate `USE_FOO` for a `true` expression,
+or was the intention to merely check whether the macro was defined?
+
+The check warns when the same active macro definition is tested both for
+definition and for value within the same source file. Conditional uses from
+different files are not compared. Only positive definition tests such as
+``#ifdef FEATURE`` and ``defined(FEATURE)`` qualify. Negated tests such as
+``#ifndef FEATURE`` and ``!defined(FEATURE)`` are ignored. It does not warn
+merely because a macro with a replacement value is tested for definition.
+Tests of undefined macros for value are handled by `-Wundef`.
+
+Compound conditions that test the same macro for both definition and value
+are treated as one coherent test and ignored. Standard predefined macros,
+including `__cplusplus` and the `__cpp_*`, `__STDC_*`, and `__STDCPP_*`
+families, are also ignored.
+
+A guard that supplies a default value for a macro is not considered a
+definition test. For example, this does not qualify for a warning:
+
+```c++
+#ifndef FEATURE
+#define FEATURE 0
+#endif
+
+#if FEATURE
+ // ...
+#endif
+```
+
+The same exception applies when the guard is written as
+``#if !defined(FEATURE)``.
+
+No fixes are offered because the intended semantics are ambiguous.
+
+To resolve a warning, decide which property of the macro is important:
+
+- If the macro's value is important, keep the value in its definition and
+ refactor definition tests to test the value, for example with
+ ``#if USE_FOO`` or an explicit comparison.
+- If the macro's presence or absence is important, make it a presence-only
+ macro and refactor value tests to use ``defined(USE_FOO)`` or
+ ``!defined(USE_FOO)`` consistently.
+
+Function-like macros are ignored, including identifiers passed in their
+argument lists.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.md b/clang-tools-extra/docs/clang-tidy/checks/list.md
index a74a26e691053d..8fd3d7408c9f5c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.md
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.md
@@ -118,6 +118,7 @@ readability/*
| {doc}`bugprone-integer-division <bugprone/integer-division>` | |
| {doc}`bugprone-invalid-enum-default-initialization <bugprone/invalid-enum-default-initialization>` | |
| {doc}`bugprone-lambda-function-name <bugprone/lambda-function-name>` | |
+| {doc}`bugprone-macro-condition <bugprone/macro-condition>` | |
| {doc}`bugprone-macro-parentheses <bugprone/macro-parentheses>` | Yes |
| {doc}`bugprone-macro-repeated-side-effects <bugprone/macro-repeated-side-effects>` | |
| {doc}`bugprone-misleading-setter-of-reference <bugprone/misleading-setter-of-reference>` | |
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/macro-condition-cross-file.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/macro-condition-cross-file.h
new file mode 100644
index 00000000000000..13418c41a6e8fb
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/macro-condition-cross-file.h
@@ -0,0 +1,3 @@
+#ifndef CROSS_FILE_MACRO
+#define CROSS_FILE_MACRO 1
+#endif
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition-command-line.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition-command-line.cpp
new file mode 100644
index 00000000000000..6ceb899592473e
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition-command-line.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy -check-suffix=DEFINED %s \
+// RUN: bugprone-macro-condition %t -- -- -DCOMMAND_LINE_MACRO=0
+// RUN: clang-tidy %s -checks=-*,bugprone-macro-condition -- \
+// RUN: -UCOMMAND_LINE_MACRO | count 0
+
+// With -UCOMMAND_LINE_MACRO, these conditions are equivalent to:
+//
+// #undef COMMAND_LINE_MACRO
+
+// With -DCOMMAND_LINE_MACRO=0, they are equivalent to:
+//
+// #define COMMAND_LINE_MACRO 0
+#ifdef COMMAND_LINE_MACRO
+#endif
+
+#if COMMAND_LINE_MACRO
+// CHECK-MESSAGES-DEFINED: :[[@LINE-1]]:5: warning: Macro 'COMMAND_LINE_MACRO' checked here for value after being checked for definition
+// CHECK-MESSAGES-DEFINED: :[[@LINE-5]]:2: note: Macro 'COMMAND_LINE_MACRO' first checked here for definition
+#endif
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition-cross-file.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition-cross-file.cpp
new file mode 100644
index 00000000000000..4fbb2e8fc29b36
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition-cross-file.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-tidy %s -checks=-*,bugprone-macro-condition -- -I %S | count 0
+
+#define CROSS_FILE_MACRO 1
+#include "Inputs/macro-condition-cross-file.h"
+
+#if CROSS_FILE_MACRO
+#endif
+
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition.cpp
new file mode 100644
index 00000000000000..f7d9a2974a2b9c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition.cpp
@@ -0,0 +1,180 @@
+// RUN: %check_clang_tidy %s bugprone-macro-condition %t
+
+#define USE_FOO 0
+
+#if defined(USE_FOO)
+void f()
+{
+ extern void foo();
+ foo();
+}
+#endif
+
+#define VALUE_DEFINED 42
+#ifndef VALUE_DEFINED
+#endif
+// CHECK-MESSAGES-NOT: warning: Macro 'VALUE_DEFINED'
+
+#if 0
+#elif OTHER_MACRO
+#elifdef OTHER_MACRO2
+#else
+#endif
+
+#if !defined(USE_FOO)
+void f2()
+{
+ extern void notFoo();
+ notFoo();
+}
+#endif
+
+#ifdef USE_FOO
+void f3()
+{
+ extern void foo();
+ foo();
+}
+#endif
+
+#ifndef USE_FOO
+void f4()
+{
+ extern void notFoo();
+ notFoo();
+}
+#endif
+
+#if 0
+#elif defined(USE_FOO)
+void f5()
+{
+ extern void foo();
+ foo();
+}
+#endif
+
+// CHECK-MESSAGES-NOT: warning: Macro 'USE_FOO'
+// CHECK-MESSAGES-NOT: warning: Undefined macro 'OTHER_MACRO'
+
+#define USE_GRONK 0
+#ifdef USE_GRONK
+#if USE_GRONK
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Macro 'USE_GRONK' checked here for value after being checked for definition
+// CHECK-MESSAGES: :[[@LINE-3]]:2: note: Macro 'USE_GRONK' first checked here for definition
+void f6()
+{
+ extern void foo();
+ foo();
+}
+#endif
+#endif
+
+#define VALUE_FIRST 0
+#if VALUE_FIRST
+#endif
+#ifndef VALUE_FIRST
+void f7()
+{
+ extern void foo();
+ foo();
+}
+#endif
+// CHECK-MESSAGES-NOT: warning: Macro 'VALUE_FIRST'
+
+#define REQUIRED_OPTION 1
+#define REQUIRED_NAME required_namespace
+#if !defined(REQUIRED_OPTION) || \
+ !defined(REQUIRED_NAME)
+#error Required options are not configured.
+#endif
+#if defined(__cplusplus) && REQUIRED_OPTION == 1
+#endif
+// CHECK-MESSAGES-NOT: warning: Macro 'REQUIRED_OPTION'
+
+#define POSITIVE_DEFINITION 1
+#define OTHER_POSITIVE_DEFINITION 1
+#if defined(POSITIVE_DEFINITION) || defined(OTHER_POSITIVE_DEFINITION)
+#endif
+#if POSITIVE_DEFINITION
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Macro 'POSITIVE_DEFINITION' checked here for value after being checked for definition
+// CHECK-MESSAGES: :[[@LINE-4]]:5: note: Macro 'POSITIVE_DEFINITION' first checked here for definition
+#endif
+
+#define SAME_CONDITION 0
+#if defined(SAME_CONDITION) && SAME_CONDITION
+void f8()
+{
+ extern void foo();
+ foo();
+}
+#endif
+
+#if __has_include(<sys/file.h>)
+#include <sys/file.h>
+#endif
+// CHECK-MESSAGES-NOT: warning: Undefined macro 'sys' checked here for value
+// CHECK-MESSAGES-NOT: warning: Undefined macro 'file' checked here for value
+// CHECK-MESSAGES-NOT: warning: Undefined macro 'h' checked here for value
+
+#if __has_builtin(__builtin_trap)
+#endif
+// CHECK-MESSAGES-NOT: warning: Undefined macro '__builtin_trap' checked here for value
+
+#if __has_cpp_attribute(gnu::always_inline)
+#endif
+// CHECK-MESSAGES-NOT: warning: Undefined macro 'gnu' checked here for value
+// CHECK-MESSAGES-NOT: warning: Undefined macro 'always_inline' checked here for value
+
+#define ALWAYS_TRUE(x) 1
+#if ALWAYS_TRUE(not_a_macro)
+#endif
+// CHECK-MESSAGES-NOT: warning: Undefined macro 'not_a_macro' checked here for value
+
+#ifdef ALWAYS_TRUE
+#endif
+// CHECK-MESSAGES-NOT: warning: Macro 'ALWAYS_TRUE' defined here with a value and checked for definition
+
+#define GUARDED 1
+#if defined(GUARDED)
+#undef GUARDED
+#if GUARDED
+#endif
+#endif
+// CHECK-MESSAGES-NOT: warning: {{.*}}'GUARDED'
+
+#define GUARDED_CONJUNCTION 1
+#ifdef GUARDED_CONJUNCTION
+#endif
+#if defined(GUARDED_CONJUNCTION) && GUARDED_CONJUNCTION
+#endif
+
+#define GUARDED_DISJUNCTION 1
+#ifdef GUARDED_DISJUNCTION
+#endif
+#if !defined(GUARDED_DISJUNCTION) || GUARDED_DISJUNCTION
+#endif
+
+#ifdef __cplusplus
+#endif
+#if __cplusplus >= 201103L
+#endif
+
+#ifdef __STDC_HOSTED__
+#endif
+#if __STDC_HOSTED__
+#endif
+
+#define DEFAULT_IFNDEF 1
+#ifndef DEFAULT_IFNDEF
+#define DEFAULT_IFNDEF 0
+#endif
+#if DEFAULT_IFNDEF
+#endif
+
+#define DEFAULT_NOT_DEFINED 1
+#if !defined(DEFAULT_NOT_DEFINED)
+#define DEFAULT_NOT_DEFINED 0
+#endif
+#if DEFAULT_NOT_DEFINED
+#endif
More information about the cfe-commits
mailing list