This revision was automatically updated to reflect the committed changes.
Closed by commit rL325764: [clangd] Not collect include headers for dynamic
index for now. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D43550
Files:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -585,7 +585,7 @@
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols,
UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(TestHeaderURI),
- IncludeHeader(""))));
+ IncludeHeader(TestHeaderURI))));
}
#ifndef LLVM_ON_WIN32
Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
@@ -181,19 +181,19 @@
EXPECT_THAT(match(M, Req), UnorderedElementsAre("X"));
}
-#ifndef LLVM_ON_WIN32
-TEST(FileIndexTest, CanonicalizeSystemHeader) {
+TEST(FileIndexTest, NoIncludeCollected) {
FileIndex M;
- std::string File = testPath("bits/basic_string");
- M.update(File, build(File, "class string {};").getPointer());
+ M.update("f", build("f", "class string {};").getPointer());
FuzzyFindRequest Req;
Req.Query = "";
+ bool SeenSymbol = false;
M.fuzzyFind(Req, [&](const Symbol &Sym) {
- EXPECT_EQ(Sym.Detail->IncludeHeader, "<string>");
+ EXPECT_TRUE(Sym.Detail->IncludeHeader.empty());
+ SeenSymbol = true;
});
+ EXPECT_TRUE(SeenSymbol);
}
-#endif
} // namespace
} // namespace clangd
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -286,24 +286,18 @@
I.documentation = D->Documentation;
if (I.detail.empty())
I.detail = D->CompletionDetail;
- // We only insert #include for items with details, since we can't tell
- // whether the file URI of the canonical declaration would be the
- // canonical #include without checking IncludeHeader in the detail.
// FIXME: delay creating include insertion command to
// "completionItem/resolve", when it is supported
- if (!D->IncludeHeader.empty() ||
- !IndexResult->CanonicalDeclaration.FileURI.empty()) {
+ if (!D->IncludeHeader.empty()) {
// LSP favors additionalTextEdits over command. But we are still using
// command here because it would be expensive to calculate #include
// insertion edits for all candidates, and the include insertion edit
// is unlikely to conflict with the code completion edits.
Command Cmd;
// Command title is not added since this is not a user-facing command.
Cmd.command = ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE;
IncludeInsertion Insertion;
- Insertion.header = D->IncludeHeader.empty()
- ? IndexResult->CanonicalDeclaration.FileURI
- : D->IncludeHeader;
+ Insertion.header = D->IncludeHeader;
Insertion.textDocument.uri = URIForFile(FileName);
Cmd.includeInsertion = std::move(Insertion);
I.command = std::move(Cmd);
Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp
@@ -15,29 +15,22 @@
namespace clangd {
namespace {
-const CanonicalIncludes *canonicalIncludesForSystemHeaders() {
- static const auto *Includes = [] {
- auto *I = new CanonicalIncludes();
- addSystemHeadersMapping(I);
- return I;
- }();
- return Includes;
-}
-
/// Retrieves namespace and class level symbols in \p Decls.
std::unique_ptr<SymbolSlab> indexAST(ASTContext &Ctx,
std::shared_ptr<Preprocessor> PP,
llvm::ArrayRef<const Decl *> Decls) {
SymbolCollector::Options CollectorOpts;
// Although we do not index symbols in main files (e.g. cpp file), information
// in main files like definition locations of class declarations will still be
// collected; thus, the index works for go-to-definition.
- // FIXME(ioeric): handle IWYU pragma for dynamic index. We might want to make
- // SymbolCollector always provides include canonicalization (e.g. IWYU, STL).
// FIXME(ioeric): get rid of `IndexMainFiles` as this is always set to false.
CollectorOpts.IndexMainFiles = false;
- CollectorOpts.CollectIncludePath = true;
- CollectorOpts.Includes = canonicalIncludesForSystemHeaders();
+ // FIXME(ioeric): we might also want to collect include headers. We would need
+ // to make sure all includes are canonicalized (with CanonicalIncludes), which
+ // is not trivial given the current way of collecting symbols: we only have
+ // AST at this point, but we also need preprocessor callbacks (e.g.
+ // CommentHandler for IWYU pragma) to canonicalize includes.
+ CollectorOpts.CollectIncludePath = false;
auto Collector = std::make_shared<SymbolCollector>(std::move(CollectorOpts));
Collector->setPreprocessor(std::move(PP));
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -150,8 +150,9 @@
}
}
-/// Gets a canonical include (<header> or "header") for header of \p Loc.
-/// Returns None if the header has no canonical include.
+/// Gets a canonical include (URI of the header or <header> or "header") for
+/// header of \p Loc.
+/// Returns None if fails to get include header for \p Loc.
/// FIXME: we should handle .inc files whose symbols are expected be exported by
/// their containing headers.
llvm::Optional<std::string>
@@ -167,10 +168,8 @@
? Mapped.str()
: ("\"" + Mapped + "\"").str();
}
- // If the header path is the same as the file path of the declaration, we skip
- // storing the #include path; users can use the URI in declaration location to
- // calculate the #include path.
- return llvm::None;
+
+ return toURI(SM, SM.getFilename(Loc), Opts);
}
// Return the symbol location of the given declaration `D`.
Index: clang-tools-extra/trunk/clangd/index/Index.h
===================================================================
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -162,8 +162,8 @@
/// directly. When this is a URI, the exact #include path needs to be
/// calculated according to the URI scheme.
///
- /// If empty, FileURI in CanonicalDeclaration should be used to calculate
- /// the #include path.
+ /// This is a canonical include for the symbol and can be different from
+ /// FileURI in the CanonicalDeclaration.
llvm::StringRef IncludeHeader;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
