This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE323658: [clangd] Add a fallback directory for collected
symbols with relative paths. (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D42638?vs=131799&id=131800#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42638
Files:
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -44,6 +44,7 @@
return arg.CompletionSnippetInsertText == S;
}
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+MATCHER_P(Path, P, "") { return arg.CanonicalDeclaration.FilePath == P; }
namespace clang {
namespace clangd {
@@ -145,18 +146,25 @@
runSymbolCollector(Header, Main);
EXPECT_THAT(Symbols,
UnorderedElementsAreArray(
- {QName("Foo"),
- QName("f1"),
- QName("f2"),
- QName("KInt"),
- QName("kStr"),
- QName("foo"),
- QName("foo::bar"),
- QName("foo::int32"),
- QName("foo::int32_t"),
- QName("foo::v1"),
- QName("foo::bar::v2"),
- QName("foo::baz")}));
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
+ QName("kStr"), QName("foo"), QName("foo::bar"),
+ QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
+ QName("foo::bar::v2"), QName("foo::baz")}));
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
+ CollectorOpts.IndexMainFiles = false;
+ runSymbolCollector("class Foo {};", /*Main=*/"");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(QName("Foo"), Path("symbols.h"))));
+}
+
+TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {
+ CollectorOpts.IndexMainFiles = false;
+ CollectorOpts.FallbackDir = "/cwd";
+ runSymbolCollector("class Foo {};", /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAre(
+ AllOf(QName("Foo"), Path("/cwd/symbols.h"))));
}
TEST_F(SymbolCollectorTest, IncludeEnums) {
Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===================================================================
--- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -35,6 +35,14 @@
namespace clang {
namespace clangd {
+static llvm::cl::opt<std::string> AssumedHeaderDir(
+ "assume-header-dir",
+ llvm::cl::desc("The index includes header that a symbol is defined in. "
+ "If the absolute path cannot be determined (e.g. an "
+ "in-memory VFS) then the relative path is resolved against "
+ "this directory, which must be absolute. If this flag is "
+ "not given, such headers will have relative paths."));
+
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
@@ -72,9 +80,11 @@
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = false;
+ auto CollectorOpts = SymbolCollector::Options();
+ CollectorOpts.FallbackDir = AssumedHeaderDir;
return new WrappedIndexAction(
- std::make_shared<SymbolCollector>(SymbolCollector::Options()),
- IndexOpts, Ctx);
+ std::make_shared<SymbolCollector>(std::move(CollectorOpts)), IndexOpts,
+ Ctx);
}
tooling::ExecutionContext *Ctx;
@@ -98,6 +108,12 @@
return 1;
}
+ if (!clang::clangd::AssumedHeaderDir.empty() &&
+ !llvm::sys::path::is_absolute(clang::clangd::AssumedHeaderDir)) {
+ llvm::errs() << "--assume-header-dir must be an absolute path.\n";
+ return 1;
+ }
+
auto Err = Executor->get()->execute(
llvm::make_unique<clang::clangd::SymbolIndexActionFactory>(
Executor->get()->getExecutionContext()));
Index: clangd/index/SymbolCollector.h
===================================================================
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -30,6 +30,10 @@
/// Whether to collect symbols in main files (e.g. the source file
/// corresponding to a TU).
bool IndexMainFiles = false;
+ // When symbol paths cannot be resolved to absolute paths (e.g. files in
+ // VFS that does not have absolute path), combine the fallback directory
+ // with symbols' paths to get absolute paths. This must be an absolute path.
+ std::string FallbackDir;
};
SymbolCollector(Options Opts);
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -14,44 +14,51 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Index/USRGeneration.h"
+#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
namespace clang {
namespace clangd {
namespace {
// Make the Path absolute using the current working directory of the given
-// SourceManager if the Path is not an absolute path.
+// SourceManager if the Path is not an absolute path. If failed, this combine
+// relative paths with \p FallbackDir to get an absolute path.
//
// The Path can be a path relative to the build directory, or retrieved from
// the SourceManager.
-std::string makeAbsolutePath(const SourceManager &SM, StringRef Path) {
+std::string makeAbsolutePath(const SourceManager &SM, StringRef Path,
+ StringRef FallbackDir) {
llvm::SmallString<128> AbsolutePath(Path);
if (std::error_code EC =
SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
AbsolutePath))
llvm::errs() << "Warning: could not make absolute file: '" << EC.message()
<< '\n';
- // Handle the symbolic link path case where the current working directory
- // (getCurrentWorkingDirectory) is a symlink./ We always want to the real
- // file path (instead of the symlink path) for the C++ symbols.
- //
- // Consider the following example:
- //
- // src dir: /project/src/foo.h
- // current working directory (symlink): /tmp/build -> /project/src/
- //
- // The file path of Symbol is "/project/src/foo.h" instead of
- // "/tmp/build/foo.h"
- const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
- llvm::sys::path::parent_path(AbsolutePath.str()));
- if (Dir) {
- StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
- SmallString<128> AbsoluteFilename;
- llvm::sys::path::append(AbsoluteFilename, DirName,
- llvm::sys::path::filename(AbsolutePath.str()));
- return AbsoluteFilename.str();
+ if (llvm::sys::path::is_absolute(AbsolutePath)) {
+ // Handle the symbolic link path case where the current working directory
+ // (getCurrentWorkingDirectory) is a symlink./ We always want to the real
+ // file path (instead of the symlink path) for the C++ symbols.
+ //
+ // Consider the following example:
+ //
+ // src dir: /project/src/foo.h
+ // current working directory (symlink): /tmp/build -> /project/src/
+ //
+ // The file path of Symbol is "/project/src/foo.h" instead of
+ // "/tmp/build/foo.h"
+ if (const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
+ llvm::sys::path::parent_path(AbsolutePath.str()))) {
+ StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+ SmallString<128> AbsoluteFilename;
+ llvm::sys::path::append(AbsoluteFilename, DirName,
+ llvm::sys::path::filename(AbsolutePath.str()));
+ AbsolutePath = AbsoluteFilename;
+ }
+ } else if (!FallbackDir.empty()) {
+ llvm::sys::fs::make_absolute(FallbackDir, AbsolutePath);
+ llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
}
return AbsolutePath.str();
}
@@ -142,8 +149,8 @@
return true;
auto &SM = ND->getASTContext().getSourceManager();
- std::string FilePath =
- makeAbsolutePath(SM, SM.getFilename(D->getLocation()));
+ std::string FilePath = makeAbsolutePath(
+ SM, SM.getFilename(D->getLocation()), Opts.FallbackDir);
SymbolLocation Location = {FilePath, SM.getFileOffset(D->getLocStart()),
SM.getFileOffset(D->getLocEnd())};
std::string QName = ND->getQualifiedNameAsString();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
