[PATCH] D157905: [include-cleaner] Filter references to identity macros

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Despite being true positives, these results just confuse users. So
filter them out.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157905

Files:
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -483,5 +483,29 @@
                 Pair(Code.point("partial"), UnorderedElementsAre(Partial)))));
 }
 
+TEST_F(WalkUsedTest, IgnoresIdentityMacros) {
+  llvm::Annotations Code(R"cpp(
+  #include "header.h"
+  void $bar^bar() {
+    $stdin^stdin();
+  }
+  )cpp");
+  Inputs.Code = Code.code();
+  Inputs.ExtraFiles["header.h"] = guard(R"cpp(
+  #include "inner.h"
+  void stdin();
+  )cpp");
+  Inputs.ExtraFiles["inner.h"] = guard(R"cpp(
+  #define stdin stdin
+  )cpp");
+
+  TestAST AST(Inputs);
+  auto &SM = AST.sourceManager();
+  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+  EXPECT_THAT(offsetToProviders(AST, SM),
+              UnorderedElementsAre(
+                  // FIXME: we should have a reference from stdin to header.h
+                  Pair(Code.point("bar"), UnorderedElementsAre(MainFile))));
+}
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -28,10 +28,29 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
+#include <cassert>
 #include <string>
 
 namespace clang::include_cleaner {
 
+namespace {
+bool shouldSkipMacro(const Macro &M) {
+  static const auto *MacroNamesToIgnore = new llvm::StringSet<>{
+      // C standard says these are implementation defined macros, hence most of
+      // the standard library providers implement it by defining these as macros
+      // that resolve to themselves.
+      // This results in surprising behavior from users point of view (we
+      // generate a usage of stdio.h, in places unrelated to standard library).
+      // FIXME: Also eliminate the false positives by treating declarations
+      // resulting from these expansions as used.
+      "stdin",
+      "stdout",
+      "stderr",
+  };
+  return MacroNamesToIgnore->contains(M.Name->getName());
+}
+} // namespace
+
 void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
               llvm::ArrayRef<SymbolReference> MacroRefs,
               const PragmaIncludes *PI, const SourceManager &SM,
@@ -51,7 +70,8 @@
   }
   for (const SymbolReference &MacroRef : MacroRefs) {
     assert(MacroRef.Target.kind() == Symbol::Macro);
-    if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)))
+    if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)) ||
+        shouldSkipMacro(MacroRef.Target.macro()))
       continue;
     CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI));
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157905.550024.patch
Type: text/x-patch
Size: 2940 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230814/5648e67b/attachment.bin>