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
