Thanks Galina! r323703 should fix this. On Mon, Jan 29, 2018 at 11:09 PM Galina Kistanova <gkistan...@gmail.com> wrote:
> 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@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits