[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-02 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D50154#1185545, @ilya-biryukov wrote: > Maybe we could simply always capitalize the messages? Any cons to > capitalizing error messages? > @simark, @malaperle, @ioeric, @hokein, WDYT? Oh sorry, I thought `-capitialize-diagnostic-text` was an

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

2018-08-02 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: ilya-biryukov, hokein. Herald added a subscriber: cfe-commits. For example, when renaming `a::b::x::foo` to `y::foo` below, replacing `x::foo()` with `y::foo()` can cause ambiguity. In such cases, we simply fully qualify the name with leading `

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

2018-08-02 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 158761. ioeric added a comment. - minor update to comment. Repository: rC Clang https://reviews.llvm.org/D50189 Files: lib/Tooling/Core/Lookup.cpp unittests/Tooling/LookupTest.cpp Index: unittests/Tooling/LookupTest.cpp ==

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

2018-08-03 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 158941. ioeric marked 2 inline comments as done. ioeric added a comment. - address review comments. Repository: rC Clang https://reviews.llvm.org/D50189 Files: lib/Tooling/Core/Lookup.cpp unittests/Tooling/LookupTest.cpp Index: unittests/Tooling/Look

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

2018-08-03 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: lib/Tooling/Core/Lookup.cpp:139 +const NamespaceDecl *NS = *I; +auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head))); +if (!LookupRes.empty()) { ilya-biryukov wrote: > This will not take using n

[PATCH] D50189: Fully qualify the renamed symbol if the shortened name is ambiguous.

2018-08-03 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL338832: Fully qualify the renamed symbol if the shortened name is ambiguous. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-03 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: test/clangd/capitalize-diagnostics.test:1 +# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} -

[PATCH] D50154: [clangd] capitalize diagnostic messages if '-capitialize-diagnostic-text' is provided

2018-08-03 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/Diagnostics.cpp:149 +/// Capitalizes the first word in the diagnostic's message. +std::string capitalizeDiagnosticText(std::string Message) { + if (!

[PATCH] D50331: [clangd] *Prototype* Drop dynamic index symbols from files that are already indexed in static index.

2018-08-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: ilya-biryukov, hokein. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Currently, dynamic index collects symbols for the entire TU for each open/active file. When static index is enabled, this can be wasteful as (intuitively)

[PATCH] D50331: [clangd] *Prototype* Drop dynamic index symbols from files that are already indexed in static index.

2018-08-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Code is not polished. I'd like to get some high-level feedback before proceeding further and splitting this into smaller patches. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50331 ___ cfe-commits mailing

[PATCH] D42942: [clangd] Collect definitions when indexing.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. looks good Comment at: clangd/index/Index.h:25 struct SymbolLocation { // The absolute path of the source file where a symbol occurs. It might be worth mentioning here whether the range covers the entire declaration/definition bod

[PATCH] D42919: [clangd] Support simpler JSON-RPC stream parsing for lit tests.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. LGTM Have we kept a lit test that uses content-length? It's unclear from the patch. Comment at: clangd/tool/ClangdMain.cpp:89 +static llvm::cl::opt Test( +"test", +l

[PATCH] D42913: [clangd] Fix incorrect file path for symbols defined by the compile command-line option.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg Comment at: clangd/index/SymbolCollector.cpp:132 +// * symbols controlled and defined by a compile command-line option +// `-DName=foo`, the spelling locatio

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/Index.h:27 + // The URI of the source file where a symbol occurs. + llvm::StringRef FileUri; // The 0-based offset to the first character of the symbol from the beginning sammccall wrote: > nit: FileURI?

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 132958. ioeric marked 2 inline comments as done. ioeric added a comment. - Make URIScheme customizable in SymbolCollector. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42915 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/i

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. I was thinking about leaving URI scheme customization to the postprocessing phase, but you are right, it would be better to make the URI scheme extendable in SymbolCollector. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42915 __

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/Index.h:27 + // The URI of the source file where a symbol occurs. + llvm::StringRef FileUri; // The 0-based offset to the first character of the symbol from the beginning hokein wrote: > sammccall wrote:

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 133000. ioeric added a comment. - s/Uri/URI/ - Address review comments. Support multiple schemes. - Merged with origin/master - Merge branch 'master' of http://llvm.org/git/clang-tools-extra into uri Repository: rCTE Clang Tools Extra https://reviews.llvm.

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 133009. ioeric added a comment. - removed leftover logs. - Merge branch 'master' of http://llvm.org/git/clang-tools-extra into uri Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42915 Files: clangd/index/Index.cpp clangd/index/Index.h

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL324358: [clangd] Use URIs in index symbols. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D42915 Files: clang-tools-extra

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 133021. ioeric marked 10 inline comments as done. ioeric added a comment. - Merge with origin/master - Renamed and moved a bunch files according to review comments. - Merge remote-tracking branch 'origin/master' into include - Half way - Merge with uri changes.

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Thanks for the comments! I addressed comments in the symbol collect part (I think?). Will add tests in a followup patch. Comment at: clangd/index/HeaderMapCollector.h:48 + // A map from header patterns to header names. + // The header names are not ow

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Thanks for the comments! I have addressed comments in the symbol collect side (I think?). Will add tests in a followup patch. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 ___ cfe-commits mailing list

[PATCH] D42942: [clangd] Collect definitions when indexing.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67 + // XXX this is just to make running the tool fast during dev! + bool BeginInvocation(CompilerInstance &CI) override { +const auto &Inputs = CI.getInvocation()

[PATCH] D43009: [clangd] Do not precent-encode numbers in URI.

2018-02-07 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, jkorous-apple, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43009 Files: clangd/URI.cpp unittests/clangd/URITests.cpp Index: unittests/clangd/URITests.cpp

[PATCH] D43009: [clangd] Do not precent-encode numbers in URI.

2018-02-07 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL324475: [clangd] Do not precent-encode numbers in URI. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43009 Files: clang-

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

2018-02-07 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 133202. ioeric added a comment. - Added tests for IncludeURI and CanonicalIncludes and minor cleanup. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

2018-02-07 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 133203. ioeric added a comment. - Merge 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 clangd/CodeComple

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

2018-02-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 133411. ioeric marked 13 inline comments as done. ioeric added a comment. - Added tests for all components; more cleanup; s/IncludeURI/IncludeHeader/ Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42640 Files: clangd/CMakeLists.txt clang

[PATCH] D42640: [clangd] Prototype: collect symbol #include & insert #include in global code completion.

2018-02-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Thanks for the comments! Sorry that I didn't clean up the code before sending out the prototype. I planned to deal with code structure and style issues after getting some early feedback, but I think the patch is ready for review now. Comment at: clang

[PATCH] D43065: [clangd] Remove threading-related code from ClangdUnit.h

2018-02-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Nice! The code looks much simpler! Just some drive-by nits. I don't know the threading work well enough to give useful comments. Will leave the approval to others. Comment at: clangd/ClangdUnit.cpp:399 + std::unique_ptr CI; + { +// FIXME(ibiryuko

[PATCH] D43068: [clangd] Remove codeComplete that returns std::future<>

2018-02-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: unittests/clangd/SyncAPI.h:8 +// +//===-===// +#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H Any reason not to put sync APIs in ClangdServer

[PATCH] D43065: [clangd] Remove threading-related code from ClangdUnit.h

2018-02-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdUnit.cpp:399 + std::unique_ptr CI; + { +// FIXME(ibiryukov): store diagnostics from CommandLine when we start ilya-biryukov wrote: > ioeric wrote: > > Do we still need this block? > I added it to avoid

[PATCH] D43075: [clang-move] Don't dump macro symbols.

2018-02-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Lg. Thanks for the change! Comment at: clang-move/ClangMove.cpp:526 unless(usingDirectiveDecl()), // using namespace decl. + notInMacro(), InOldHeader, I'd probably relax the condition a bit; theoretically tools would

[PATCH] D43068: [clangd] Remove codeComplete that returns std::future<>

2018-02-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: unittests/clangd/SyncAPI.h:8 +// +//===-===// +#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H ilya-biryukov wrote: > ioeric wrote: > > Any rea

[PATCH] D43068: [clangd] Remove codeComplete that returns std::future<>

2018-02-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: unittests/clangd/SyncAPI.h:8 +// +//===-===// +#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H ilya-biryukov wrote: > ioeric wrote: > > ilya-bi

[PATCH] D42942: [clangd] Collect definitions when indexing.

2018-02-09 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. Lg Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67 + // XXX this is just to make running the tool fast during dev! + bool BeginInvocation(CompilerInstance &CI) override { +const aut

[PATCH] D43075: [clang-move] Don't dump macro symbols.

2018-02-09 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Ooops, forgot to stamp! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43075 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-09 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-09 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Thanks! PTAL Comment at: clangd/ClangdServer.cpp:465 +auto &HeaderSearchInfo = Clang->getPreprocessor().getHeaderSearchInfo(); +std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics( +*Resolved, CompileCommand.Directory

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-09 Thread Eric Liu via Phabricator via cfe-commits
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 clangd/CodeComplete.cpp

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-09 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-09 Thread Eric Liu via Phabricator via cfe-commits
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 wro

[PATCH] D43174: [clang-move] Fix the incorrect expansion end location.

2018-02-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. The fix is quite subtle and not obvious to tell from the test. Could you elaborate on the issue this fixes in the patch description? Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43174 ___ cfe-commi

[PATCH] D43174: [clang-move] Fix the incorrect expansion end location.

2018-02-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43174 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D39050: Add index-while-building support to Clang

2018-02-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. I'm wondering if there is any further plan for this? ;) https://reviews.llvm.org/D39050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-12 Thread Eric Liu via Phabricator via cfe-commits
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 wr

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-12 Thread Eric Liu via Phabricator via cfe-commits
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 clangd/ClangdServe

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. I think it's useful to explicit initialize non-trivial types like `FileChangeType`. But I think it's safe and probably easier to rely on default values of primitive types like int, bool etc. Explicitly setting default values is probably fine (so this change LGTM), but do

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:194 +if (!Replacements) + return replyError(ErrorCode::InternalError, +llvm::toString(Replacements.takeError())); nit: since we are not spelling out

[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D43230#1006469, @ilya-biryukov wrote: > In https://reviews.llvm.org/D43230#1006104, @ioeric wrote: > > > But I think it's safe and probably easier to rely on default values of > > primitive types like int, bool etc > > > It's not always safe, a

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. I think another option to prevent the bug in r325029 from happening would be providing a canonical approach for retrieving (absolute) paths from `FileManager`. We already have code in symbol collector that does this, and we have similar code in XRefs.cpp now. We should p

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:194 +if (!Replacements) + return replyError(ErrorCode::InternalError, +llvm::toString(Replacements.takeError())); ilya-biryukov wrote: > ioeric wrote

[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. lg Comment at: clangd/ClangdServer.cpp:209 + auto Action = [Contents, Pos, TaggedFS, + PCHs](Path File, decltype(Callback) Callback, + llvm::Expected IP) { ilya-biryuk

[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.

2018-02-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/Protocol.h:52 struct URIForFile { + URIForFile() = default; ilya-biryukov wrote: > sammccall wrote: > > I don't like how the API changes here take us further away from the other > > structs in this file. > > W

[PATCH] D43229: [clangd] Enable snippet completion based on client capabilities.

2018-02-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Comment at: clangd/ClangdLSPServer.cpp:90 void ClangdLSPServer::onInitialize(InitializeParams &Params) { + if (Params.rootUri && !Params.rootUri->file.empty()) +Se

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-15 Thread Eric Liu via Phabricator via cfe-commits
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 clangd/CodeCompl

[PATCH] D43381: [clangd] Fix use-after-free in SymbolYAML: strings are owned by yaml::Input!

2018-02-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Thanks a lot for the fix! Suggest a test case which reproduced the bug: // comment with 'quote' void f() {} `runSymbolCollector` -> `SymbolToYAML` -> `SymbolFromYAML` -> check `Documenta

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-16 Thread Eric Liu via Phabricator via cfe-commits
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 cl

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-16 Thread Eric Liu via Phabricator via cfe-commits
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 I'

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-16 Thread Eric Liu via Phabricator via cfe-commits
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 https://reviews.l

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-16 Thread Eric Liu via Phabricator via cfe-commits
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&id=134603#

[PATCH] D43429: [clangd] Add missing library (clangLex) in a few places

2018-02-17 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D43437: Fix link failures for Preprocessor::addCommentHandler

2018-02-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a subscriber: malaperle. ioeric added a comment. This revision is now accepted and ready to land. lg. Thanks! fyi, @malaperle also sent https://reviews.llvm.org/D43429 for the same fix but has not landed it yet ;) Repository: rCTE Clang Tools Extra

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: sammccall, ilya-biryukov. Herald added subscribers: cfe-commits, jkorous-apple, klimek. o Avoid inserting a header include into the header itself. o Avoid inserting non-header files. o Canonicalize include paths for symbols in dynamic index.

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/Headers.cpp:45 +bool hasHeaderExtension(PathRef Path) { + constexpr static const char *HeaderExtensions[] = {".h", ".hpp", ".hh", + ".hxx"}; ilya-biryukov wrote:

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 134932. ioeric marked 6 inline comments as done. ioeric added a comment. - Stop indexing main files in dynamic index; addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43462 Files: clangd/ClangdServer.cpp clangd/H

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 134943. ioeric marked 3 inline comments as done. ioeric added a comment. - addressed comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43462 Files: clangd/ClangdServer.cpp clangd/Headers.cpp clangd/Headers.h clangd/index/Canon

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric marked 5 inline comments as done. ioeric added inline comments. Comment at: clangd/Headers.cpp:60 Argv.push_back(S.c_str()); IgnoringDiagConsumer IgnoreDiags; auto CI = clang::createInvocationFromCommandLine( ilya-biryukov wrote: > Not related t

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 134944. ioeric marked 2 inline comments as done. ioeric added a comment. - added a comment about thread safety Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43462 Files: clangd/ClangdServer.cpp clangd/Headers.cpp clangd/Headers.h cl

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 134947. ioeric added a comment. - assert in the very beginning! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43462 Files: clangd/ClangdServer.cpp clangd/Headers.cpp clangd/Headers.h clangd/index/CanonicalIncludes.cpp clangd/index

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL325523: [clangd] Fixes for #include insertion. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43462 Files: clang-tools-ex

[PATCH] D43462: [clangd] Fixes for #include insertion.

2018-02-19 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325523: [clangd] Fixes for #include insertion. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D43462?vs=134947&id=134952#toc Repository: rL LLVM https://rev

[PATCH] D43510: [clangd] don't insert new includes if either original header or canonical header is already included.

2018-02-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. Changes: o Store both the original header and the canonical header in LSP command. o Also check that both original and canonical headers are not already in

[PATCH] D43510: [clangd] don't insert new includes if either original header or canonical header is already included.

2018-02-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdServer.h:243 + /// string quoted with <> or "" that can be #included directly. + /// \p PreferredHeader The preferred header to be inserted. The has the same + /// semantic as \p OriginalHeader. If empty, \p OriginalHeader

[PATCH] D43510: [clangd] don't insert new includes if either original header or canonical header is already included.

2018-02-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 135074. ioeric marked 16 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43510 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

2018-02-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: ilya-biryukov, sammccall, hokein. Herald added subscribers: cfe-commits, jkorous-apple, klimek. The new behaviors introduced by this patch: o When include collection is enabled, we always set IncludeHeader field in Symbol even if it's the same

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

2018-02-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D43550#1014319, @ilya-biryukov wrote: > Is there a way to still enable include insertion but in a restricted manner, > e.g. only for the current project? I'm not sure if this is currently supported, as we don't have a good definition of a "p

[PATCH] D43567: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.

2018-02-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: bkramer. Herald added subscribers: cfe-commits, klimek. Example: template class X {}; class A {}; // Explicit instantiation declaration. extern template class X; Repository: rC Clang https://reviews.llvm.org/D43567 Files: include/cla

[PATCH] D43567: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation declaration.

2018-02-21 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL325678: [ASTMatchers] isTemplateInstantiation: also match explicit instantiation… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

2018-02-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/FileIndex.cpp:18 -const CanonicalIncludes *canonicalIncludesForSystemHeaders() { - static const auto *Includes = [] { ilya-biryukov wrote: > Should we also remove the mutex from `CanonicalIncludes` now? I

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

2018-02-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 135244. ioeric marked 2 inline comments as done. ioeric added a comment. Properly use toURI. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43550 Files: clangd/CodeComplete.cpp clangd/index/FileIndex.cpp clangd/index/Index.h clangd/i

[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

2018-02-22 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL325764: [clangd] Not collect include headers for dynamic index for now. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43550

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2018-02-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D43500#1015208, @jdemeule wrote: > In https://reviews.llvm.org/D43500#1013558, @malcolm.parsons wrote: > > > In https://reviews.llvm.org/D43500#1013497, @aaron.ballman wrote: > > > > > Is there a way to make clang-apply-replacements smarter rath

[PATCH] D43634: [clangd] Extend textDocument/didChange to specify whether diagnostics should be generated.

2018-02-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek. This would allow us to disable diagnostics when didChange is called but diagnostics are not wanted (e.g. code completion). Repository: rCTE Clang Tools

[PATCH] D43634: [clangd] Extend textDocument/didChange to specify whether diagnostics should be generated.

2018-02-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 135463. ioeric marked 2 inline comments as done. ioeric added a comment. Addressed review comments. Removed test. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43634 Files: clangd/ClangdLSPServer.cpp clangd/Protocol.cpp clangd/Protoco

[PATCH] D43634: [clangd] Extend textDocument/didChange to specify whether diagnostics should be generated.

2018-02-22 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE325813: [clangd] Extend textDocument/didChange to specify whether diagnostics should be… (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D43634?vs=135463&id=1354

[PATCH] D43671: [clangd] Address FIXME and fix comment

2018-02-23 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Thanks for the cleanup Kirill :) Comment at: clangd/tool/ClangdMain.cpp:153 + if (RunSynchronously) { +if (WorkerThreadsCount != 0) { + llvm::errs() `-j` is non-zero by default, and we shouldn't show warning if users only spec

[PATCH] D43671: [clangd] Address FIXME and fix comment

2018-02-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Oops, just realized I forgot to push the "send" button! Comment at: clangd/tool/ClangdMain.cpp:153 + if (RunSynchronously) { +if (WorkerThreadsCount.getNumOccurrences())

[PATCH] D43510: [clangd] don't insert new includes if either original header or canonical header is already included.

2018-02-26 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 135857. ioeric marked an inline comment as done. ioeric added a comment. - Merge with origin/master - Use llvm::StringSet Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43510 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp cl

[PATCH] D43510: [clangd] don't insert new includes if either original header or canonical header is already included.

2018-02-26 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL326070: [clangd] don't insert new includes if either original header or canonical… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

2017-06-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Lgtm. Comment at: lib/Format/Format.cpp:1406 for (const auto &Category : Style.IncludeCategories) - CategoryRegexs.emplace_back(Category.Regex); + CategoryRege

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

2017-06-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: unittests/Format/SortIncludesTest.cpp:269 + // Support case-sensitive and case insensitive matching. + Style.IncludeRegexCaseInsensitive = false; + EXPECT_EQ("#include \"GTest/GTest.h\"\n" I think this TEST is only for

[PATCH] D34252: Add arbitrary file/path support to clang-format style file selection

2017-06-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Hi Dan, thanks for the patch! https://reviews.llvm.org/D33560, which adds the same feature, is already under review :) https://reviews.llvm.org/D34252 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

2017-12-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 126916. ioeric marked 7 inline comments as done. ioeric added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/index/Index.h clangd/index/MemIndex.cpp clang

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

2017-12-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 126917. ioeric marked an inline comment as done. ioeric added a comment. - Fix comment for fuzzyFind Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Files: clangd/CMakeLists.txt clangd/index/Index.h clangd/index/MemIndex.cpp cla

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

2017-12-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 126919. ioeric marked 3 inline comments as done. ioeric added a comment. - Address more comments. # I am going to land it now. - Merge remote-tracking branch 'origin/master' into index Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D40548 Fil

[PATCH] D40548: [clangd] Symbol index interfaces and an in-memory index implementation.

2017-12-14 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE320688: [clangd] Symbol index interfaces and an in-memory index implementation. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D40548?vs=126919&id=126924#toc R

[PATCH] D41232: [clangd] Add a FileSymbols container that manages symbols from multiple files.

2017-12-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, ilya-biryukov, mgorny, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41232 Files: clangd/CMakeLists.txt clangd/index/FileSymbols.cpp clangd/index/FileSymbols.h

[PATCH] D41232: [clangd] Add a FileSymbols container that manages symbols from multiple files.

2017-12-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 126943. ioeric marked 2 inline comments as done. ioeric added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41232 Files: clangd/CMakeLists.txt clangd/index/FileSymbols.cpp clangd/index/FileSymbols.

<    1   2   3   4   5   6   7   8   9   10   >