[clang-tools-extra] 7cd6ce9 - [include-cleaner] Visit the VarDecl in ASTWalker.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2023-03-29T22:20:30+02:00
New Revision: 7cd6ce9e917ff4dfe0c0afa236df6e284d98e620

URL: https://github.com/llvm/llvm-project/commit/7cd6ce9e917ff4dfe0c0afa236df6e284d98e620
DIFF: https://github.com/llvm/llvm-project/commit/7cd6ce9e917ff4dfe0c0afa236df6e284d98e620.diff

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

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

Differential Revision: https://reviews.llvm.org/D147135

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index e70a24367d6a9..0db1f2bec8e6b 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -156,6 +156,12 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
       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

diff  --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index fc7d14582f632..be2fc142eec61 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -64,7 +64,7 @@ class WalkUsedTest : public testing::Test {
     };
   }
 
-  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 @@ class WalkUsedTest : public testing::Test {
         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 @@ TEST_F(WalkUsedTest, Basic) {
   #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 @@ TEST_F(WalkUsedTest, Basic) {
       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, MultipleProviders) {
 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 @@ TEST_F(WalkUsedTest, MacroRefs) {
   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 @@ TEST_F(WalkUsedTest, MacroRefs) {
                                          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 {

diff  --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 8fcc2b5886ae4..8ce556f5fbf1f 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -300,6 +300,12 @@ TEST(WalkAST, Operator) {
            "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() {}");