[clang-tools-extra] r373197 - [clangd] Implement a smart version of HeaderSource switch.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 30 03:48:03 PDT 2019
Author: hokein
Date: Mon Sep 30 03:48:02 2019
New Revision: 373197
URL: http://llvm.org/viewvc/llvm-project?rev=373197&view=rev
Log:
[clangd] Implement a smart version of HeaderSource switch.
Summary:
This patch implements another version header-source switch by incorporating the
AST and index, it will be used:
- to improve the current header-source switch feature (layer with the
existing file heuristic);
- by the incoming define-outline code action;
Reviewers: kadircet
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D67907
Modified:
clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp
clang-tools-extra/trunk/clangd/HeaderSourceSwitch.h
clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp
Modified: clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp?rev=373197&r1=373196&r2=373197&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp (original)
+++ clang-tools-extra/trunk/clangd/HeaderSourceSwitch.cpp Mon Sep 30 03:48:02 2019
@@ -7,6 +7,10 @@
//===----------------------------------------------------------------------===//
#include "HeaderSourceSwitch.h"
+#include "AST.h"
+#include "Logger.h"
+#include "index/SymbolCollector.h"
+#include "clang/AST/Decl.h"
namespace clang {
namespace clangd {
@@ -64,5 +68,86 @@ llvm::Optional<Path> getCorrespondingHea
return None;
}
+llvm::Optional<Path> getCorrespondingHeaderOrSource(const Path &OriginalFile,
+ ParsedAST &AST,
+ const SymbolIndex *Index) {
+ if (!Index) {
+ // FIXME: use the AST to do the inference.
+ return None;
+ }
+ LookupRequest Request;
+ // Find all symbols present in the original file.
+ for (const auto *D : getIndexableLocalDecls(AST)) {
+ if (auto ID = getSymbolID(D))
+ Request.IDs.insert(*ID);
+ }
+ llvm::StringMap<int> Candidates; // Target path => score.
+ auto AwardTarget = [&](const char *TargetURI) {
+ if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
+ if (*TargetPath != OriginalFile) // exclude the original file.
+ ++Candidates[*TargetPath];
+ };
+ };
+ // If we switch from a header, we are looking for the implementation
+ // file, so we use the definition loc; otherwise we look for the header file,
+ // we use the decl loc;
+ //
+ // For each symbol in the original file, we get its target location (decl or
+ // def) from the index, then award that target file.
+ bool IsHeader = AST.getASTContext().getLangOpts().IsHeaderFile;
+ Index->lookup(Request, [&](const Symbol &Sym) {
+ if (IsHeader)
+ AwardTarget(Sym.Definition.FileURI);
+ else
+ AwardTarget(Sym.CanonicalDeclaration.FileURI);
+ });
+ // FIXME: our index doesn't have any interesting information (this could be
+ // that the background-index is not finished), we should use the decl/def
+ // locations from the AST to do the inference (from .cc to .h).
+ if (Candidates.empty())
+ return None;
+
+ // Pickup the winner, who contains most of symbols.
+ // FIXME: should we use other signals (file proximity) to help score?
+ auto Best = Candidates.begin();
+ for (auto It = Candidates.begin(); It != Candidates.end(); ++It) {
+ if (It->second > Best->second)
+ Best = It;
+ else if (It->second == Best->second && It->first() < Best->first())
+ // Select the first one in the lexical order if we have multiple
+ // candidates.
+ Best = It;
+ }
+ return Path(Best->first());
+}
+
+std::vector<const Decl *> getIndexableLocalDecls(ParsedAST &AST) {
+ std::vector<const Decl *> Results;
+ std::function<void(Decl *)> TraverseDecl = [&](Decl *D) {
+ auto *ND = llvm::dyn_cast<NamedDecl>(D);
+ if (!ND || ND->isImplicit())
+ return;
+ if (!SymbolCollector::shouldCollectSymbol(*ND, D->getASTContext(), {},
+ /*IsMainFileSymbol=*/false))
+ return;
+ if (!llvm::isa<FunctionDecl>(ND)) {
+ // Visit the children, but we skip function decls as we are not interested
+ // in the function body.
+ if (auto *Scope = llvm::dyn_cast<DeclContext>(ND)) {
+ for (auto *D : Scope->decls())
+ TraverseDecl(D);
+ }
+ }
+ if (llvm::isa<NamespaceDecl>(D))
+ return; // namespace is indexable, but we're not interested.
+ Results.push_back(D);
+ };
+ // Traverses the ParsedAST directly to collect all decls present in the main
+ // file.
+ for (auto *TopLevel : AST.getLocalTopLevelDecls())
+ TraverseDecl(TopLevel);
+ return Results;
+}
+
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/HeaderSourceSwitch.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/HeaderSourceSwitch.h?rev=373197&r1=373196&r2=373197&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/HeaderSourceSwitch.h (original)
+++ clang-tools-extra/trunk/clangd/HeaderSourceSwitch.h Mon Sep 30 03:48:02 2019
@@ -21,6 +21,16 @@ llvm::Optional<Path> getCorrespondingHea
const Path &OriginalFile,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS);
+/// Given a header file, returns the best matching source file, and vice visa.
+/// The heuristics incorporate with the AST and the index (if provided).
+llvm::Optional<Path> getCorrespondingHeaderOrSource(const Path &OriginalFile,
+ ParsedAST &AST,
+ const SymbolIndex *Index);
+
+/// Returns all indexable decls that are present in the main file of the AST.
+/// Exposed for unittests.
+std::vector<const Decl *> getIndexableLocalDecls(ParsedAST &AST);
+
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp?rev=373197&r1=373196&r2=373197&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp Mon Sep 30 03:48:02 2019
@@ -10,6 +10,7 @@
#include "TestFS.h"
#include "TestTU.h"
+#include "index/MemIndex.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -71,6 +72,174 @@ TEST(HeaderSourceSwitchTest, FileHeurist
EXPECT_FALSE(PathResult.hasValue());
}
+MATCHER_P(DeclNamed, Name, "") {
+ if (const NamedDecl *ND = dyn_cast<NamedDecl>(arg))
+ if (ND->getQualifiedNameAsString() == Name)
+ return true;
+ return false;
+}
+
+TEST(HeaderSourceSwitchTest, GetLocalDecls) {
+ TestTU TU;
+ TU.HeaderCode = R"cpp(
+ void HeaderOnly();
+ )cpp";
+ TU.Code = R"cpp(
+ void MainF1();
+ class Foo {};
+ namespace ns {
+ class Foo {
+ void method();
+ int field;
+ };
+ } // namespace ns
+
+ // Non-indexable symbols
+ namespace {
+ void Ignore1() {}
+ }
+
+ )cpp";
+
+ auto AST = TU.build();
+ EXPECT_THAT(getIndexableLocalDecls(AST),
+ testing::UnorderedElementsAre(
+ DeclNamed("MainF1"), DeclNamed("Foo"), DeclNamed("ns::Foo"),
+ DeclNamed("ns::Foo::method"), DeclNamed("ns::Foo::field")));
+}
+
+TEST(HeaderSourceSwitchTest, FromHeaderToSource) {
+ // build a proper index, which contains symbols:
+ // A_Sym1, declared in TestTU.h, defined in a.cpp
+ // B_Sym[1-2], declared in TestTU.h, defined in b.cpp
+ SymbolSlab::Builder AllSymbols;
+ TestTU Testing;
+ Testing.HeaderFilename = "TestTU.h";
+ Testing.HeaderCode = "void A_Sym1();";
+ Testing.Filename = "a.cpp";
+ Testing.Code = "void A_Sym1() {};";
+ for (auto &Sym : Testing.headerSymbols())
+ AllSymbols.insert(Sym);
+
+ Testing.HeaderCode = R"cpp(
+ void B_Sym1();
+ void B_Sym2();
+ )cpp";
+ Testing.Filename = "b.cpp";
+ Testing.Code = R"cpp(
+ void B_Sym1() {}
+ void B_Sym2() {}
+ )cpp";
+ for (auto &Sym : Testing.headerSymbols())
+ AllSymbols.insert(Sym);
+ auto Index = MemIndex::build(std::move(AllSymbols).build(), {}, {});
+
+ // Test for swtich from .h header to .cc source
+ struct {
+ llvm::StringRef HeaderCode;
+ llvm::Optional<std::string> ExpectedSource;
+ } TestCases[] = {
+ {"// empty, no header found", llvm::None},
+ {R"cpp(
+ // no definition found in the index.
+ void NonDefinition();
+ )cpp",
+ llvm::None},
+ {R"cpp(
+ void A_Sym1();
+ )cpp",
+ testPath("a.cpp")},
+ {R"cpp(
+ // b.cpp wins.
+ void A_Sym1();
+ void B_Sym1();
+ void B_Sym2();
+ )cpp",
+ testPath("b.cpp")},
+ {R"cpp(
+ // a.cpp and b.cpp have same scope, but a.cpp because "a.cpp" < "b.cpp".
+ void A_Sym1();
+ void B_Sym1();
+ )cpp",
+ testPath("a.cpp")},
+ };
+ for (const auto &Case : TestCases) {
+ TestTU TU = TestTU::withCode(Case.HeaderCode);
+ TU.Filename = "TestTU.h";
+ TU.ExtraArgs.push_back("-xc++-header"); // inform clang this is a header.
+ auto HeaderAST = TU.build();
+ EXPECT_EQ(Case.ExpectedSource,
+ getCorrespondingHeaderOrSource(testPath(TU.Filename), HeaderAST,
+ Index.get()));
+ }
+}
+
+TEST(HeaderSourceSwitchTest, FromSourceToHeader) {
+ // build a proper index, which contains symbols:
+ // A_Sym1, declared in a.h, defined in TestTU.cpp
+ // B_Sym[1-2], declared in b.h, defined in TestTU.cpp
+ TestTU TUForIndex = TestTU::withCode(R"cpp(
+ #include "a.h"
+ #include "b.h"
+
+ void A_Sym1() {}
+
+ void B_Sym1() {}
+ void B_Sym2() {}
+ )cpp");
+ TUForIndex.AdditionalFiles["a.h"] = R"cpp(
+ void A_Sym1();
+ )cpp";
+ TUForIndex.AdditionalFiles["b.h"] = R"cpp(
+ void B_Sym1();
+ void B_Sym2();
+ )cpp";
+ TUForIndex.Filename = "TestTU.cpp";
+ auto Index = TUForIndex.index();
+
+ // Test for switching from .cc source file to .h header.
+ struct {
+ llvm::StringRef SourceCode;
+ llvm::Optional<std::string> ExpectedResult;
+ } TestCases[] = {
+ {"// empty, no header found", llvm::None},
+ {R"cpp(
+ // symbol not in index, no header found
+ void Local() {}
+ )cpp",
+ llvm::None},
+
+ {R"cpp(
+ // a.h wins.
+ void A_Sym1() {}
+ )cpp",
+ testPath("a.h")},
+
+ {R"cpp(
+ // b.h wins.
+ void A_Sym1() {}
+ void B_Sym1() {}
+ void B_Sym2() {}
+ )cpp",
+ testPath("b.h")},
+
+ {R"cpp(
+ // a.h and b.h have same scope, but a.h wins because "a.h" < "b.h".
+ void A_Sym1() {}
+ void B_Sym1() {}
+ )cpp",
+ testPath("a.h")},
+ };
+ for (const auto &Case : TestCases) {
+ TestTU TU = TestTU::withCode(Case.SourceCode);
+ TU.Filename = "Test.cpp";
+ auto AST = TU.build();
+ EXPECT_EQ(Case.ExpectedResult,
+ getCorrespondingHeaderOrSource(testPath(TU.Filename), AST,
+ Index.get()));
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list