[PATCH] D54519: [clangd] Fix no results returned for global symbols in dexp

2018-11-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/dex/dexp/Dexp.cpp:64 +// add the global scope to the request. +Request.Scopes = {""}; ioeric wrote: > I think the old

[PATCH] D54519: [clangd] Fix no results returned for global symbols in dexp

2018-11-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/dex/dexp/Dexp.cpp:64 +// add the global scope to the request. +Request.Scopes = {""}; I think the old behavior used `AnyScope` for a unqualified name. Do we want to keep that? Repository: rCTE

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

2018-11-14 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/ASTMatchers/ASTMatchFinder.cpp:674 + // Nodes may have no parents if: + // a) the node is the TranslationUnitDecl + // a) there is a bug

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

2018-11-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a subscriber: thakis. ioeric added a comment. The new API and the refactoring look good to me. Just a nit and a question. Comment at: lib/AST/ASTContext.cpp:10292 + if (!Parents) // We always need to run over the whole translation unit, as // hasAncestor

[PATCH] D54427: [clangd] Allow symbols from AnyScope in dexp.

2018-11-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/dex/dexp/Dexp.cpp:55 FuzzyFindRequest Request; + Request.AnyScope = true; // Remove leading "::" qualifier as FuzzyFind doesn't need leading "::" I don't think you would want AnyScope here. For example

[PATCH] D52273: [clangd] Initial implementation of expected types

2018-11-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ExpectedTypes.cpp:27 +return llvm::None; + auto *VD = llvm::dyn_cast(R.Declaration); + if (!VD) maybe add a comment what `ValueDecl` covers roughly? E.g. functions, classes, variables etc.

[PATCH] D54300: [clangd] Respect shouldIndexFile when collecting symbols.

2018-11-09 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:217 +bool shouldIndexFile(const Decl& D, const SymbolCollector::Options &Opts, + llvm::DenseMap *FilesToIndexCache) { nit: this is very easily confused with the callb

[PATCH] D54300: [clangd] Respect shouldIndexFile when collecting symbols.

2018-11-09 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:555 + auto Loc = findNameLoc(&ND); + if (!shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache)) +return nullptr; Should we use `getTokenLocation` like what we do below?

[PATCH] D54105: [clangd] Deduplicate query scopes.

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL346224: [clangd] Deduplicate query scopes. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D54105 Files: clang-tools-extra/

[PATCH] D54106: [clangd] Limit the index-returned results in dexp.

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/dex/dexp/Dexp.cpp:166 + cl::init(10), + cl::desc("Max results to display. This flag is only meaningful when -name" + " is set."), Maybe `The max number of symbols with the same name b

[PATCH] D53933: [clangd] Get rid of QueryScopes.empty() == AnyScope special case.

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE346223: [clangd] Get rid of QueryScopes.empty() == AnyScope special case. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D53933?vs=172735&id=172737#toc Reposit

[PATCH] D53933: [clangd] Get rid of QueryScopes.empty() == AnyScope special case.

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 172735. ioeric added a comment. - rebase Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53933 Files: clangd/FindSymbols.cpp clangd/index/Index.h clangd/index/MemIndex.cpp clangd/index/dex/Dex.cpp unittests/clangd/DexTests.cpp uni

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL346221: [clangd] auto-index stores symbols per-file instead of per-TU. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53433

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 172732. ioeric added a comment. - Rebase Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/index/Background.h clangd/index/FileIndex.cpp clangd/index/FileIndex.h clangd/index/SymbolColle

[PATCH] D54077: [clangd] Implemented DraftFileSystem

2018-11-06 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D54077#1288387, @sammccall wrote: > Someone mentioned to me that the interaction-between-features argument wasn't > clear here: > > - we **don't** currently update diagnostics for `A.cc` when `A.h` is edited > - we should, this seems more obvio

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/Background.cpp:235 +IndexedFileDigests[Path] = FilesToUpdate.lookup(Path); +IndexedSymbols.update(Path, + make_unique(std::move(Syms).build()), kadircet wrote: > This call is

[PATCH] D54106: [clangd] Limit the index-returned results in dexp.

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/dex/dexp/Dexp.cpp:128 }; cl::opt Limit{ "limit", I think we should make it configurable like what we do here. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54106 __

[PATCH] D53933: [clangd] Get rid of QueryScopes.empty() == AnyScope special case.

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 172591. ioeric marked an inline comment as done. ioeric added a comment. - Remove addressed FIXME. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53933 Files: clangd/FindSymbols.cpp clangd/index/Index.h clangd/index/MemIndex.cpp clan

[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric abandoned this revision. ioeric marked an inline comment as done. ioeric added inline comments. Comment at: clangd/CodeComplete.cpp:563 for (auto *Context : CCContext.getVisitedContexts()) { - if (isa(Context)) + if (isa(Context)) { Info.AccessibleS

[PATCH] D54105: [clangd] Deduplicate query scopes.

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay. For example, when anonymous namespace is present, duplicated namespaces might be generated for the enclosing namespace. Repository: rCTE Clang Tool

[PATCH] D54104: [Tooling] Correct the total number of files being processed when `filter` is provided.

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: lib/Tooling/AllTUsExecution.cpp:103 +if (!RegexFilter.match(File)) + continue; +Files.push_back(File); nit: `if (match) then pus

[PATCH] D54092: [Tooling] Add "-filter" option to AllTUsExecution

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. maybe add a test? Repository: rC Clang https://reviews.llvm.org/D54092 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54092: [Tooling] Add "-filter" option to AllTUsExecution

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: lib/Tooling/AllTUsExecution.cpp:120 for (std::string File : Files) { + if (Filter.getNumOccurrences() != 0 && !RegexFilter.match(File)) +con

[PATCH] D54092: [Tooling] Add "-filter" option to AllTUsExecution

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: lib/Tooling/AllTUsExecution.cpp:120 for (std::string File : Files) { + if (Filter.getNumOccurrences() != 0 && !RegexFilter.match(File)) +continue; hokein wrote: > ioeric wrote: > > > `Filter.getNumOccurr

[PATCH] D54092: [Tooling] Add "-filter" option to AllTUsExecution

2018-11-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: lib/Tooling/AllTUsExecution.cpp:58 +"filter", +llvm::cl::desc("Only process files that match this filter"), +llvm::cl::init(".*")); Please also mention that this only applies to all-TUs. Com

[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric planned changes to this revision. ioeric added a comment. I got the behavior of `printNamespaceScope` wrong in this patch. Will update. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53926 ___ cfe-commits mailing list cfe-com

[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 171928. ioeric added a comment. - revert wrong comment Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53926 Files: clangd/AST.h clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.

[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 171924. ioeric added a comment. - Clarify documentation for printNamespaceScope Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53926 Files: clangd/AST.h clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/

[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/CodeComplete.cpp:563 for (auto *Context : CCContext.getVisitedContexts()) { - if (isa(Context)) + if (isa(Context)) { Info.AccessibleScopes.push_back(""); // global namespace ilya-biryukov w

[PATCH] D53933: [clangd] Get rid of QueryScopes.empty() == AnyScope special case.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53933 Files: clangd/FindSymbols.cpp clangd/index/Index.h clangd/inde

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Made some more changes to make the code work in multiple threads (add mutex for digests, take snapshot of digests for each run etc). PTAL Comment at: clangd/index/Background.cpp:311 SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs())); - // FIXME: parti

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 171901. ioeric marked 3 inline comments as done. ioeric added a comment. - Merged with multi-threading changes. Added mutex for file digests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/

[PATCH] D53926: [clangd] Only add global scope to completion query scopes for TU context.

2018-10-31 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. This avoids duplicated scopes in the query e.g. when anonymous namespace is present. Repository: rCTE Clang Tools Extra https://review

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/Background.cpp:311 SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs())); - // FIXME: partition the symbols by file rather than TU, to avoid duplication. - IndexedSymbols.update(AbsolutePath, -llvm::ma

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-30 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 171684. ioeric marked 11 inline comments as done. ioeric added a comment. - address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/index/Background.h clangd/index/FileInd

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-29 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 171467. ioeric added a comment. - minor cleanup and a friendly ping. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/index/Background.h clangd/index/FileIndex.cpp clangd/index/FileIndex.

[PATCH] D53638: [clangd] Downrank members from base class

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL345140: [clangd] Downrank members from base class (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53638 Files: clang-tools

[PATCH] D53638: [clangd] Downrank members from base class

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/Quality.cpp:380 + if (InBaseClass) +Score *= 0.7; + sammccall wrote: > This seems like a pretty light penalty to me, I'd consider 0.5... 0.5 sounds reasonable. I think we should penalize the non-instance membe

[PATCH] D53638: [clangd] Downrank members from base class

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170870. ioeric marked an inline comment as done. ioeric added a comment. - adjust parameter Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53638 Files: clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests.cpp Index: unit

[PATCH] D53571: [clangd] Don't show base class versions of members as completions.

2018-10-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. The diff seems to be wrong. Please rebase. Comment at: clangd/CodeComplete.cpp:732 + // Class members that are shadowed by subclasses are usually noise. + if (Resul

[PATCH] D53638: [clangd] Downrank members from base class

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170862. ioeric marked an inline comment as done. ioeric added a comment. - restore accidentally removed test. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53638 Files: clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests

[PATCH] D53638: [clangd] Downrank members from base class

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric marked an inline comment as done. ioeric added inline comments. Comment at: unittests/clangd/QualityTests.cpp:187 Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42)); - EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope); } i

[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:112 +SPAN_ATTACH(Tracer, "Params", Params); +ReplyOnce Reply(ID, Method, &Server, Tracer.Args); log("<-- {0}({1})", Method, ID); Do we have guarantee that `Tracer.Args` outlives `Re

[PATCH] D53571: [clangd] Don't show base class versions of members as completions.

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. > Keep required quailifier machinery around though, for cross-ns completion. Do we have cross-ns completion in sema? Comment at: clangd/CodeComplete.cpp:732 + // Class members that are shadowed by subclasses are usually noise. + if (Result.Hid

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL345135: [CodeComplete] Expose InBaseClass signal in code completion results. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: lib/Sema/CodeCompleteConsumer.cpp:557 + if (!Tags.empty()) +OS << " (" << llvm::join(Tags, ",") << ")"; + if (CodeCompletionString *CCS = Results[I].CreateCodeCompletionString( ilya-biryukov wrote: > NIT

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170855. ioeric added a comment. - Rebase Repository: rC Clang https://reviews.llvm.org/D53635 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/CodeCompleteConsumer.cpp lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/member-access.cpp

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170854. ioeric marked an inline comment as done. ioeric added a comment. - move tags into case. Repository: rC Clang https://reviews.llvm.org/D53635 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/CodeCompleteConsumer.cpp lib/Sema/SemaCode

[PATCH] D53638: [clangd] Downrank members from base class

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: sammccall, ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay. ioeric added a dependency: D53635: [CodeComplete] Expose InBaseClass signal in code completion results.. Repository: rCTE Clang Tools Extr

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D53635#1273904, @ilya-biryukov wrote: > LG, but could we add a test for the new flag it by printing it in > `PrintingCodeCompleteConsumer::ProcessCodeCompleteResults()` and adding > corresponding tests to `clang/test/CodeCompletion`? > > Simil

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170838. ioeric added a comment. - Add tests for the new flag. Repository: rC Clang https://reviews.llvm.org/D53635 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/CodeCompleteConsumer.cpp lib/Sema/SemaCodeComplete.cpp test/CodeCompletion

[PATCH] D53635: [CodeComplete] Expose InBaseClass signal in code completion results.

2018-10-24 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added a subscriber: cfe-commits. No new tests as the existing tests for result priority should give us coverage. Also as the new flag is trivial enough, I'm reluctant to plumb the flag to c-index-test output. Repository

[PATCH] D53587: [clangd] Truncate SymbolID to 16 bytes.

2018-10-23 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. We should watch out for the downstream backward compatibility. Comment at: clangd/index/Index.h:92 // As USRs (Unified Symbol Resolution) could be large, especially for func

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-23 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/IndexAction.h:33 +std::function RefsCallback, +std::function FileDigestsCallback); sammccall wrote: > thinking about what we eventually want here: > - the index action needs to tell the auto-indexe

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-23 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170664. ioeric marked 5 inline comments as done. ioeric added a comment. - address review comments - merged with origin/master - Cleanup Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/index

[PATCH] D53433: [clangd] *Prototype* auto-index stores symbols per-file instead of per-TU.

2018-10-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170440. ioeric marked 2 inline comments as done. ioeric added a comment. - address review comments - minor cleanup Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/index/Background.h clangd

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-10-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/FileIndex.h:49 +/// rename the existing FileSymbols to something else e.g. TUSymbols? +class SymbolsGroupedByFiles { +public: sammccall wrote: > `FileSymbols` isn't actually that opinionated about the data it

[PATCH] D53503: [clangd] Support URISchemes configuration in BackgroundIndex.

2018-10-22 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL344912: [clangd] Support URISchemes configuration in BackgroundIndex. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53503

[PATCH] D53503: [clangd] Support URISchemes configuration in BackgroundIndex.

2018-10-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53503 Files: clangd/index/Background.cpp clangd/index/Background.h I

[PATCH] D53489: [change-namespace] Enhance detection of conflicting namespaces.

2018-10-22 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL344897: [change-namespace] Enhance detection of conflicting namespaces. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53489

[PATCH] D53489: [change-namespace] Enhance detection of conflicting namespaces.

2018-10-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: hokein. Herald added a subscriber: cfe-commits. For example: namespace util { class Base; } namespace new { namespace util { class Internal; } } namespace old { util::Base b1; } When changing `old::` to `new::`, `util::`

[PATCH] D53369: [CodeComplete] Fix accessibility of protected members when accessing members implicitly.

2018-10-22 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC344889: [CodeComplete] Fix accessibility of protected members when accessing members… (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D53369?vs=169995&id=170377#to

[PATCH] D53433: [clangd] *Prototype* auto-index stores symbols per-file instead of per-TU.

2018-10-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: sammccall, hokein. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. This allows us to deduplicate header symbols across TUs. File digests are collects when collecting symbols/refs. And the index store

[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)

2018-10-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdLSPServer.cpp:182 + // - if there were multiple replies, only the first is sent + class ReplyOnce { +struct State { As discussed offline, we could potentially make this non-copyable to make harder to

[PATCH] D53398: [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.

2018-10-18 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. Looks good! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.

2018-10-18 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL344736: [clangd] Names that are not spelled in source code are reserved. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D5337

[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.

2018-10-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170068. ioeric marked an inline comment as done. ioeric added a comment. - add isImplementationDetail helper Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53374 Files: clangd/AST.cpp clangd/AST.h clangd/Quality.cpp clangd/Quality.h

[PATCH] D53387: [clangd] Lay JSONRPCDispatcher to rest.

2018-10-18 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:148 + if (Trace) +(*Trace)["Reply"] = *Result; + Server.reply(ID, json::Value(std::move(*Result)));

[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.

2018-10-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/Quality.cpp:182 if (auto *ID = SemaCCResult.Declaration->getIdentifier()) - ReservedName = ReservedName || isReserved(ID->getName()); + ReservedName = ReservedName || isReserved(ID->getName()) || +

[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.

2018-10-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 170046. ioeric marked 3 inline comments as done. ioeric added a comment. - address comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53374 Files: clangd/AST.cpp clangd/AST.h clangd/Quality.cpp clangd/Quality.h clangd/index/In

[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.

2018-10-17 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. These are often not expected to be used directly e.g. TEST_F(Fixture, X) { ^ // "Fixture_X_Test" expanded in the macro should be do

[PATCH] D53369: [CodeComplete] Fix accessibility of protected members when accessing members implicitly.

2018-10-17 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman. Repository: rC Clang https://reviews.llvm.org/D53369 Files: lib/Sema/SemaCodeComplete.cpp test/Index/complete-access-checks.cpp Index: test/Index/complete-access-checks.

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-17 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL344688: [clangd] Support scope proximity in code completion. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53131 Files:

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-17 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 169983. ioeric added a comment. - rebase Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53131 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp clangd/FileDistance.cpp clangd/FileDistance.h clangd/Quality.cpp clangd/Qu

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-17 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 169982. ioeric marked an inline comment as done. ioeric added a comment. - address comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53131 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp clangd/FileDistance.cpp cla

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-17 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 169962. ioeric marked 18 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53131 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp clangd/FileDistanc

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-17 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Thanks for the suggestions! After taking a closer look at boosting factors for other signals, I think I was being too conservative about boosting preferred scopes and penalizing non-preferred ones. I have tuned the parameters to make these more aggressive now according t

[PATCH] D53286: [clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction.

2018-10-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clangd/JSONRPCDispatcher.h:70 /// A handler responds to requests for a particular method name. + /// It returns true if the server should now shut down.

[PATCH] D53286: [clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction.

2018-10-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Nice! The code looks much clearer. Just some nits Comment at: clangd/ClangdLSPServer.cpp:156 +void ClangdLSPServer::onExit(ExitParams &Params) { + // XXX handler should return true. +} ? Comment at: clangd/ClangdLSPSe

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 169814. ioeric marked 3 inline comments as done. ioeric added a comment. - refactor to use FileDistance and address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53131 Files: clangd/AST.cpp clangd/AST.h clangd/CodeCom

[PATCH] D53317: [clangd] Allow disble down traversals from root.

2018-10-16 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE344604: [clangd] Allow disble down traversals from root. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D53317?vs=169804&id=169805#toc Repository: rCTE Clang

[PATCH] D53317: [clangd] Allow disble down traversals from root.

2018-10-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 169804. ioeric marked an inline comment as done. ioeric added a comment. - Simplify according to review suggestion. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53317 Files: clangd/FileDistance.cpp clangd/FileDistance.h unittests/cla

[PATCH] D53312: [clangd] Support limiting down traversals from sources in FileDistance.

2018-10-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric abandoned this revision. ioeric added a comment. As discussed offline, the semantics is getting a bit complicated. As what we really want for scope proximity is just disallow down traversals from root, we can go with special-casing the root path for now: https://reviews.llvm.org/D53317

[PATCH] D53317: [clangd] Allow disble down traversals from root.

2018-10-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. This is useful for symbo scope proximity, where down traversals from the global scope if not desired. Repository: rCTE Clang Tools Extr

[PATCH] D53312: [clangd] Support limiting down traversals from sources in FileDistance.

2018-10-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. This is useful for symbo scope proximity, where down traversals from the global scope if not desired. The limitation in the current impleme

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

2018-10-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. still lgtm Comment at: clangd/Compiler.cpp:83 +std::string getStandardResourceDir() { + static int Dummy; // Just an address in this process. Maybe also revert this change? Repository: rCTE Clang Too

[PATCH] D53284: [CodeComplete] Make sure keyword 'template' is added even when code pattern is disabled.

2018-10-15 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC344509: [CodeComplete] Make sure keyword 'template' is added even when code pattern is… (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D53284?vs=169681&id=169685

[PATCH] D53284: [CodeComplete] Make sure keyword 'template' is added even when code pattern is disabled.

2018-10-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: sammccall, hokein. Herald added subscribers: cfe-commits, arphaman. Repository: rC Clang https://reviews.llvm.org/D53284 Files: lib/Sema/SemaCodeComplete.cpp test/Index/complete-template-keywords.cpp Index: test/Index/complete-templat

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

2018-10-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: unittests/clangd/BackgroundIndexTests.cpp:14 + +TEST(BackgroundIndexTest, IndexesOneFile) { + MockFSProvider FS; sammccall wrote: > ioeric wrote: > > sammccall wrote: > > > ioeric wrote: > > > > Also add a test for `enqu

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

2018-10-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:89 llvm::cl::init(false), llvm::cl::cat(ClangDocCategory)); +static llvm::cl::opt ClangDocExecutorName( This flag can also be removed. Users can still use `--execut

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

2018-10-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: unittests/clangd/BackgroundIndexTests.cpp:14 + +TEST(BackgroundIndexTest, IndexesOneFile) { + MockFSProvider FS; sammccall wrote: > ioeric wrote: > > Also add a test for `enqueueAll` with multiple TUs ? > Is it important

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

2018-10-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/Background.cpp:51 + std::unique_lock Lock(QueueMu); + assert(!HasActiveTask); + QueueCV.wait(Lock, [&] { return ShouldStop || !Que

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

2018-10-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Looks good in general. Comment at: clangd/ClangdLSPServer.h:117 +llvm::Optional CompileCommandsDir, +std::function); please document what the callback is for and how often it's called. Comment at: clang

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

2018-10-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric requested changes to this revision. ioeric added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:203 +llvm::Expected> +createClangDocExecutor(int &argc, const char **argv, +

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-11 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/CodeComplete.cpp:558 +if (const auto *NS = dyn_cast(Ctx)) + return NS->getQualifiedNameAsString() + "::"; + return llvm::None; sammccall wrote: > does this do the right thing if it's anonymous or inline,

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-11 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 169231. ioeric marked 2 inline comments as done. ioeric added a comment. - address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53131 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h unittests/clan

[PATCH] D52979: [clangd] Add removeFile interface in FileIndex.

2018-10-11 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/FileIndex.h:78 + /// Remove all index data associated with the file \p Path. + void removeFile(PathRef Path); + hokein wrote: > ioeric wrote: > > should we use this somewhere? E.g. when file is closed in Cl

[PATCH] D53034: [clangd] Remove no-op crash handler, we never set a crash context.

2018-10-11 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 > Of course the question arises: *should* we set one Agree with your points. If the cleanup is unsafe, it might end up biting us harder in the future. Repository: rCTE Clang Tools Ex

[PATCH] D53131: [clangd] Support scope proximity in code completion.

2018-10-11 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. This should make all-scope completion more usable. Scope proximity for indexes will be added in followup patch. Measurements showed a slig

[PATCH] D52991: [clangd] Avoid cache main file status in preamble.

2018-10-09 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL344024: [clangd] Avoid cache main file status in preamble. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52991 Files: cl

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