hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang-tools-extra.

include-cleaner didn't report the usage of token-pasting symbols, as
these symbols are spelled in a "scratch space" file which is not a real
file, thus we have some false postives of unused includes for these
symbols (e.g. ABSL_FLAG).

This patch adds a simple heuristic to handle this case, we use the
expansion location as a "proxy" reference location, this should work
well on token-pasting formed from macro arguemtns.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157434

Files:
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/lib/HTMLReport.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
@@ -197,6 +197,31 @@
           Pair(Code.point("4"), UnorderedElementsAre(MainFile))));
 }
 
+TEST_F(WalkUsedTest, TokenPasting) {
+  llvm::Annotations Code(R"cpp(
+    #define DEFINE_FLAG(name) int FLAG_##name = 0;
+    #define MY_FLAG(name) DEFINE_FLAG(MY_##name)
+
+    #define PASTED_TOKEN X##2
+
+    $1^DEFINE_FLAG(abc);
+    $2^MY_FLAG(abc);
+
+    int $3^TPASTED_TOKEN = 3;
+  )cpp");
+  Inputs.Code = Code.code();
+
+  TestAST AST(Inputs);
+  auto &SM = AST.sourceManager();
+  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+
+  EXPECT_THAT(offsetToProviders(AST, SM),
+              UnorderedElementsAre(
+                  Pair(Code.point("1"), UnorderedElementsAre(MainFile)),
+                  Pair(Code.point("2"), UnorderedElementsAre(MainFile)),
+                  Pair(Code.point("3"), UnorderedElementsAre(MainFile))));
+}
+
 class AnalyzeTest : public testing::Test {
 protected:
   TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -519,6 +519,9 @@
   const auto& SM = Ctx.getSourceManager();
   for (Decl *Root : Roots)
     walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) {
+      if (SM.isWrittenInScratchSpace( SM.getSpellingLoc(Loc)))
+        Loc = SM.getExpansionLoc(Loc);
+
       if(!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc)))
         return;
       R.addRef(SymbolReference{D, Loc, T});
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
@@ -40,6 +40,15 @@
   tooling::stdlib::Recognizer Recognizer;
   for (auto *Root : ASTRoots) {
     walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
+      // If the symbol is spelled as a token paste, it spelling location points
+      // to <scratch space> file which is not a real file. We fallback to use
+      // the expansion location, this herustic is based on the assumption that
+      // most of token pastings are formed with the macro arguement, and it can
+      // allow us to detect uses of symbol defined by the common macro like
+      // `ABSL_FLAG`.
+      if (SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc)))
+        Loc = SM.getExpansionLoc(Loc);
+
       auto FID = SM.getFileID(SM.getSpellingLoc(Loc));
       if (FID != SM.getMainFileID() && FID != SM.getPreambleFileID())
         return;


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
@@ -197,6 +197,31 @@
           Pair(Code.point("4"), UnorderedElementsAre(MainFile))));
 }
 
+TEST_F(WalkUsedTest, TokenPasting) {
+  llvm::Annotations Code(R"cpp(
+    #define DEFINE_FLAG(name) int FLAG_##name = 0;
+    #define MY_FLAG(name) DEFINE_FLAG(MY_##name)
+
+    #define PASTED_TOKEN X##2
+
+    $1^DEFINE_FLAG(abc);
+    $2^MY_FLAG(abc);
+
+    int $3^TPASTED_TOKEN = 3;
+  )cpp");
+  Inputs.Code = Code.code();
+
+  TestAST AST(Inputs);
+  auto &SM = AST.sourceManager();
+  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+
+  EXPECT_THAT(offsetToProviders(AST, SM),
+              UnorderedElementsAre(
+                  Pair(Code.point("1"), UnorderedElementsAre(MainFile)),
+                  Pair(Code.point("2"), UnorderedElementsAre(MainFile)),
+                  Pair(Code.point("3"), UnorderedElementsAre(MainFile))));
+}
+
 class AnalyzeTest : public testing::Test {
 protected:
   TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -519,6 +519,9 @@
   const auto& SM = Ctx.getSourceManager();
   for (Decl *Root : Roots)
     walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) {
+      if (SM.isWrittenInScratchSpace( SM.getSpellingLoc(Loc)))
+        Loc = SM.getExpansionLoc(Loc);
+
       if(!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc)))
         return;
       R.addRef(SymbolReference{D, Loc, T});
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
@@ -40,6 +40,15 @@
   tooling::stdlib::Recognizer Recognizer;
   for (auto *Root : ASTRoots) {
     walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
+      // If the symbol is spelled as a token paste, it spelling location points
+      // to <scratch space> file which is not a real file. We fallback to use
+      // the expansion location, this herustic is based on the assumption that
+      // most of token pastings are formed with the macro arguement, and it can
+      // allow us to detect uses of symbol defined by the common macro like
+      // `ABSL_FLAG`.
+      if (SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc)))
+        Loc = SM.getExpansionLoc(Loc);
+
       auto FID = SM.getFileID(SM.getSpellingLoc(Loc));
       if (FID != SM.getMainFileID() && FID != SM.getPreambleFileID())
         return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to