<div dir="ltr">Hello Eric,<br><br>It look like some of your recent commits broke the test on one of our builders:<br><a href="http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast">http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast</a><br>. . .<br>Failing Tests (1):<br> Extra Tools Unit Tests :: clangd/./ClangdTests.exe/SymbolCollectorTest.SymbolRelativeWithFallback<br><br>The builder was already red and did not send notifications on the changes.<br>Please have a look?<br><br>Thanks<br><br>Galina<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 29, 2018 at 7:13 AM, Eric Liu via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ioeric<br>
Date: Mon Jan 29 07:13:29 2018<br>
New Revision: 323658<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=323658&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=323658&view=rev</a><br>
Log:<br>
[clangd] Add a fallback directory for collected symbols with relative paths.<br>
<br>
Reviewers: hokein, sammccall<br>
<br>
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D42638" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D42638</a><br>
<br>
Modified:<br>
clang-tools-extra/trunk/<wbr>clangd/global-symbol-builder/<wbr>GlobalSymbolBuilderMain.cpp<br>
clang-tools-extra/trunk/<wbr>clangd/index/SymbolCollector.<wbr>cpp<br>
clang-tools-extra/trunk/<wbr>clangd/index/SymbolCollector.h<br>
clang-tools-extra/trunk/<wbr>unittests/clangd/<wbr>SymbolCollectorTests.cpp<br>
<br>
Modified: clang-tools-extra/trunk/<wbr>clangd/global-symbol-builder/<wbr>GlobalSymbolBuilderMain.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=323658&r1=323657&r2=323658&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/clangd/global-symbol-<wbr>builder/<wbr>GlobalSymbolBuilderMain.cpp?<wbr>rev=323658&r1=323657&r2=<wbr>323658&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/<wbr>clangd/global-symbol-builder/<wbr>GlobalSymbolBuilderMain.cpp (original)<br>
+++ clang-tools-extra/trunk/<wbr>clangd/global-symbol-builder/<wbr>GlobalSymbolBuilderMain.cpp Mon Jan 29 07:13:29 2018<br>
@@ -35,6 +35,14 @@ using clang::clangd::SymbolSlab;<br>
namespace clang {<br>
namespace clangd {<br>
<br>
+static llvm::cl::opt<std::string> AssumedHeaderDir(<br>
+ "assume-header-dir",<br>
+ llvm::cl::desc("The index includes header that a symbol is defined in. "<br>
+ "If the absolute path cannot be determined (e.g. an "<br>
+ "in-memory VFS) then the relative path is resolved against "<br>
+ "this directory, which must be absolute. If this flag is "<br>
+ "not given, such headers will have relative paths."));<br>
+<br>
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {<br>
public:<br>
SymbolIndexActionFactory(<wbr>tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}<br>
@@ -72,9 +80,11 @@ public:<br>
IndexOpts.SystemSymbolFilter =<br>
index::IndexingOptions::<wbr>SystemSymbolFilterKind::All;<br>
IndexOpts.IndexFunctionLocals = false;<br>
+ auto CollectorOpts = SymbolCollector::Options();<br>
+ CollectorOpts.FallbackDir = AssumedHeaderDir;<br>
return new WrappedIndexAction(<br>
- std::make_shared<<wbr>SymbolCollector>(<wbr>SymbolCollector::Options()),<br>
- IndexOpts, Ctx);<br>
+ std::make_shared<<wbr>SymbolCollector>(std::move(<wbr>CollectorOpts)), IndexOpts,<br>
+ Ctx);<br>
}<br>
<br>
tooling::ExecutionContext *Ctx;<br>
@@ -98,6 +108,12 @@ int main(int argc, const char **argv) {<br>
return 1;<br>
}<br>
<br>
+ if (!clang::clangd::<wbr>AssumedHeaderDir.empty() &&<br>
+ !llvm::sys::path::is_absolute(<wbr>clang::clangd::<wbr>AssumedHeaderDir)) {<br>
+ llvm::errs() << "--assume-header-dir must be an absolute path.\n";<br>
+ return 1;<br>
+ }<br>
+<br>
auto Err = Executor->get()->execute(<br>
llvm::make_unique<clang::<wbr>clangd::<wbr>SymbolIndexActionFactory>(<br>
Executor->get()-><wbr>getExecutionContext()));<br>
<br>
Modified: clang-tools-extra/trunk/<wbr>clangd/index/SymbolCollector.<wbr>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=323658&r1=323657&r2=323658&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/clangd/index/<wbr>SymbolCollector.cpp?rev=<wbr>323658&r1=323657&r2=323658&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/<wbr>clangd/index/SymbolCollector.<wbr>cpp (original)<br>
+++ clang-tools-extra/trunk/<wbr>clangd/index/SymbolCollector.<wbr>cpp Mon Jan 29 07:13:29 2018<br>
@@ -14,6 +14,7 @@<br>
#include "clang/Basic/SourceManager.h"<br>
#include "clang/Index/IndexSymbol.h"<br>
#include "clang/Index/USRGeneration.h"<br>
+#include "llvm/Support/FileSystem.h"<br>
#include "llvm/Support/MemoryBuffer.h"<br>
#include "llvm/Support/Path.h"<br>
<br>
@@ -22,36 +23,42 @@ namespace clangd {<br>
<br>
namespace {<br>
// Make the Path absolute using the current working directory of the given<br>
-// SourceManager if the Path is not an absolute path.<br>
+// SourceManager if the Path is not an absolute path. If failed, this combine<br>
+// relative paths with \p FallbackDir to get an absolute path.<br>
//<br>
// The Path can be a path relative to the build directory, or retrieved from<br>
// the SourceManager.<br>
-std::string makeAbsolutePath(const SourceManager &SM, StringRef Path) {<br>
+std::string makeAbsolutePath(const SourceManager &SM, StringRef Path,<br>
+ StringRef FallbackDir) {<br>
llvm::SmallString<128> AbsolutePath(Path);<br>
if (std::error_code EC =<br>
SM.getFileManager().<wbr>getVirtualFileSystem()-><wbr>makeAbsolute(<br>
AbsolutePath))<br>
llvm::errs() << "Warning: could not make absolute file: '" << EC.message()<br>
<< '\n';<br>
- // Handle the symbolic link path case where the current working directory<br>
- // (getCurrentWorkingDirectory) is a symlink./ We always want to the real<br>
- // file path (instead of the symlink path) for the C++ symbols.<br>
- //<br>
- // Consider the following example:<br>
- //<br>
- // src dir: /project/src/foo.h<br>
- // current working directory (symlink): /tmp/build -> /project/src/<br>
- //<br>
- // The file path of Symbol is "/project/src/foo.h" instead of<br>
- // "/tmp/build/foo.h"<br>
- const DirectoryEntry *Dir = SM.getFileManager().<wbr>getDirectory(<br>
- llvm::sys::path::parent_path(<wbr>AbsolutePath.str()));<br>
- if (Dir) {<br>
- StringRef DirName = SM.getFileManager().<wbr>getCanonicalName(Dir);<br>
- SmallString<128> AbsoluteFilename;<br>
- llvm::sys::path::append(<wbr>AbsoluteFilename, DirName,<br>
- llvm::sys::path::filename(<wbr>AbsolutePath.str()));<br>
- return AbsoluteFilename.str();<br>
+ if (llvm::sys::path::is_absolute(<wbr>AbsolutePath)) {<br>
+ // Handle the symbolic link path case where the current working directory<br>
+ // (getCurrentWorkingDirectory) is a symlink./ We always want to the real<br>
+ // file path (instead of the symlink path) for the C++ symbols.<br>
+ //<br>
+ // Consider the following example:<br>
+ //<br>
+ // src dir: /project/src/foo.h<br>
+ // current working directory (symlink): /tmp/build -> /project/src/<br>
+ //<br>
+ // The file path of Symbol is "/project/src/foo.h" instead of<br>
+ // "/tmp/build/foo.h"<br>
+ if (const DirectoryEntry *Dir = SM.getFileManager().<wbr>getDirectory(<br>
+ llvm::sys::path::parent_path(<wbr>AbsolutePath.str()))) {<br>
+ StringRef DirName = SM.getFileManager().<wbr>getCanonicalName(Dir);<br>
+ SmallString<128> AbsoluteFilename;<br>
+ llvm::sys::path::append(<wbr>AbsoluteFilename, DirName,<br>
+ llvm::sys::path::filename(<wbr>AbsolutePath.str()));<br>
+ AbsolutePath = AbsoluteFilename;<br>
+ }<br>
+ } else if (!FallbackDir.empty()) {<br>
+ llvm::sys::fs::make_absolute(<wbr>FallbackDir, AbsolutePath);<br>
+ llvm::sys::path::remove_dots(<wbr>AbsolutePath, /*remove_dot_dot=*/true);<br>
}<br>
return AbsolutePath.str();<br>
}<br>
@@ -142,8 +149,8 @@ bool SymbolCollector::<wbr>handleDeclOccurenc<br>
return true;<br>
<br>
auto &SM = ND->getASTContext().<wbr>getSourceManager();<br>
- std::string FilePath =<br>
- makeAbsolutePath(SM, SM.getFilename(D->getLocation(<wbr>)));<br>
+ std::string FilePath = makeAbsolutePath(<br>
+ SM, SM.getFilename(D->getLocation(<wbr>)), Opts.FallbackDir);<br>
SymbolLocation Location = {FilePath, SM.getFileOffset(D-><wbr>getLocStart()),<br>
SM.getFileOffset(D->getLocEnd(<wbr>))};<br>
std::string QName = ND->getQualifiedNameAsString()<wbr>;<br>
<br>
Modified: clang-tools-extra/trunk/<wbr>clangd/index/SymbolCollector.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=323658&r1=323657&r2=323658&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/clangd/index/<wbr>SymbolCollector.h?rev=323658&<wbr>r1=323657&r2=323658&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/<wbr>clangd/index/SymbolCollector.h (original)<br>
+++ clang-tools-extra/trunk/<wbr>clangd/index/SymbolCollector.h Mon Jan 29 07:13:29 2018<br>
@@ -30,6 +30,10 @@ public:<br>
/// Whether to collect symbols in main files (e.g. the source file<br>
/// corresponding to a TU).<br>
bool IndexMainFiles = false;<br>
+ // When symbol paths cannot be resolved to absolute paths (e.g. files in<br>
+ // VFS that does not have absolute path), combine the fallback directory<br>
+ // with symbols' paths to get absolute paths. This must be an absolute path.<br>
+ std::string FallbackDir;<br>
};<br>
<br>
SymbolCollector(Options Opts);<br>
<br>
Modified: clang-tools-extra/trunk/<wbr>unittests/clangd/<wbr>SymbolCollectorTests.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=323658&r1=323657&r2=323658&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/clang-tools-extra/<wbr>trunk/unittests/clangd/<wbr>SymbolCollectorTests.cpp?rev=<wbr>323658&r1=323657&r2=323658&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- clang-tools-extra/trunk/<wbr>unittests/clangd/<wbr>SymbolCollectorTests.cpp (original)<br>
+++ clang-tools-extra/trunk/<wbr>unittests/clangd/<wbr>SymbolCollectorTests.cpp Mon Jan 29 07:13:29 2018<br>
@@ -44,6 +44,7 @@ MATCHER_P(Snippet, S, "") {<br>
return arg.<wbr>CompletionSnippetInsertText == S;<br>
}<br>
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }<br>
+MATCHER_P(Path, P, "") { return arg.CanonicalDeclaration.<wbr>FilePath == P; }<br>
<br>
namespace clang {<br>
namespace clangd {<br>
@@ -145,18 +146,25 @@ TEST_F(SymbolCollectorTest, CollectSymbo<br>
runSymbolCollector(Header, Main);<br>
EXPECT_THAT(Symbols,<br>
UnorderedElementsAreArray(<br>
- {QName("Foo"),<br>
- QName("f1"),<br>
- QName("f2"),<br>
- QName("KInt"),<br>
- QName("kStr"),<br>
- QName("foo"),<br>
- QName("foo::bar"),<br>
- QName("foo::int32"),<br>
- QName("foo::int32_t"),<br>
- QName("foo::v1"),<br>
- QName("foo::bar::v2"),<br>
- QName("foo::baz")}));<br>
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),<br>
+ QName("kStr"), QName("foo"), QName("foo::bar"),<br>
+ QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),<br>
+ QName("foo::bar::v2"), QName("foo::baz")}));<br>
+}<br>
+<br>
+TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {<br>
+ CollectorOpts.IndexMainFiles = false;<br>
+ runSymbolCollector("class Foo {};", /*Main=*/"");<br>
+ EXPECT_THAT(Symbols,<br>
+ UnorderedElementsAre(AllOf(<wbr>QName("Foo"), Path("symbols.h"))));<br>
+}<br>
+<br>
+TEST_F(SymbolCollectorTest, SymbolRelativeWithFallback) {<br>
+ CollectorOpts.IndexMainFiles = false;<br>
+ CollectorOpts.FallbackDir = "/cwd";<br>
+ runSymbolCollector("class Foo {};", /*Main=*/"");<br>
+ EXPECT_THAT(Symbols, UnorderedElementsAre(<br>
+ AllOf(QName("Foo"), Path("/cwd/symbols.h"))));<br>
}<br>
<br>
TEST_F(SymbolCollectorTest, IncludeEnums) {<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>