[clang-tools-extra] r323658 - [clangd] Add a fallback directory for collected symbols with relative paths.
Galina Kistanova via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 29 14:09:36 PST 2018
Hello Eric,
It look like some of your recent commits broke the test on one of our
builders:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
. . .
Failing Tests (1):
Extra Tools Unit Tests ::
clangd/./ClangdTests.exe/SymbolCollectorTest.SymbolRelativeWithFallback
The builder was already red and did not send notifications on the changes.
Please have a look?
Thanks
Galina
On Mon, Jan 29, 2018 at 7:13 AM, Eric Liu via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: ioeric
> Date: Mon Jan 29 07:13:29 2018
> New Revision: 323658
>
> URL: http://llvm.org/viewvc/llvm-project?rev=323658&view=rev
> Log:
> [clangd] Add a fallback directory for collected symbols with relative
> paths.
>
> Reviewers: hokein, sammccall
>
> Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D42638
>
> Modified:
> clang-tools-extra/trunk/clangd/global-symbol-builder/
> GlobalSymbolBuilderMain.cpp
> clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
> clang-tools-extra/trunk/clangd/index/SymbolCollector.h
> clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/
> GlobalSymbolBuilderMain.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?
> rev=323658&r1=323657&r2=323658&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
> (original)
> +++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
> Mon Jan 29 07:13:29 2018
> @@ -35,6 +35,14 @@ using clang::clangd::SymbolSlab;
> 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 @@ public:
> 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 @@ int main(int argc, const char **argv) {
> 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()));
>
> Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/index/SymbolCollector.cpp?rev=323658&r1=323657&r2=323658&
> view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
> +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Jan 29
> 07:13:29 2018
> @@ -14,6 +14,7 @@
> #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"
>
> @@ -22,36 +23,42 @@ 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 @@ bool SymbolCollector::handleDeclOccurenc
> 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();
>
> Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/clangd/index/SymbolCollector.h?rev=323658&
> r1=323657&r2=323658&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
> +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Mon Jan 29
> 07:13:29 2018
> @@ -30,6 +30,10 @@ public:
> /// 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);
>
> Modified: clang-tools-extra/trunk/unittests/clangd/
> SymbolCollectorTests.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=
> 323658&r1=323657&r2=323658&view=diff
> ============================================================
> ==================
> --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
> (original)
> +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Mon
> Jan 29 07:13:29 2018
> @@ -44,6 +44,7 @@ MATCHER_P(Snippet, S, "") {
> 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 @@ TEST_F(SymbolCollectorTest, CollectSymbo
> 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) {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180129/e3bead77/attachment-0001.html>
More information about the cfe-commits
mailing list