[PATCH] D147135: [include-cleaner] Visit the VarDecl in ASTWalker.

[Haojian Wu via Phabricator via cfe-commits]

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

Fixes https://github.com/clangd/clangd/issues/1554


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147135

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


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -300,6 +300,12 @@
            "int k = string() ^+ string();");
 }
 
+TEST(WalkAST, VarDecls) {
+  // Definition uses declaration, not the other way around.
+  testWalk("extern int $explicit^x;", "int ^x = 1;");
+  testWalk("int x = 1;", "extern int ^x;");
+}
+
 TEST(WalkAST, Functions) {
   // Definition uses declaration, not the other way around.
   testWalk("void $explicit^foo();", "void ^foo() {}");
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
@@ -64,7 +64,7 @@
     };
   }
 
-  llvm::DenseMap<size_t, std::vector<Header>>
+  std::multimap<size_t, std::vector<Header>>
   offsetToProviders(TestAST &AST, SourceManager &SM,
                     llvm::ArrayRef<SymbolReference> MacroRefs = {}) {
     llvm::SmallVector<Decl *> TopLevelDecls;
@@ -73,13 +73,13 @@
         continue;
       TopLevelDecls.emplace_back(D);
     }
-    llvm::DenseMap<size_t, std::vector<Header>> OffsetToProviders;
+    std::multimap<size_t, std::vector<Header>> OffsetToProviders;
     walkUsed(TopLevelDecls, MacroRefs, &PI, SM,
              [&](const SymbolReference &Ref, llvm::ArrayRef<Header> Providers) {
                auto [FID, Offset] = SM.getDecomposedLoc(Ref.RefLocation);
                if (FID != SM.getMainFileID())
                  ADD_FAILURE() << "Reference outside of the main file!";
-               OffsetToProviders.try_emplace(Offset, Providers.vec());
+               OffsetToProviders.emplace(Offset, Providers.vec());
              });
     return OffsetToProviders;
   }
@@ -90,9 +90,9 @@
   #include "header.h"
   #include "private.h"
 
-  void $bar^bar($private^Private) {
+  void $bar^bar($private^Private $p^p) {
     $foo^foo();
-    std::$vector^vector $vconstructor^v;
+    std::$vector^vector $vconstructor^$v^v;
   }
   )cpp");
   Inputs.Code = Code.code();
@@ -116,11 +116,14 @@
       offsetToProviders(AST, SM),
       UnorderedElementsAre(
           Pair(Code.point("bar"), UnorderedElementsAre(MainFile)),
+          Pair(Code.point("p"), UnorderedElementsAre(MainFile)),
           Pair(Code.point("private"),
                UnorderedElementsAre(PublicFile, PrivateFile)),
           Pair(Code.point("foo"), UnorderedElementsAre(HeaderFile)),
           Pair(Code.point("vector"), UnorderedElementsAre(VectorSTL)),
-          Pair(Code.point("vconstructor"), UnorderedElementsAre(VectorSTL))));
+          Pair(Code.point("vconstructor"), UnorderedElementsAre(VectorSTL)),
+          Pair(Code.point("v"), UnorderedElementsAre(MainFile))
+          ));
 }
 
 TEST_F(WalkUsedTest, MultipleProviders) {
@@ -155,8 +158,8 @@
 TEST_F(WalkUsedTest, MacroRefs) {
   llvm::Annotations Code(R"cpp(
     #include "hdr.h"
-    int x = $1^ANSWER;
-    int y = $2^ANSWER;
+    int $3^x = $1^ANSWER;
+    int $4^y = $2^ANSWER;
   )cpp");
   llvm::Annotations Hdr(guard("#define ^ANSWER 42"));
   Inputs.Code = Code.code();
@@ -164,6 +167,8 @@
   TestAST AST(Inputs);
   auto &SM = AST.sourceManager();
   const auto *HdrFile = SM.getFileManager().getFile("hdr.h").get();
+  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+
   auto HdrID = SM.translateFile(HdrFile);
 
   IdentifierTable Idents;
@@ -181,7 +186,10 @@
                                          Answer2, RefType::Explicit}}),
       UnorderedElementsAre(
           Pair(Code.point("1"), UnorderedElementsAre(HdrFile)),
-          Pair(Code.point("2"), UnorderedElementsAre(HdrFile))));
+          Pair(Code.point("2"), UnorderedElementsAre(HdrFile)),
+          Pair(Code.point("3"), UnorderedElementsAre(MainFile)),
+          Pair(Code.point("4"), UnorderedElementsAre(MainFile))
+              ));
 }
 
 class AnalyzeTest : public testing::Test {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -156,6 +156,12 @@
       report(FD->getLocation(), FD);
     return true;
   }
+  bool VisitVarDecl(VarDecl *VD) {
+    // Mark declaration from definition as it needs type-checking.
+    if (VD->isThisDeclarationADefinition())
+      report(VD->getLocation(), VD);
+    return true;
+  }
 
   bool VisitEnumDecl(EnumDecl *D) {
     // Definition of an enum with an underlying type references declaration for

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits