[clang-tools-extra] [clang-tidy][include-cleaner] Fix false positive for stdlib-like macros (PR #173149)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: mitchell (zeyi2)

<details>
<summary>Changes</summary>

Closes #<!-- -->172816

---
Full diff: https://github.com/llvm/llvm-project/pull/173149.diff


4 Files Affected:

- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp (+4-3) 
- (modified) clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp (+16-5) 
- (added) clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.c (+7) 


``````````diff
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 924b2c03cfd18..b8a642ee50f8f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -517,6 +517,10 @@ Changes in existing checks
 - Improved :doc:`misc-header-include-cycle
   <clang-tidy/checks/misc/header-include-cycle>` check performance.
 
+- Improved :doc:`misc-include-cleaner
+  <clang-tidy/checks/misc/include-cleaner>` check by avoiding warnings when a
+  standard library symbol is redefined within the codebase.
+
 - Improved :doc:`modernize-avoid-c-arrays
   <clang-tidy/checks/modernize/avoid-c-arrays>` to not diagnose array types
   which are part of an implicit instantiation of a template.
diff --git a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
index b7433305152f9..e50d8ee58d817 100644
--- a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
+++ b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
@@ -56,10 +56,11 @@ std::vector<Hinted<SymbolLocation>> locateDecl(const Decl &D) {
 
 std::vector<Hinted<SymbolLocation>> locateMacro(const Macro &M,
                                                 const tooling::stdlib::Lang L) {
-  // FIXME: Should we also provide physical locations?
+  std::vector<Hinted<SymbolLocation>> Result;
+  Result.push_back({M.Definition, Hints::CompleteSymbol});
   if (auto SS = tooling::stdlib::Symbol::named("", M.Name->getName(), L))
-    return {{*SS, Hints::CompleteSymbol}};
-  return {{M.Definition, Hints::CompleteSymbol}};
+    Result.push_back({*SS, Hints::CompleteSymbol});
+  return Result;
 }
 } // namespace
 
diff --git a/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp b/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
index 1e7baf142a75a..0884fd1db8ad7 100644
--- a/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
@@ -43,10 +43,12 @@ struct LocateExample {
   TestAST AST;
 
 public:
-  LocateExample(llvm::StringRef AnnotatedCode)
-      : Target(AnnotatedCode), AST([this] {
+  LocateExample(llvm::StringRef AnnotatedCode,
+                std::vector<const char *> ExtraArgs = {"-std=c++17"})
+      : Target(AnnotatedCode), AST([&] {
           TestInputs Inputs(Target.code());
-          Inputs.ExtraArgs.push_back("-std=c++17");
+          for (auto Arg : ExtraArgs)
+            Inputs.ExtraArgs.push_back(Arg);
           return Inputs;
         }()) {}
 
@@ -126,9 +128,10 @@ TEST(LocateSymbol, Stdlib) {
         ElementsAre(*tooling::stdlib::Symbol::named("std::", "vector")));
   }
   {
-    LocateExample Test("#define assert(x)\nvoid foo() { assert(true); }");
+    LocateExample Test("#define ^assert(x)\nvoid foo() { assert(true); }");
     EXPECT_THAT(locateSymbol(Test.findMacro("assert"), Test.langOpts()),
-                ElementsAre(*tooling::stdlib::Symbol::named("", "assert")));
+                ElementsAre(Test.points().front(),
+                            *tooling::stdlib::Symbol::named("", "assert")));
   }
 }
 
@@ -139,6 +142,14 @@ TEST(LocateSymbol, Macros) {
               ElementsAreArray(Test.points()));
 }
 
+TEST(LocateSymbol, MacroI_C) {
+  LocateExample Test("#define ^I 42", {"-x", "c", "-std=c99"});
+  EXPECT_THAT(locateSymbol(Test.findMacro("I"), Test.langOpts()),
+              ElementsAre(Test.points().front(),
+                          *tooling::stdlib::Symbol::named(
+                              "", "I", tooling::stdlib::Lang::C)));
+}
+
 MATCHER_P2(HintedSymbol, Symbol, Hint, "") {
   return std::tie(arg.Hint, arg) == std::tie(Hint, Symbol);
 }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.c b/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.c
new file mode 100644
index 0000000000000..5f7d323ad79e7
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.c
@@ -0,0 +1,7 @@
+// RUN: %check_clang_tidy %s misc-include-cleaner %t
+
+#define I 42
+void f(void) { I; }
+
+#define H 42
+void g(void) { H; }

``````````

</details>


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