[PATCH] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.

2018-03-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 138313. ioeric marked 5 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44305 Files: clangd/index/FileIndex.cpp clangd/index/FileIndex.h clangd/index/Index.h

[PATCH] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.

2018-03-14 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL327487: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.or

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353 +const FileEntry *Entry = FileAndReplacements.first; +ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry); +for (const auto &R : FileAndReplacements.second)

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353 +const FileEntry *Entry = FileAndReplacements.first; +ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry); +for (const auto &R : FileAndReplacements.second)

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: hokein. Herald added subscribers: cfe-commits, klimek. Previously, the matcher matches a function call/ref multiple times, one for each decl ancestor. This might cause problems. For example, in the following case, `func()` would be matched onc

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 138550. ioeric added a comment. - small fix. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44517 Files: change-namespace/ChangeNamespace.cpp unittests/change-namespace/ChangeNamespaceTests.cpp Index: unittests/change-namespace/ChangeN

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE327629: [change-namespace] Don't match a function call/ref multiple times. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D44517?vs=138550&id=138556#toc Reposi

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL327629: [change-namespace] Don't match a function call/ref multiple times. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-16 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353 +const FileEntry *Entry = FileAndReplacements.first; +ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry); +for (const auto &R : FileAndReplacements.second)

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

2018-03-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. lgtm (Please add reviewers to your patch if you intend it to be reviewed.) Comment at: clangd/CMakeLists.txt:7 +if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB) + list(APPEND CLANGD_ATOMIC_LIB atomic) +endif() nit: s/atomic/"atomic"/ Repositor

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

2018-03-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D44248#1041531, @sdardis wrote: > Thanks, I wasn't sure who to add as a reviewer. Authors/reviewers of recent patches for the same files are usually good approximations :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D4424

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. This looks great! Just a few nits left. Comment at: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:75 +/// \brief Deduplicate, check for conflicts, and convert all Replacements stored +/// in \c TUs to AtomicChang

[PATCH] D47068: Move #include manipulation code to new lib/Tooling/Inclusions.

2018-05-30 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 149091. ioeric added a comment. - Fix module build. Repository: rC Clang https://reviews.llvm.org/D47068 Files: include/clang/Format/Format.h include/clang/Tooling/Core/HeaderIncludes.h include/clang/Tooling/Core/IncludeStyle.h include/clang/Tooli

[PATCH] D47068: Move #include manipulation code to new lib/Tooling/Inclusions.

2018-05-30 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL333532: Reland "Move #include manipulation code to new lib/Tooling/Inclusions." (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D47068?vs=149091&id=149092#toc Rep

[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

2018-06-01 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 with just a few more nits. Comment at: clang-doc/BitcodeWriter.cpp:484 #undef EMITINFO Nit: `EMITINFO` is a bit confusing with `writeInfo`. Are the

[PATCH] D47537: [clang-tools-extra] Cleanup documentation routine

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clang-tools-extra/docs/index.rst:27 pp-trace - clang-rename clangd It seems that the clang-rename tool is still in the extra repository. I think we should probably "advertise" `clang-rename` as part of clang-re

[PATCH] D44954: [clangd] Add "member" symbols to the index

2018-06-01 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/index/Index.h:158 unsigned References = 0; - + /// Whether or not this is symbol is meant to be used for the global + /// completion. ---

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov, klimek. These decls are sometime used as the canonical declarations (e.g. for go-to-def), which seems to be bad. Repository: rCTE Clang Tools Extra https:

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:293 assert(CompletionAllocator && CompletionTUInfo); + // A declaration created for a friend declaration should not be used as the + // canonical declaration in the index. ilya-biryuk

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 149446. ioeric marked an inline comment as done. ioeric edited the summary of this revision. ioeric added a comment. - Addressed review comment. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47623 Files: clangd/index/SymbolCollector.cpp

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:297 +// If OrigD is an object of a friend declaration, skip it. +if (ASTNode.OrigD->getFriendObjectKind() != +Decl::FriendObjectKind::FOK_None) sammccall wrote: > this seem

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:293 assert(CompletionAllocator && CompletionTUInfo); + // A declaration created for a friend declaration should not be used as the + // canonical declaration in the index. ilya-biryuk

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 149465. ioeric added a comment. - Clarify. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47623 Files: clangd/index/SymbolCollector.cpp unittests/clangd/SymbolCollectorTests.cpp Index: unittests/clangd/SymbolCollectorTests.cpp

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/SymbolCollector.cpp:297 +// If OrigD is an object of a friend declaration, skip it. +if (ASTNode.OrigD->getFriendObjectKind() != +Decl::FriendObjectKind::FOK_None) sammccall wrote: > ioeric wr

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 149526. ioeric added a comment. - Make canonical decls determinstic. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47623 Files: clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h unittests/clangd/SymbolCollectorTests.cpp

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 149528. ioeric added a comment. - Remove debug message. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47623 Files: clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h unittests/clangd/SymbolCollectorTests.cpp Index: unit

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-01 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D47623#1118951, @ilya-biryukov wrote: > In https://reviews.llvm.org/D47623#1118810, @sammccall wrote: > > > - friend decls that are not definitions should be ignored for indexing > > purposes > > > This is not generally true IIUC. A friend decl

[PATCH] D43667: [clang-doc] Implement a YAML generator

2018-06-04 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clang-doc/Generators.h:25 +// +// Derived classes must implement the generate*ForInfo methods, which should +// emit documentation for the specified info in the relevant format. This is nit: This seems a bit redundant as

[PATCH] D47701: [clangd] Hover should return null when not hovering over anything.

2018-06-04 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/D47701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-04 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D47623#1120547, @ilya-biryukov wrote: > In https://reviews.llvm.org/D47623#1119426, @ioeric wrote: > > > Sorry for the late response Ilya. I was trying to test these cases. So, > > with the current change, if a real "canonical" declaration come

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-04 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 149721. ioeric marked an inline comment as done. ioeric added a comment. Herald added a subscriber: mgorny. - Addressed review comment. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47623 Files: clangd/CMakeLists.txt clangd/Headers.h

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-04 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 149723. ioeric added a comment. - Revert unintended changes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47623 Files: clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h unittests/clangd/SymbolCollectorTests.cpp Index:

[PATCH] D47623: [clangd] Avoid indexing decls associated with friend decls.

2018-06-04 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE333885: [clangd] Avoid indexing decls associated with friend decls. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D47623?vs=149723&id=149724#toc Repository:

[PATCH] D43667: [clang-doc] Implement a YAML generator

2018-06-05 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: clang-doc/Generators.h:25 +// +// Derived classes must implement the generate*ForInfo methods, which should +// emit documentation for the specified info in

[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)

2018-06-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Looks like a great improvement on ranking! Comment at: clangd/Quality.cpp:62 + bool InClass; + for (const DeclContext *DC = D.getDeclContext(); DC != nullptr; + DC = DC->getParent()) { nit: maybe stop when`!DC->isFileContext()`?

[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)

2018-06-05 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/Quality.cpp:70 +return SymbolRelevanceSignals::ClassScope; + if (D.getLinkageInternal() < ExternalLinkage) +return SymbolRelevanceSignals::Fi

[PATCH] D47737: [clangd] Remove unused variables

2018-06-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. fyi, you could actually accept your own patch phab (e.g. when you would like post-commit review) :P Repository: rL LLVM https://reviews.llvm.org/D47737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D47762: [clangd] Boost code completion results that are narrowly scoped (local, members)

2018-06-05 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: unittests/clangd/QualityTests.cpp:1 //===-- SourceCodeTests.cpp *- C++ -*-===// // sammccall wrote: > ioeric wrote: > > sammccall wrote: > > > ioeric wrote: > > > > Could you also a

[PATCH] D47787: [clangd] Adjust symbol score based on crude symbol type.

2018-06-05 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/Quality.cpp:33 +static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl &ND) { + class Switch Have you considered `

[PATCH] D47815: [clangd] Boost fuzzy match score by 2x (so a maximum of 2) when the query is the full identifier name.

2018-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. lg Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D47871: [clangd] Code completion: drop explicit injected names/operators, ignore Sema priority

2018-06-07 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/CodeComplete.cpp:542 continue; - // Destructor completion is rarely useful, and works inconsistently. - // (s.^ completes ~string, b

[PATCH] D47931: [clangd] Customizable URI schemes for dynamic index.

2018-06-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov. This allows dynamic index to have consistent URI schemes with the static index which can have customized URI schemes, which would make file proximity scoring ba

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov. Also move unittest: URI scheme to TestFS so that it can be shared by different tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 F

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 150475. ioeric added a comment. Rebase on https://reviews.llvm.org/D47931 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Q

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 150477. ioeric added a comment. - Rebase again... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests.cpp unittests/clangd/TestFS.cpp

[PATCH] D47936: [clangd] Require case-insensitive prefix match for macro completions.

2018-06-08 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. I like this. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D47937: [clangd] Support proximity paths in index fuzzy find.

2018-06-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47937 Files: clangd/CodeComplete.cpp clangd/index/Index.h Index: clangd/index/Index.h ===

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D47935#1126283, @sammccall wrote: > Can you benchmark this? I'm nervous about the URI stuff in the hot path. > Timing CodeCompleteFlow::measureResults before/after with index enabled > seems like a reasonable test. > (But you might want to ma

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-08 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Here are some numbers by completing "clang::^" 40 times (with result limit 1000 instead of 100). Timing in `CodeCompleteFlow::measureResults` Before: Avg: 1811 us Med: 1792 us After: Avg: 2714 us Med: 2689 us As a reference, a full CodeCompleteFlow (with 1000 candi

[PATCH] D47937: [clangd] Support proximity paths in index fuzzy find.

2018-06-11 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/index/Index.h:275 + /// Contextually relevant files (e.g. the file we're code-completing in). + /// Paths should be absolute. + std::vector ProximityPaths; sammccall wrote: > Hmm, the returned results have URIs,

[PATCH] D47937: [clangd] Support proximity paths in index fuzzy find.

2018-06-12 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334485: [clangd] Support proximity paths in index fuzzy find. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47937 Files:

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 150923. ioeric added a comment. - Merge branch 'uri' into proximity - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h unittests/clangd

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 150924. ioeric added a comment. - Cleanup comment a bit. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests.cpp unittests/clangd/Tes

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-12 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. PTAL Comment at: clangd/Quality.cpp:215 void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) { if (SemaCCResult.Availability == CXAvailability_NotAvailable || ioeric wrote: > sammccall wrote: > > proximity p

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151169. ioeric added a comment. - Introduced a one-per-query structure for relevance signals; use multiplication for proximity; simplify tests a bit; separate index and sema proximity scores. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D4

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D47935#1129987, @sammccall wrote: > Sorry for the delay on this change. There's a bunch of complexity in this > problem that I haven't seen how to slice through: > > 1. the signals needed seem like a weird fit for the Symbol*Signals structs >

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-13 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151172. ioeric added a comment. - Rebased. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/CodeComplete.cpp clangd/FindSymbols.cpp clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests.cpp unittests

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: sammccall, ilya-biryukov, hokein. Herald added subscribers: cfe-commits, jkorous, MaskRay. For completion items that would trigger include insertions (i.e. index symbols that are not #included yet), add a visual indicator "+" before the complet

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151321. ioeric marked 2 inline comments as done. ioeric added a comment. - addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/CodeComplete.cpp c

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151322. ioeric added a comment. - Rebase... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests.cpp unittests/clangd/TestFS.cpp uni

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/Quality.h:71 +class SymbolRelevanceContext { + public: sammccall wrote: > This is ambiguously a couple of different (and good!) things: > - an encapsulation of the proximitypaths state and logic > - a grouping

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151357. ioeric added a comment. - s/+/•/ and make the icon an code completion option to avoid hardcodes in multiple places. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48163 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clan

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/CodeComplete.cpp:278 +if (AllowIndexCompletion) + I.label = (InsertingInclude ? "+" : " ") + I.label; I.scoreInfo = Scores; sammccall wrote: > I think we should avoid tokens that occur commonly in C++

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151358. ioeric marked 6 inline comments as done. ioeric added a comment. - addressed review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h unittests/cl

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-14 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/Quality.cpp:185 + } + if (F == Fs.begin() && T == Ts.begin()) // No common directory. +return 0; sammccall wrote: > why is this a special case? > - /x/a/b vs /x/a/c is 1 up + 1 down --> 0.59 > - /a/b vs /a/

[PATCH] D47931: [clangd] Customizable URI schemes for dynamic index.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151467. ioeric marked an inline comment as done. ioeric added a comment. - Pulled unittest URI refactoring from https://reviews.llvm.org/D47935 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47931 Files: clangd/ClangdServer.cpp clangd/Cl

[PATCH] D47931: [clangd] Customizable URI schemes for dynamic index.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334809: [clangd] Customizable URI schemes for dynamic index. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47931 Files:

[PATCH] D47931: [clangd] Customizable URI schemes for dynamic index.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: unittests/clangd/FileIndexTests.cpp:99 +class TestURIScheme : public URIScheme { +public: sammccall wrote: > This can use the shared unittest: scheme from the followup patch, right? > > Add a fixme or lift that into th

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151469. ioeric marked 6 inline comments as done. ioeric added a comment. - addressed review comments and rebase. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 Files: clangd/CodeComplete.cpp clangd/Quality.cpp clangd/Quality.h

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Thanks for the review! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47935: [clangd] Boost completion score according to file proximity.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL334810: [clangd] Boost completion score according to file proximity. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47935 F

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Thanks for the review! Comment at: clangd/CodeComplete.cpp:187 + // Whether or not this candidate is in a completion where index is allowed. + // This can affect how items are built (e.g. indent label to align with visual + // indicator in index resu

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151492. ioeric marked 6 inline comments as done. ioeric added a comment. - Addressed review comments. - Merged with orgin/master Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48163 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151496. ioeric marked 6 inline comments as done. ioeric added a comment. - addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48163 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Headers.cpp clangd

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/CodeComplete.h:63 - // Populated internally by clangd, do not set. + /// Populated internally by clangd, do not set. /// If `Index` is set, it is used to augment the code completion sammccall wrote: > if you

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151497. ioeric added a comment. - fix typo and clang-format Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48163 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Headers.cpp clangd/Headers.h test/clangd/completion-snippe

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151498. ioeric added a comment. - clang-format again... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48163 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Headers.cpp clangd/Headers.h test/clangd/completion-snippets.t

[PATCH] D48163: [clangd] UI for completion items that would trigger include insertion.

2018-06-15 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE334828: [clangd] UI for completion items that would trigger include insertion. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D48163?vs=151498&id=151499#toc Re

[PATCH] D48290: [clangd] Use workspace root path as hint path for resolving URIs in workspace/symbol

2018-06-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: sammccall, malaperle. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov. Some URI schemes require a hint path to be provided, and workspace root path seems to be a good fit. Repository: rCTE Clang Tools Extra https://r

[PATCH] D48290: [clangd] Use workspace root path as hint path for resolving URIs in workspace/symbol

2018-06-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/ClangdServer.cpp:119 + auto FS = FSProvider.getFileSystem(); + auto Status = FS->status(RootPath); + if (!Status) sammccall wrote: > why validate it? This is the current behavior, except `llvm::sys::fs::is_direc

[PATCH] D48290: [clangd] Use workspace root path as hint path for resolving URIs in workspace/symbol

2018-06-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151869. ioeric marked 3 inline comments as done. ioeric added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48290 Files: clangd/ClangdServer.cpp clangd/FindSymbols.cpp clangd/FindSymbols.h unitte

[PATCH] D48290: [clangd] Use workspace root path as hint path for resolving URIs in workspace/symbol

2018-06-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 151877. ioeric added a comment. - Require '/' in front of unittest: body Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48290 Files: clangd/ClangdServer.cpp clangd/FindSymbols.cpp clangd/FindSymbols.h unittests/clangd/FindSymbolsTest

[PATCH] D48290: [clangd] Use workspace root path as hint path for resolving URIs in workspace/symbol

2018-06-19 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL335035: [clangd] Use workspace root path as hint path for resolving URIs in… (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D

[PATCH] D48375: [clangd] Remove FilterText from the index.

2018-06-21 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/D48375 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D48341: [clang-doc] Adding a second reduce pass

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clang-tools-extra/clang-doc/Representation.h:173 bool mergeable(const Info &Other); + llvm::Expected getEnclosingScope(); }; Comment? What is an enclong scope? Comment at: clang-tools-extra/clang-d

[PATCH] D48418: [clangd] Expose 'shouldCollectSymbol' helper from SymbolCollector.

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48418 Files: clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h Index: clangd

[PATCH] D48418: [clangd] Expose 'shouldCollectSymbol' helper from SymbolCollector.

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 152249. ioeric marked an inline comment as done. ioeric added a comment. - added tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48418 Files: clangd/index/SymbolCollector.cpp clangd/index/SymbolCollector.h unittests/clangd/Symbol

[PATCH] D48418: [clangd] Expose 'shouldCollectSymbol' helper from SymbolCollector.

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. In https://reviews.llvm.org/D48418#1138984, @sammccall wrote: > Can you motivate this change a bit, and add tests? Done. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48418 ___ cfe-commits mailing list cf

[PATCH] D48418: [clangd] Expose 'shouldCollectSymbol' helper from SymbolCollector.

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL335218: [clangd] Expose 'shouldCollectSymbol' helper from SymbolCollector. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org

[PATCH] D48425: [clangd] Expose qualified symbol names in CompletionItem (C++ structure only, no json).

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov. The qualified name can be used to match a completion item to its corresponding symbol. This can be useful for tools that measure code completion quality. Qualif

[PATCH] D48425: [clangd] Expose qualified symbol names in CompletionItem (C++ structure only, no json).

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 152265. ioeric added a comment. - Fix build Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48425 Files: clangd/CodeComplete.cpp clangd/Protocol.h unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp ==

[PATCH] D48425: [clangd] Expose qualified symbol names in CompletionItem (C++ structure only, no json).

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: clangd/Protocol.h:743 + /// FIXME: find a more precise way to identify a completion item. + std::string QualifiedSymbolName; }; sammccall wrote: > So this is always set to scope + filterText (except for non-decl comple

[PATCH] D48425: [clangd] Expose qualified symbol names in CompletionItem (C++ structure only, no json).

2018-06-21 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 152331. ioeric marked 2 inline comments as done. ioeric added a comment. - store scope in completion item. Add splitQualifiedName for NamedDecl. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48425 Files: clangd/CodeComplete.cpp clangd/P

[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.

2018-06-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. Looks great! Thanks for doing this! The algorithm looks pretty efficient. It might still make sense to collect some performance numbers for real files with potentially large #include tree (we have plenty of those in our internal codebase :) Comment at

[PATCH] D48425: [clangd] Expose qualified symbol names in CompletionItem (C++ structure only, no json).

2018-06-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 152447. ioeric marked 2 inline comments as done. ioeric added a comment. - Add printQualifiedName in AST.h Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48425 Files: clangd/AST.cpp clangd/AST.h clangd/CodeComplete.cpp clangd/Protoco

[PATCH] D48425: [clangd] Expose qualified symbol names in CompletionItem (C++ structure only, no json).

2018-06-22 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE335334: [clangd] Expose qualified symbol names in CompletionItem (C++ structure only… (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D48425?vs=152447&id=152448#

[PATCH] D48475: [clangd] More precise representation of symbol names/labels in the index.

2018-06-22 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/CodeCompletionStrings.cpp:122 + // Typed-text chunk is the actual name. Text before it is qualifiers. + if (Qualifiers) +*Qualifiers

[PATCH] D48492: [clang-format] Add a default format style that can be used by users of `getStyle`

2018-06-22 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. ioeric added reviewers: sammccall, djasper. Herald added a subscriber: cfe-commits. Tools that reformat code often call `getStyle` to decide the format style to use on a certain source file. In practice, "file" style is widely used. As a result, many tools hardcode "f

[PATCH] D48492: [clang-format] Add a default format style that can be used by users of `getStyle`

2018-06-25 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 152636. ioeric marked an inline comment as done. ioeric added a comment. - Add DefaultFallbackStyle Repository: rC Clang https://reviews.llvm.org/D48492 Files: include/clang/Format/Format.h lib/Format/Format.cpp tools/clang-format/ClangFormat.cpp

[PATCH] D48492: [clang-format] Add a default format style that can be used by users of `getStyle`

2018-06-25 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: tools/clang-format/ClangFormat.cpp:67 static cl::opt FallbackStyle("fallback-style", cl::desc("The name of the predefined style used as a\n" sammccall wrote: > My only question is if we want to do the sam

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