[clang-tools-extra] [clang-tidy] Add bugprone-macro-condition check (PR #210768)
Richard Thomson via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 8 11:52:51 PDT 2026
https://github.com/LegalizeAdulthood updated https://github.com/llvm/llvm-project/pull/210768
>From de4e4daa7d83fc9b974be4631c64bfc79cdc190e 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 | 617 ++++++++++++++++++
.../clang-tidy/bugprone/MacroConditionCheck.h | 30 +
.../clang-tidy/modernize/MacroToEnumCheck.cpp | 22 +-
.../AvoidUnconditionalPreprocessorIfCheck.cpp | 22 +-
.../UseConcisePreprocessorDirectivesCheck.cpp | 55 +-
.../clang-tidy/utils/LexerUtils.cpp | 40 ++
.../clang-tidy/utils/LexerUtils.h | 8 +
clang-tools-extra/docs/ReleaseNotes.md | 5 +
.../checks/bugprone/macro-condition.md | 100 +++
.../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 | 213 ++++++
.../unittests/clang-tidy/LexerUtilsTest.cpp | 24 +
17 files changed, 1110 insertions(+), 61 deletions(-)
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..bf5977ca15cc3e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/MacroConditionCheck.cpp
@@ -0,0 +1,617 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "../utils/LexerUtils.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/STLExtras.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 Else(SourceLocation Loc, SourceLocation IfLoc) override;
+ void Endif(SourceLocation Loc, 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;
+ };
+
+ struct DefinitionGuard {
+ std::string Name;
+ FileID File;
+ };
+
+ using ConditionReferences = SmallVector<MacroReference, 4>;
+ using DefinitionGuards = SmallVector<DefinitionGuard, 2>;
+
+ ConditionReferences referencesInCondition(SourceRange ConditionRange) const;
+ DefinitionGuards
+ definitionGuardsInCondition(SourceRange ConditionRange) const;
+ std::optional<std::string>
+ defaultedMacroInSkippedRange(SourceRange Range) const;
+ MacroReference referenceFromRange(SourceRange Range,
+ SourceLocation Loc) const;
+ bool branchUnconditionallyErrors(SourceLocation Loc) const;
+ bool isEnclosedByDefinitionGuard(const MacroReference &Reference) const;
+ void beginConditional(DefinitionGuards Guards);
+ void changeConditional(DefinitionGuards Guards);
+ 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;
+ SmallVector<DefinitionGuards, 8> ConditionalGuards;
+ MacroConditionCheck *Check;
+ const SourceManager &SM;
+ Preprocessor &PP;
+};
+
+} // namespace
+
+using utils::lexer::getTokenName;
+
+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 std::vector<Token> Tokens = utils::lexer::getRawTokens(
+ CharSourceRange::getTokenRange(ConditionRange), SM, PP.getLangOpts());
+
+ 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;
+}
+
+static bool
+parseDefinitionGuardExpression(ArrayRef<Token> Tokens, size_t &Index,
+ SmallVectorImpl<std::string> &Guards);
+
+static bool parseDefinitionGuardPrimary(ArrayRef<Token> Tokens, size_t &Index,
+ SmallVectorImpl<std::string> &Guards) {
+ if (Index >= Tokens.size())
+ return false;
+
+ if (Tokens[Index].is(tok::l_paren)) {
+ ++Index;
+ if (!parseDefinitionGuardExpression(Tokens, Index, Guards) ||
+ Index >= Tokens.size() || Tokens[Index].isNot(tok::r_paren))
+ return false;
+ ++Index;
+ return true;
+ }
+
+ if (getTokenName(Tokens[Index]) != "defined")
+ return false;
+ ++Index;
+
+ const bool Parenthesized =
+ Index < Tokens.size() && Tokens[Index].is(tok::l_paren);
+ if (Parenthesized)
+ ++Index;
+ if (Index >= Tokens.size() || Tokens[Index].isNot(tok::raw_identifier))
+ return false;
+ Guards.push_back(Tokens[Index++].getRawIdentifier().str());
+ if (Parenthesized) {
+ if (Index >= Tokens.size() || Tokens[Index].isNot(tok::r_paren))
+ return false;
+ ++Index;
+ }
+ return true;
+}
+
+static bool
+parseDefinitionGuardExpression(ArrayRef<Token> Tokens, size_t &Index,
+ SmallVectorImpl<std::string> &Guards) {
+ if (!parseDefinitionGuardPrimary(Tokens, Index, Guards))
+ return false;
+ while (Index < Tokens.size() && Tokens[Index].is(tok::ampamp)) {
+ ++Index;
+ if (!parseDefinitionGuardPrimary(Tokens, Index, Guards))
+ return false;
+ }
+ return true;
+}
+
+MacroConditionCallbacks::DefinitionGuards
+MacroConditionCallbacks::definitionGuardsInCondition(
+ SourceRange ConditionRange) const {
+ const std::vector<Token> Tokens = utils::lexer::getRawTokens(
+ CharSourceRange::getTokenRange(ConditionRange), SM, PP.getLangOpts());
+ SmallVector<std::string, 2> Names;
+ size_t Index = 0;
+ if (!parseDefinitionGuardExpression(Tokens, Index, Names) ||
+ Index != Tokens.size())
+ return {};
+
+ DefinitionGuards Guards;
+ const FileID File =
+ SM.getFileID(SM.getSpellingLoc(ConditionRange.getBegin()));
+ for (std::string &Name : Names)
+ Guards.push_back({std::move(Name), File});
+ return Guards;
+}
+
+bool MacroConditionCallbacks::branchUnconditionallyErrors(
+ SourceLocation Loc) const {
+ const SourceLocation BeginLoc = SM.getExpansionLoc(Loc);
+ if (BeginLoc.isInvalid())
+ return false;
+
+ 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 false;
+
+ size_t Body = Decomposed.second;
+ while (Body < Buffer.size()) {
+ if (Buffer[Body] != '\r' && Buffer[Body] != '\n') {
+ ++Body;
+ continue;
+ }
+ if (Body > Decomposed.second && Buffer[Body - 1] == '\\') {
+ if (Buffer[Body] == '\r' && Body + 1 < Buffer.size() &&
+ Buffer[Body + 1] == '\n')
+ ++Body;
+ ++Body;
+ continue;
+ }
+ break;
+ }
+ while (Body < Buffer.size() && (Buffer[Body] == '\r' || Buffer[Body] == '\n'))
+ ++Body;
+
+ const SourceLocation BodyLoc =
+ SM.getLocForStartOfFile(Decomposed.first).getLocWithOffset(Body);
+ std::string Text = Buffer.drop_front(Body).str();
+ Lexer Lex(BodyLoc, PP.getLangOpts(), Text.data(), Text.data(),
+ Text.data() + Text.size());
+ unsigned Depth = 1;
+ Token Tok;
+ while (!Lex.LexFromRawLexer(Tok)) {
+ if (Tok.isNot(tok::hash) || !Tok.isAtStartOfLine())
+ continue;
+
+ Token DirectiveTok;
+ if (Lex.LexFromRawLexer(DirectiveTok))
+ return false;
+ StringRef Directive = getTokenName(DirectiveTok);
+ if (Directive == "if" || Directive == "ifdef" || Directive == "ifndef") {
+ ++Depth;
+ continue;
+ }
+ if (Directive == "endif") {
+ if (--Depth == 0)
+ return false;
+ continue;
+ }
+ if (Depth != 1)
+ continue;
+ if (Directive == "else" || Directive.starts_with("elif"))
+ return false;
+ if (Directive == "error")
+ return true;
+ }
+ return false;
+}
+
+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))
+ return std::nullopt;
+ ++Index;
+ if (Index >= Tokens.size() || getTokenName(Tokens[Index]) != "defined")
+ return std::nullopt;
+ ++Index;
+ 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();
+}
+
+bool MacroConditionCallbacks::isEnclosedByDefinitionGuard(
+ const MacroReference &Reference) const {
+ const FileID File = SM.getFileID(SM.getSpellingLoc(Reference.Loc));
+ for (const DefinitionGuards &Guards : reverse(ConditionalGuards)) {
+ for (const DefinitionGuard &Guard : Guards)
+ if (Guard.File == File && Guard.Name == Reference.Name)
+ return true;
+ }
+ return false;
+}
+
+void MacroConditionCallbacks::beginConditional(DefinitionGuards Guards) {
+ ConditionalGuards.push_back(std::move(Guards));
+}
+
+void MacroConditionCallbacks::changeConditional(DefinitionGuards Guards) {
+ if (!ConditionalGuards.empty())
+ ConditionalGuards.back() = std::move(Guards);
+}
+
+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) ||
+ (Reference.Kind == ReferenceKind::Value &&
+ isEnclosedByDefinitionGuard(Reference)))
+ 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) {
+ if (!branchUnconditionallyErrors(Loc))
+ processReferences(referencesInCondition(ConditionRange));
+ beginConditional(definitionGuardsInCondition(ConditionRange));
+}
+
+void MacroConditionCallbacks::Ifdef(SourceLocation Loc,
+ const Token &MacroNameTok,
+ const MacroDefinition &MD) {
+ const StringRef Name = getTokenName(MacroNameTok);
+ if (!branchUnconditionallyErrors(Loc))
+ processDefinitionReference(Name, Loc);
+ DefinitionGuards Guards;
+ if (!Name.empty())
+ Guards.push_back({Name.str(), SM.getFileID(SM.getSpellingLoc(
+ MacroNameTok.getLocation()))});
+ beginConditional(std::move(Guards));
+}
+
+void MacroConditionCallbacks::Ifndef(SourceLocation, const Token &,
+ const MacroDefinition &) {
+ beginConditional({});
+}
+
+void MacroConditionCallbacks::Elif(SourceLocation Loc,
+ SourceRange ConditionRange,
+ ConditionValueKind ConditionValue,
+ SourceLocation IfLoc) {
+ changeConditional({});
+ if (!branchUnconditionallyErrors(Loc))
+ processReferences(referencesInCondition(ConditionRange));
+ changeConditional(definitionGuardsInCondition(ConditionRange));
+}
+
+void MacroConditionCallbacks::Elifdef(SourceLocation Loc,
+ const Token &MacroNameTok,
+ const MacroDefinition &MD) {
+ changeConditional({});
+ const StringRef Name = getTokenName(MacroNameTok);
+ if (!branchUnconditionallyErrors(Loc))
+ processDefinitionReference(Name, Loc);
+ DefinitionGuards Guards;
+ if (!Name.empty())
+ Guards.push_back({Name.str(), SM.getFileID(SM.getSpellingLoc(
+ MacroNameTok.getLocation()))});
+ changeConditional(std::move(Guards));
+}
+
+void MacroConditionCallbacks::Elifdef(SourceLocation Loc,
+ SourceRange ConditionRange,
+ SourceLocation IfLoc) {
+ changeConditional({});
+ MacroReference Reference = referenceFromRange(ConditionRange, Loc);
+ if (!Reference.Name.empty() && !branchUnconditionallyErrors(Loc))
+ checkReference(Reference);
+ DefinitionGuards Guards;
+ if (!Reference.Name.empty())
+ Guards.push_back({std::move(Reference.Name),
+ SM.getFileID(SM.getSpellingLoc(Reference.Loc))});
+ changeConditional(std::move(Guards));
+}
+
+void MacroConditionCallbacks::Elifndef(SourceLocation, const Token &,
+ const MacroDefinition &) {
+ changeConditional({});
+}
+
+void MacroConditionCallbacks::Elifndef(SourceLocation, SourceRange,
+ SourceLocation) {
+ changeConditional({});
+}
+
+void MacroConditionCallbacks::Else(SourceLocation, SourceLocation) {
+ changeConditional({});
+}
+
+void MacroConditionCallbacks::Endif(SourceLocation, SourceLocation) {
+ if (!ConditionalGuards.empty())
+ ConditionalGuards.pop_back();
+}
+
+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();
+ const auto MacroIt = MacroUsages.find(Macro);
+ if (MacroIt == MacroUsages.end())
+ return;
+
+ const SourceLocation GuardLoc = SM.getSpellingLoc(Range.getBegin());
+ const FileID GuardFile = SM.getFileID(GuardLoc);
+ const 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/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
index bef2e3cca57a77..2b5b8fe0e07e7b 100644
--- a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "MacroToEnumCheck.h"
+#include "../utils/LexerUtils.h"
#include "IntegralLiteralExpressionMatcher.h"
#include "clang/AST/ASTContext.h"
@@ -19,6 +20,8 @@
namespace clang::tidy::modernize {
+using utils::lexer::getTokenName;
+
static bool hasOnlyComments(SourceLocation Loc, const LangOptions &Options,
StringRef Text) {
// Use a lexer to look for tokens; if we find something other than a single
@@ -72,11 +75,6 @@ static bool hasOnlyComments(SourceLocation Loc, const LangOptions &Options,
return true;
}
-static StringRef getTokenName(const Token &Tok) {
- return Tok.is(tok::raw_identifier) ? Tok.getRawIdentifier()
- : Tok.getIdentifierInfo()->getName();
-}
-
namespace {
struct EnumMacro {
@@ -258,17 +256,9 @@ void MacroToEnumCallbacks::conditionStart(const SourceLocation &Loc) {
}
void MacroToEnumCallbacks::checkCondition(SourceRange Range) {
- const CharSourceRange CharRange = Lexer::makeFileCharRange(
- CharSourceRange::getTokenRange(Range), SM, LangOpts);
- std::string Text = Lexer::getSourceText(CharRange, SM, LangOpts).str();
- Lexer Lex(CharRange.getBegin(), LangOpts, Text.data(), Text.data(),
- Text.data() + Text.size());
- Token Tok;
- bool End = false;
- while (!End) {
- End = Lex.LexFromRawLexer(Tok);
- if (Tok.is(tok::raw_identifier) &&
- Tok.getRawIdentifier().str() != "defined")
+ for (const Token &Tok : utils::lexer::getRawTokens(
+ CharSourceRange::getTokenRange(Range), SM, LangOpts)) {
+ if (Tok.is(tok::raw_identifier) && getTokenName(Tok) != "defined")
checkName(Tok);
}
}
diff --git a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
index a962c4659cfbf0..0519184fe15943 100644
--- a/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/AvoidUnconditionalPreprocessorIfCheck.cpp
@@ -44,26 +44,14 @@ struct AvoidUnconditionalPreprocessorIfPPCallbacks : public PPCallbacks {
if (Loc.isMacroID())
return false;
- Token Tok;
- if (Lexer::getRawToken(Loc, Tok, SM, LangOpts, true)) {
- std::optional<Token> TokOpt =
- utils::lexer::findNextTokenSkippingComments(Loc, SM, LangOpts);
- if (!TokOpt || TokOpt->getLocation().isMacroID())
- return false;
- Tok = *TokOpt;
- }
-
- while (Tok.getLocation() <= ConditionRange.getEnd()) {
+ const std::vector<Token> Tokens = utils::lexer::getRawTokens(
+ CharSourceRange::getTokenRange(ConditionRange), SM, LangOpts);
+ if (Tokens.empty())
+ return false;
+ for (const Token &Tok : Tokens)
if (!isImmutableToken(Tok))
return false;
- std::optional<Token> TokOpt = utils::lexer::findNextTokenSkippingComments(
- Tok.getLocation(), SM, LangOpts);
- if (!TokOpt || TokOpt->getLocation().isMacroID())
- return false;
- Tok = *TokOpt;
- }
-
return true;
}
diff --git a/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.cpp
index c75243784d5bef..ebc6c7c5ba574e 100644
--- a/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UseConcisePreprocessorDirectivesCheck.cpp
@@ -7,8 +7,8 @@
//===----------------------------------------------------------------------===//
#include "UseConcisePreprocessorDirectivesCheck.h"
+#include "../utils/LexerUtils.h"
#include "clang/Basic/TokenKinds.h"
-#include "clang/Lex/Lexer.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
@@ -16,6 +16,8 @@
namespace clang::tidy::readability {
+using utils::lexer::getTokenName;
+
namespace {
class IfPreprocessorCallbacks final : public PPCallbacks {
@@ -37,50 +39,47 @@ class IfPreprocessorCallbacks final : public PPCallbacks {
private:
void impl(SourceLocation DirectiveLoc, SourceRange ConditionRange,
const std::array<StringRef, 2> &Replacements) {
- // Lexer requires its input range to be null-terminated.
- SmallString<128> Condition =
- Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange),
- PP.getSourceManager(), PP.getLangOpts());
- Condition.push_back('\0');
- Lexer Lex(DirectiveLoc, PP.getLangOpts(), Condition.data(),
- Condition.data(), Condition.data() + Condition.size() - 1);
- Token Tok;
+ const std::vector<Token> Tokens = utils::lexer::getRawTokens(
+ CharSourceRange::getTokenRange(ConditionRange), PP.getSourceManager(),
+ PP.getLangOpts());
bool Inverted = false; // The inverted form of #*def is #*ndef.
std::size_t ParensNestingDepth = 0;
- for (;;) {
- if (Lex.LexFromRawLexer(Tok))
- return;
-
+ std::size_t Index = 0;
+ while (Index < Tokens.size()) {
+ const Token &Tok = Tokens[Index];
if (Tok.is(tok::TokenKind::exclaim) ||
(PP.getLangOpts().CPlusPlus &&
Tok.is(tok::TokenKind::raw_identifier) &&
- Tok.getRawIdentifier() == "not"))
+ getTokenName(Tok) == "not")) {
Inverted = !Inverted;
- else if (Tok.is(tok::TokenKind::l_paren))
+ ++Index;
+ } else if (Tok.is(tok::TokenKind::l_paren)) {
++ParensNestingDepth;
- else
+ ++Index;
+ } else {
break;
+ }
}
- if (Tok.isNot(tok::TokenKind::raw_identifier) ||
- Tok.getRawIdentifier() != "defined")
+ if (Index >= Tokens.size() ||
+ Tokens[Index].isNot(tok::TokenKind::raw_identifier) ||
+ getTokenName(Tokens[Index]) != "defined")
return;
+ ++Index;
- bool NoMoreTokens = Lex.LexFromRawLexer(Tok);
- if (Tok.is(tok::TokenKind::l_paren)) {
- if (NoMoreTokens)
- return;
+ if (Index < Tokens.size() && Tokens[Index].is(tok::TokenKind::l_paren)) {
++ParensNestingDepth;
- NoMoreTokens = Lex.LexFromRawLexer(Tok);
+ ++Index;
}
- if (Tok.isNot(tok::TokenKind::raw_identifier))
+ if (Index >= Tokens.size() ||
+ Tokens[Index].isNot(tok::TokenKind::raw_identifier))
return;
- const StringRef Macro = Tok.getRawIdentifier();
+ const StringRef Macro = getTokenName(Tokens[Index++]);
- while (!NoMoreTokens) {
- NoMoreTokens = Lex.LexFromRawLexer(Tok);
- if (Tok.isNot(tok::TokenKind::r_paren))
+ while (Index < Tokens.size()) {
+ if (Tokens[Index++].isNot(tok::TokenKind::r_paren) ||
+ ParensNestingDepth == 0)
return;
--ParensNestingDepth;
}
diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
index d83713f06b8003..c3d907c9ca6574 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -14,6 +14,46 @@
namespace clang::tidy::utils::lexer {
+StringRef getTokenName(const Token &Tok) {
+ if (Tok.is(tok::raw_identifier))
+ return Tok.getRawIdentifier();
+ if (const IdentifierInfo *Info = Tok.getIdentifierInfo())
+ return Info->getName();
+ return {};
+}
+
+std::vector<Token> getRawTokens(CharSourceRange Range, const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ if (Range.isInvalid())
+ return {};
+
+ const CharSourceRange FileRange =
+ Lexer::makeFileCharRange(Range, SM, LangOpts);
+ if (FileRange.isInvalid())
+ return {};
+
+ bool Invalid = false;
+ const StringRef Text =
+ Lexer::getSourceText(FileRange, SM, LangOpts, &Invalid);
+ if (Invalid || Text.empty())
+ return {};
+
+ const auto [File, BeginOffset] = SM.getDecomposedLoc(FileRange.getBegin());
+ const StringRef Buffer = SM.getBufferData(File, &Invalid);
+ if (Invalid || BeginOffset + Text.size() > Buffer.size())
+ return {};
+
+ Lexer RawLexer(SM.getLocForStartOfFile(File), LangOpts, Buffer.begin(),
+ Buffer.begin() + BeginOffset, Buffer.end());
+ const size_t EndOffset = BeginOffset + Text.size();
+ std::vector<Token> Tokens;
+ Token Tok;
+ while (!RawLexer.LexFromRawLexer(Tok) && Tok.isNot(tok::eof) &&
+ SM.getFileOffset(Tok.getLocation()) < EndOffset)
+ Tokens.push_back(Tok);
+ return Tokens;
+}
+
std::pair<std::optional<Token>, SourceLocation>
getPreviousTokenAndStart(SourceLocation Location, const SourceManager &SM,
const LangOptions &LangOpts, bool SkipComments) {
diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.h b/clang-tools-extra/clang-tidy/utils/LexerUtils.h
index 9b2daf74965e41..1d820ad92652a1 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.h
@@ -23,6 +23,14 @@ class Stmt;
namespace tidy::utils::lexer {
+/// Returns the spelling of an identifier token.
+StringRef getTokenName(const Token &Tok);
+
+/// Returns the raw tokens in \p Range. The returned tokens refer to the
+/// source manager's backing buffer.
+std::vector<Token> getRawTokens(CharSourceRange Range, const SourceManager &SM,
+ const LangOptions &LangOpts);
+
/// Returns previous token or ``std::nullopt`` if not found.
std::optional<Token> getPreviousToken(SourceLocation Location,
const SourceManager &SM,
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 0207a72fc6cce0..de5d3c1e283201 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -141,6 +141,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..a3f6d2286e3632
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/macro-condition.md
@@ -0,0 +1,100 @@
+```{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 compares uses only when they refer to the same active definition
+of an object-like macro with a nonempty replacement list and occur in the
+same physical source file. It emits at most one warning for each macro
+definition in each file. Merely defining a macro with a value and testing
+its definition does not produce a warning by itself.
+
+## Excluded scenarios
+
+The check excludes the following scenarios:
+
+- Uses in different physical source files are not compared.
+- Uses resolving to different active macro definitions are not compared.
+ Consequently, uses separated by ``#undef`` or a redefinition are not
+ combined.
+- Undefined macros, including macros undefined with command-line ``-U``,
+ are ignored. Value tests of undefined macros are handled by `-Wundef`.
+- Function-like macros and function-like invocations are ignored. This
+ includes preprocessing operators such as `__has_builtin`,
+ `__has_include`, and `__has_cpp_attribute`, as well as identifiers in
+ their argument lists.
+- Compiler-provided builtin macros are ignored.
+- Standard predefined macros are ignored. These are `__cplusplus`,
+ `__DATE__`, `__FILE__`, `__LINE__`, `__TIME__`, and names beginning with
+ `__cpp_`, `__STDC_`, or `__STDCPP_`.
+- Object-like macros with empty replacement lists are ignored.
+- Negated definition tests are ignored. These include ``#ifndef``,
+ ``#elifndef``, and ``!defined(FEATURE)``.
+- A compound condition that tests the same macro for both definition and
+ value is treated as one coherent test and ignored.
+- A value test nested in a same-file positive definition guard is ignored.
+ Qualifying guards are ``#ifdef``, ``#elifdef``, and ``#if`` or ``#elif``
+ expressions consisting solely of positive definition tests joined by
+ ``&&``. Disjunctions and ``#else`` branches do not establish a guard.
+- A guard that supplies a default value for its macro is not considered a
+ definition test. This applies to both ``#ifndef FEATURE`` and
+ ``#if !defined(FEATURE)`` forms.
+- A condition whose branch contains a top-level ``#error`` directive is
+ treated as configuration validation and ignored. An ``#error`` inside a
+ nested conditional is not unconditional and does not suppress a warning.
+- Non-macro language tokens such as `true`, `false`, and C++ operator
+ keywords are not considered macro references.
+
+For example, a value test nested inside a positive definition guard does
+not qualify for a warning:
+
+```c++
+#ifdef FEATURE
+#if FEATURE >= 2
+ // ...
+#endif
+#endif
+```
+
+A guard that supplies a default value also does not qualify:
+
+```c++
+#ifndef FEATURE
+#define FEATURE 0
+#endif
+
+#if FEATURE
+ // ...
+#endif
+```
+
+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.
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..f48b4555a04b93
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/macro-condition.cpp
@@ -0,0 +1,213 @@
+// 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
+void f6()
+{
+ extern void foo();
+ foo();
+}
+#endif
+#endif
+// CHECK-MESSAGES-NOT: warning: Macro 'USE_GRONK'
+
+#define ENCLOSED_DEFINED 1
+#if defined(ENCLOSED_DEFINED)
+#if ENCLOSED_DEFINED
+#endif
+#endif
+// CHECK-MESSAGES-NOT: warning: Macro 'ENCLOSED_DEFINED'
+
+#define ENCLOSED_CONJUNCTION 1
+#define ENCLOSING_SUPPORT 1
+#if defined(ENCLOSED_CONJUNCTION) && defined(ENCLOSING_SUPPORT)
+#if ENCLOSED_CONJUNCTION
+#endif
+#endif
+// CHECK-MESSAGES-NOT: warning: Macro 'ENCLOSED_CONJUNCTION'
+
+#define VALIDATED_VALUE 100
+#ifdef VALIDATED_VALUE
+#endif
+#if VALIDATED_VALUE < 10
+#error VALIDATED_VALUE is too small.
+#endif
+// CHECK-MESSAGES-NOT: warning: Macro 'VALIDATED_VALUE'
+
+#define CONDITIONAL_ERROR 1
+#ifdef CONDITIONAL_ERROR
+#endif
+#if CONDITIONAL_ERROR
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Macro 'CONDITIONAL_ERROR' checked here for value after being checked for definition
+// CHECK-MESSAGES: :[[@LINE-4]]:2: note: Macro 'CONDITIONAL_ERROR' first checked here for definition
+#if 0
+#error This error is conditional.
+#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
diff --git a/clang-tools-extra/unittests/clang-tidy/LexerUtilsTest.cpp b/clang-tools-extra/unittests/clang-tidy/LexerUtilsTest.cpp
index 485b527c281a2c..2f63a1db1865cb 100644
--- a/clang-tools-extra/unittests/clang-tidy/LexerUtilsTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/LexerUtilsTest.cpp
@@ -48,6 +48,30 @@ static bool isRawIdentifierNamed(const Token &Tok, StringRef Name) {
namespace {
+TEST(LexerUtilsTest, GetRawTokens) {
+ llvm::Annotations Code(R"cpp(
+void f() {
+ int alpha = 0;
+ int beta = 0;
+ bool value = $range[[alpha /* ignored */ && beta]];
+}
+)cpp");
+ std::unique_ptr<ASTUnit> AST = buildAST(Code.code());
+ ASSERT_TRUE(AST);
+ const ASTContext &Context = AST->getASTContext();
+ const SourceManager &SM = Context.getSourceManager();
+ const LangOptions &LangOpts = Context.getLangOpts();
+
+ const CharSourceRange Range =
+ rangeFromAnnotations(Code, SM, SM.getMainFileID(), "range");
+ const std::vector<Token> Tokens =
+ utils::lexer::getRawTokens(Range, SM, LangOpts);
+ ASSERT_EQ(3u, Tokens.size());
+ EXPECT_EQ("alpha", utils::lexer::getTokenName(Tokens[0]));
+ EXPECT_TRUE(Tokens[1].is(tok::ampamp));
+ EXPECT_EQ("beta", utils::lexer::getTokenName(Tokens[2]));
+}
+
TEST(LexerUtilsTest, GetCommentsInRangeAdjacentComments) {
llvm::Annotations Code(R"cpp(
void f() {
More information about the cfe-commits
mailing list