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() {}"); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits