[clang-tools-extra] r323658 - [clangd] Add a fallback directory for collected symbols with relative paths.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 29 14:30:35 PST 2018
Thanks Galina! r323703 should fix this.
On Mon, Jan 29, 2018 at 11:09 PM Galina Kistanova <gkistanova at 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 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/12545ffc/attachment-0001.html>
More information about the cfe-commits
mailing list