I've sent r325482 which (hopefully) fixes the failure. I'll keep an eye on
the bot to make sure this is fixed ASAP.
Sorry about the inconvenience!
o Eric
On Mon, Feb 19, 2018 at 11:39 AM Carlos Alberto Enciso via Phabricator <
revi...@reviews.llvm.org> wrote:
> CarlosAlbertoEnciso added a
CarlosAlbertoEnciso added a subscriber: aheejin.
CarlosAlbertoEnciso added a comment.
Thanks @aheejin
Repository:
rL LLVM
https://reviews.llvm.org/D42640
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Hi Carlos, did this fail on a Windows build bot? Would you mind pointing me
to the broken test? Thanks!
On Fri, Feb 16, 2018 at 4:23 PM Carlos Alberto Enciso via Phabricator <
revi...@reviews.llvm.org> wrote:
> CarlosAlbertoEnciso added a comment.
>
> Hi @ioeric:
>
> Just to let you know that
CarlosAlbertoEnciso added a comment.
Hi @ioeric:
Just to let you know that your submission seems to break a Windows test.
FAIL: Extra Tools Unit Tests ::
clangd/Checking/./ClangdTests.exe/SymbolCollectorTest.IWYUPragma (15474 of
36599)
Thanks
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE325343: [clangd] collect symbol #include insert
#include in global code completion. (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D42640?vs=134600=134603#toc
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325343: [clangd] collect symbol #include insert
#include in global code completion. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
ioeric updated this revision to Diff 134600.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- Merged with origin/master
- Addressed review comments; removed .inc handling.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42640
Files:
clangd/CMakeLists.txt
ioeric added a comment.
Thank you for reviewing this!
Comment at: clangd/index/CanonicalIncludes.h:52
+ /// a canonical header name.
+ llvm::StringRef mapHeader(llvm::StringRef Header) const;
+
sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > So
sammccall added inline comments.
Comment at: unittests/clangd/HeadersTests.cpp:29
+ // absolute path.
+ std::string addToSubdir(PathRef File, llvm::StringRef Code = "") {
+assert(llvm::sys::path::is_relative(File) && "FileName should be
relative");
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
LG apart from the .inc handling (happy to chat more)
Comment at: clangd/index/CanonicalIncludes.h:52
+ /// a canonical header name.
+ llvm::StringRef
ioeric updated this revision to Diff 134394.
ioeric added a comment.
- Merged with origin/master
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42640
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
ioeric updated this revision to Diff 133895.
ioeric marked 15 inline comments as done.
ioeric added a comment.
- Addressed some review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42640
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
ioeric added inline comments.
Comment at: clangd/ClangdServer.cpp:370
+/// File, by matching \p Header against all include search directories for \p
+/// File via `clang::HeaderSearch`.
+///
sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > sammccall
sammccall added a comment.
Insertion still LG (couple of nits, inline).
For indexing, my biggest questions:
- I worry CanonicalIncludes doesn't get enough information to make good
decisions - passing the include stack in some form may be better
- CanonicalIncludes has slightly weird patterns
ioeric added inline comments.
Comment at: clangd/ClangdServer.cpp:368
+/// Calculates the shortest possible include path when inserting \p Header to
\p
+/// File, by matching \p Header against all include search directories for \p
sammccall wrote:
> ioeric
ioeric updated this revision to Diff 133635.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- Addressed review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42640
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
sammccall added a comment.
Insertion side LGTM, feel free to split and land.
Sorry I need to take off and will need to get to indexing on monday :(
Comment at: clangd/ClangdServer.cpp:368
+/// Calculates the shortest possible include path when inserting \p Header to
\p
+///
ioeric updated this revision to Diff 133599.
ioeric added a comment.
- fix a leftover bug
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42640
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
ioeric added a comment.
Thanks! PTAL
Comment at: clangd/ClangdServer.cpp:465
+auto = Clang->getPreprocessor().getHeaderSearchInfo();
+std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics(
+*Resolved, CompileCommand.Directory);
ioeric updated this revision to Diff 133595.
ioeric marked 5 inline comments as done.
ioeric added a comment.
- Addressed review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42640
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
sammccall added a comment.
Some comments on the insert side, which looks pretty good. I'll take another
look at indexing tomorrow.
Comment at: clangd/ClangdServer.cpp:465
+auto = Clang->getPreprocessor().getHeaderSearchInfo();
+std::string Suggested =
21 matches
Mail list logo
