[clang-tools-extra] [clang-tidy] Add bugprone-macro-condition check (PR #210768)

[Zeyi Xu via cfe-commits]

================
@@ -0,0 +1,357 @@
+//===----------------------------------------------------------------------===//
+//
+// 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/Lex/Lexer.h"
+#include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringSet.h"
+#include <memory>
+#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;
+
+private:
+  struct MacroReference {
+    std::string Name;
+    SourceLocation Loc;
+  };
+
+  struct ConditionReferences {
+    SmallVector<MacroReference, 2> Definition;
+    SmallVector<MacroReference, 2> Value;
+  };
+
+  struct DefinitionCheck {
+    std::string Name;
+    SourceLocation DefinitionLoc;
+    SourceLocation CheckLoc;
+    bool ValueTested = false;
+  };
+
+  struct ConditionalBranch {
+    SmallVector<DefinitionCheck, 2> Checks;
+    llvm::StringSet<> DefinitionTests;
+  };
+
+  ConditionReferences referencesInCondition(SourceRange ConditionRange) const;
+  MacroReference referenceFromRange(SourceRange Range,
+                                    SourceLocation Loc) const;
+  bool isIgnoredIdentifier(StringRef Name) const;
+  void startCondition(const ConditionReferences &References);
+  void startDefinitionCondition(StringRef Name, SourceLocation Loc);
+  void nextBranch(const ConditionReferences &References = {});
+  void processReferences(const ConditionReferences &References);
+  void finishBranch(ConditionalBranch &Branch);
+  void checkDefinitionReference(const MacroReference &Reference,
+                                ConditionalBranch &Branch);
+  void checkValueReference(const MacroReference &Reference);
+  bool isDefinitionTestActive(StringRef Name) const;
+
+  SmallVector<ConditionalBranch, 8> Conditions;
+  llvm::DenseSet<unsigned> DiagnosedDefinitions;
+  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 {};
+}
+
+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) {
----------------
zeyi2 wrote:

I ran the check on some real world projects:

| Project | Status | Warnings | Errors | Crash |
| :--- | :--- | :--- | :--- | :--- |
| **abseil-cpp** |  Warnings | 418 | 0 | - |
| **cppcheck** |  Warnings | 40 | 0 | - |
| **curl** |  Warnings | 1102 | 0 | - |
| **doxygen** |  Warnings | 331 | 0 | - |
| **llvm-project** |  Warnings | 204 | 0 | - |
| **poco** |  Warnings | 989 | 0 | - |
| **stdexec** | Warnings | 53 | 0 | - |

The majority of these warnings appear to be FPs caused by the same underlying issue: `referencesInCondition()` appears to treat every raw identifier in a preprocessor condition as a macro value reference. For example, it would diagnose the identifiers shown below:

```
#if __has_include(<vector>) // diagnoses `vector` 
#if __has_builtin(__builtin_trap) // diagnoses `__builtin_trap` 
#if __has_cpp_attribute(gnu::always_inline) // diagnoses `gnu` and `always_inline` 
#define ALWAYS_TRUE(x) 1 
#if ALWAYS_TRUE(not_a_macro) // diagnoses `not_a_macro`
```

Given the warning volume across these projects, this is not an isolated edge case. It appears to be the primary source of false positives in the current implementation.

https://github.com/llvm/llvm-project/pull/210768