kbobyrev marked an inline comment as done.
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:64
+ if (ParsedURI->scheme() != "file") {
+elog("Can not parse URI with scheme other than \"file\" {0}", URI);
+
This revision was automatically updated to reflect the committed changes.
Closed by commit rG93bb9944cb57: [clangd] Implement path and URI translation
for remote index (authored by kbobyrev).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
kbobyrev updated this revision to Diff 276692.
kbobyrev marked 15 inline comments as done.
kbobyrev added a comment.
Address post-LGTM round of comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Rest is just nits, thanks!
Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:38
+/// provided by the client.
+llvm::Optional
kbobyrev updated this revision to Diff 276647.
kbobyrev added a comment.
Replace Sym in Client response with a more generic name (it can be a reference).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
kbobyrev updated this revision to Diff 276646.
kbobyrev added a comment.
Rebase on top of master
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
Files:
clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
kbobyrev updated this revision to Diff 276642.
kbobyrev added a comment.
Fix formatting on one line.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
Files:
kbobyrev updated this revision to Diff 276454.
kbobyrev marked 10 inline comments as done.
kbobyrev added a comment.
Address the rest of the comments and refactor code.
Patch is ready for the review again.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
kbobyrev marked 2 inline comments as done.
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:25
+const char *unittestURIToFilesystem(const char *UnittestURI,
+llvm::UniqueStringSaver )
kbobyrev updated this revision to Diff 276366.
kbobyrev marked 12 inline comments as done.
kbobyrev added a comment.
Store progress. Still WIP.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
Files:
sammccall added a comment.
This looks better to me but:
- i think we need to consider what happens when paths are not under the
expected path, and handle that in some appropriate way
- I find it hard to see what the tests are actually testing - would be good to
make them more explicit, using a
kbobyrev updated this revision to Diff 276030.
kbobyrev added a comment.
Add assertions for ensuring paths have UNIX slashes during deserialization.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
Files:
kbobyrev updated this revision to Diff 276029.
kbobyrev added a comment.
Convert prefix paths to native slash format for _serialization_ calls
(deserializations should use UNIX slashes).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
kbobyrev updated this revision to Diff 275945.
kbobyrev added a comment.
Don't convert test paths to system native, UNIX native is good enough.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
Files:
kbobyrev updated this revision to Diff 275670.
kbobyrev added a comment.
Remove IndexRoot conversion to UNIX slashes from the Marshalling hot path.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
Files:
kbobyrev updated this revision to Diff 275667.
kbobyrev marked 12 inline comments as done.
kbobyrev added a comment.
Resolve the rest of patch comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
kbobyrev updated this revision to Diff 275672.
kbobyrev added a comment.
Use std::string() instead of static_cast()
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
Files:
kbobyrev updated this revision to Diff 275613.
kbobyrev marked 19 inline comments as done.
kbobyrev added a comment.
Store the progress so that I don't lose it. Still WIP.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:61
+ if (IndexedProjectRoot.empty())
+return static_cast(Result);
+ llvm::sys::path::replace_path_prefix(Result, IndexedProjectRoot, "");
sammccall added a comment.
Generally I think this is missing high-level documentation describing the
translation that happens at each level - I had to refer back to the notes from
our meeting to understand what the scheme was.
I think Marshalling.h is probably a good place to document how this
kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous,
MaskRay, ilya-biryukov.
Herald added a project: clang.
kbobyrev updated this revision to Diff 274719.
kbobyrev added a comment.
Herald added a
kbobyrev updated this revision to Diff 274719.
kbobyrev added a comment.
Herald added a subscriber: ormris.
%s/PrefixPath/ProjectRoot/g
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82938/new/
https://reviews.llvm.org/D82938
Files:
22 matches
Mail list logo
