ioeric created this revision.
ioeric added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, jkorous-apple, klimek.
o Avoid inserting a header include into the header itself.
o Avoid inserting non-header files.
o Canonicalize include paths for symbols in dynamic index.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43462
Files:
clangd/ClangdServer.cpp
clangd/Headers.cpp
clangd/Headers.h
clangd/index/FileIndex.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/HeadersTests.cpp
Index: unittests/clangd/HeadersTests.cpp
===================================================================
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -24,24 +24,14 @@
}
protected:
- // Adds \p File with content \p Code to the sub directory and returns the
- // absolute path.
-// std::string addToSubdir(PathRef File, llvm::StringRef Code = "") {
-// assert(llvm::sys::path::is_relative(File) && "FileName should be relative");
-// llvm::SmallString<32> Path;
-// llvm::sys::path::append(Path, Subdir, File);
-// FS.Files[Path] = Code;
-// return Path.str();
-// };
-
- // Shortens the header and returns "" on error.
- std::string shorten(PathRef Header) {
+ // Calculates the include path for \p Header, or returns "" on error.
+ std::string calculate(PathRef Header) {
auto VFS = FS.getTaggedFileSystem(MainFile).Value;
auto Cmd = CDB.getCompileCommand(MainFile);
assert(static_cast<bool>(Cmd));
VFS->setCurrentWorkingDirectory(Cmd->Directory);
auto Path =
- shortenIncludePath(MainFile, FS.Files[MainFile], Header, *Cmd, VFS);
+ calculateIncludePath(MainFile, FS.Files[MainFile], Header, *Cmd, VFS);
if (!Path) {
llvm::consumeError(Path.takeError());
return std::string();
@@ -58,25 +48,25 @@
TEST_F(HeadersTest, InsertInclude) {
std::string Path = testPath("sub/bar.h");
FS.Files[Path] = "";
- EXPECT_EQ(shorten(Path), "\"bar.h\"");
+ EXPECT_EQ(calculate(Path), "\"bar.h\"");
}
TEST_F(HeadersTest, DontInsertDuplicateSameName) {
FS.Files[MainFile] = R"cpp(
#include "bar.h"
)cpp";
std::string BarHeader = testPath("sub/bar.h");
FS.Files[BarHeader] = "";
- EXPECT_EQ(shorten(BarHeader), "");
+ EXPECT_EQ(calculate(BarHeader), "");
}
TEST_F(HeadersTest, DontInsertDuplicateDifferentName) {
std::string BarHeader = testPath("sub/bar.h");
FS.Files[BarHeader] = "";
FS.Files[MainFile] = R"cpp(
#include "sub/bar.h" // not shortest
)cpp";
- EXPECT_EQ(shorten(BarHeader), "");
+ EXPECT_EQ(calculate(BarHeader), "");
}
TEST_F(HeadersTest, StillInsertIfTrasitivelyIncluded) {
@@ -89,7 +79,19 @@
FS.Files[MainFile] = R"cpp(
#include "bar.h"
)cpp";
- EXPECT_EQ(shorten(BazHeader), "\"baz.h\"");
+ EXPECT_EQ(calculate(BazHeader), "\"baz.h\"");
+}
+
+TEST_F(HeadersTest, DoNotInsertIfInSameFile) {
+ MainFile = testPath("main.h");
+ FS.Files[MainFile] = "";
+ EXPECT_EQ(calculate(MainFile), "");
+}
+
+TEST_F(HeadersTest, DoNotInsertCppFile) {
+ std::string CppFile = testPath("sub/x.cpp");
+ FS.Files[CppFile] = "";
+ EXPECT_EQ(calculate(CppFile), "");
}
} // namespace
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -7,6 +7,7 @@
//
//===----------------------------------------------------------------------===//
+#include "TestFS.h"
#include "index/FileIndex.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
@@ -169,6 +170,20 @@
EXPECT_THAT(match(M, Req), UnorderedElementsAre("X"));
}
+#ifndef LLVM_ON_WIN32
+TEST(FileIndexTest, CanonicalizeSystemHeader) {
+ FileIndex M;
+ std::string File = testPath("bits/basic_string.h");
+ M.update(File, build(File, "class string {};").getPointer());
+
+ FuzzyFindRequest Req;
+ Req.Query = "";
+ M.fuzzyFind(Req, [&](const Symbol &Sym) {
+ EXPECT_EQ(Sym.Detail->IncludeHeader, "<string>");
+ });
+}
+#endif
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/FileIndex.cpp
===================================================================
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -15,14 +15,24 @@
namespace clangd {
namespace {
+CanonicalIncludes *CanonicalIncludesForSystemHeaders() {
+ auto *Includes = new CanonicalIncludes();
+ addSystemHeadersMapping(Includes);
+ 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;
// Code completion gets main-file results from Sema.
// But we leave this option on because features like go-to-definition want it.
CollectorOpts.IndexMainFiles = true;
+ CollectorOpts.CollectIncludePath = true;
+ static auto *Includes = CanonicalIncludesForSystemHeaders();
+ CollectorOpts.Includes = Includes;
+
auto Collector = std::make_shared<SymbolCollector>(std::move(CollectorOpts));
Collector->setPreprocessor(std::move(PP));
index::IndexingOptions IndexOpts;
Index: clangd/Headers.h
===================================================================
--- clangd/Headers.h
+++ clangd/Headers.h
@@ -18,18 +18,22 @@
namespace clang {
namespace clangd {
+
/// Determines the preferred way to #include a file, taking into account the
/// search path. Usually this will prefer a shorter representation like
/// 'Foo/Bar.h' over a longer one like 'Baz/include/Foo/Bar.h'.
///
/// \param Header is an absolute file path.
-/// \return A quoted "path" or <path>. If \p Header is already (directly)
-/// included in the file (including those included via different paths), this
-/// returns an empty string.
+/// \return A quoted "path" or <path>. This returns an empty string if:
+/// - \p Header is already (directly) included in the file (including those
+/// included via different paths).
+/// - \p Header is the same as \p File.
+/// - \p Header does not ends with a proper extenstion for a header file e.g.
+/// ".h".
llvm::Expected<std::string>
-shortenIncludePath(PathRef File, llvm::StringRef Code, llvm::StringRef Header,
- const tooling::CompileCommand &CompileCommand,
- IntrusiveRefCntPtr<vfs::FileSystem> FS);
+calculateIncludePath(PathRef File, llvm::StringRef Code, llvm::StringRef Header,
+ const tooling::CompileCommand &CompileCommand,
+ IntrusiveRefCntPtr<vfs::FileSystem> FS);
} // namespace clangd
} // namespace clang
Index: clangd/Headers.cpp
===================================================================
--- clangd/Headers.cpp
+++ clangd/Headers.cpp
@@ -40,15 +40,25 @@
std::set<std::string> &Headers;
};
+/// Returns true if \p Path has header extensions like .h and .hpp etc.
+bool hasHeaderExtension(PathRef Path) {
+ constexpr static const char *HeaderExtensions[] = {".h", ".hpp", ".hh",
+ ".hxx"};
+ return std::any_of(std::begin(HeaderExtensions), std::end(HeaderExtensions),
+ [&Path](const char *Ext) { return Path.endswith(Ext); });
+}
+
} // namespace
/// FIXME(ioeric): we might not want to insert an absolute include path if the
/// path is not shortened.
llvm::Expected<std::string>
-shortenIncludePath(llvm::StringRef File, llvm::StringRef Code,
- llvm::StringRef Header,
- const tooling::CompileCommand &CompileCommand,
- IntrusiveRefCntPtr<vfs::FileSystem> FS) {
+calculateIncludePath(llvm::StringRef File, llvm::StringRef Code,
+ llvm::StringRef Header,
+ const tooling::CompileCommand &CompileCommand,
+ IntrusiveRefCntPtr<vfs::FileSystem> FS) {
+ if (File == Header || !hasHeaderExtension(Header))
+ return "";
// Set up a CompilerInstance and create a preprocessor to collect existing
// #include headers in \p Code. Preprocesor also provides HeaderSearch with
// which we can calculate the shortest include path for \p Header.
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -333,7 +333,7 @@
FS->setCurrentWorkingDirectory(CompileCommand.Directory);
auto Include =
- shortenIncludePath(File, Code, *Resolved, CompileCommand, FS);
+ calculateIncludePath(File, Code, *Resolved, CompileCommand, FS);
if (!Include)
return Include.takeError();
if (Include->empty())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
