kbobyrev updated this revision to Diff 156997.
kbobyrev marked 19 inline comments as done.
kbobyrev added a comment.
Herald added a subscriber: jfb.
As discussed offline: I simplified the implementation and cleaned up unit
tests. Should look better now.
The next step is to document the implement
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/QueryIterator.cpp:175
+// FIXME(kbobyrev): This compares IDs of two iterators...
+auto CompareIterators = [](std::unique_ptr &Left,
+ std::unique_ptr &Right) -> bool {
---
kbobyrev updated this revision to Diff 157004.
kbobyrev added a comment.
Refactored unit tests in few places.
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Iterator.cpp
clang-tools-extra/clangd/index/dex/Iterator.h
clan
kbobyrev updated this revision to Diff 157020.
kbobyrev marked 12 inline comments as done.
kbobyrev added a comment.
Address all comments, refine tests.
https://reviews.llvm.org/D49591
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Token.cpp
clang-tools-
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:38
+ llvm::makeMutableArrayRef(Roles.data(), Identifier.size()));
+ std::transform(begin(Identifier), end(Identifier), begin(Identifier),
+ ::tolower);
kbobyrev updated this revision to Diff 157196.
kbobyrev marked 7 inline comments as done.
kbobyrev added a comment.
Address last round of comments.
https://reviews.llvm.org/D49591
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Token.h
clang-tools-extra/c
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:56
+ for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
+Next[I] = {{NextTail, NextHead, NextNextHead}};
+NextTail = Roles[I] == Tail ? I : 0;
sammccall w
kbobyrev updated this revision to Diff 157203.
kbobyrev marked 14 inline comments as done.
kbobyrev added a comment.
Address the last round of comments.
Incoming: documentation overhaul.
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:181
+const auto HighestID = peek();
+DocID ChildrenToAdvance = 0;
+while ((ChildrenToAdvance++ < Children.size()) && !reachedEnd() &&
sammccall wrote:
> I can'
kbobyrev updated this revision to Diff 157206.
kbobyrev added a comment.
Slightly refactored the code, improved documentation. This patch is ready for
review.
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Iterator.cpp
cl
kbobyrev updated this revision to Diff 157208.
kbobyrev retitled this revision from "[clangd] Implement query iterators for
Dex symbol index" to "[clangd] Proof-of-concept query iterators for Dex symbol
index".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
Choose better
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337901: [clangd] Introduce Dex symbol index search tokens
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49591?vs=15719
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, jkorous, MaskRay.
`global-symbol-builder` help message mentions `-executor=` option, but
doesn't give any example of what the value cou
kbobyrev updated this revision to Diff 157263.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.
Addressed two comments, added an example of running the tool over a number of
source files without providing `compile_commands.json`; added a note about
collecting files from heade
kbobyrev updated this revision to Diff 157439.
kbobyrev added a comment.
Typo: "Returns false if ..., false otherwise" ->"Returns false if ..., true
otherwise".
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Iterator.cpp
kbobyrev updated this revision to Diff 157438.
kbobyrev marked 30 inline comments as done.
kbobyrev added a comment.
Addressed a round of comments: cleaned up the code, improved documentation and
properly introduced such terms like Posting List and Query Tree. Tests are now
more modular and each
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338015: [clangd] Give an example for symbol-builder usage
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49785?vs=15726
kbobyrev updated this revision to Diff 157447.
kbobyrev added a comment.
Rebase on top of https://reviews.llvm.org/rL337901
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Iterator.cpp
clang-tools-extra/clangd/index/dex/Ite
kbobyrev updated this revision to Diff 157450.
kbobyrev marked 10 inline comments as done.
kbobyrev added a comment.
Address post-lg round of comments.
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Iterator.cpp
clang-tool
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE338017: [clangd] Proof-of-concept query iterators for Dex
symbol index (authored by omtcyfz, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D49546?vs=157450&id=157451#toc
Repositor
omtcyfz created this revision.
omtcyfz added a reviewer: ioeric.
omtcyfz added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov.
- Address a FIXME by warning the user that both -run-synchronously and -j X are
passed.
- Fix a comment to suppress cla
omtcyfz updated this revision to Diff 135626.
omtcyfz added a comment.
Addressed review comment by actually checking whether -j option was actually
passed to clangd.
https://reviews.llvm.org/D43671
Files:
clangd/Context.cpp
clangd/tool/ClangdMain.cpp
Index: clangd/tool/ClangdMain.cpp
===
omtcyfz marked an inline comment as done.
omtcyfz added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:153
+ if (RunSynchronously) {
+if (WorkerThreadsCount != 0) {
+ llvm::errs()
ioeric wrote:
> `-j` is non-zero by default, and we shouldn't sh
omtcyfz updated this revision to Diff 135815.
omtcyfz marked an inline comment as done.
omtcyfz added a comment.
Address Eric's nit: make warning message shorter so that it would fit into one
line in order to omit braces for a single statement for compliance with the
clang-tools-extra codestyle,
This revision was automatically updated to reflect the committed changes.
Closed by commit rL326051: [clangd] Address FIXME and fix comment (authored by
omtcyfz, committed by ).
Herald added subscribers: llvm-commits, klimek.
Changed prior to commit:
https://reviews.llvm.org/D43671?vs=135815&id
omtcyfz created this revision.
This patch introduces cosmetic changes while making ClangD code slightly more
LLVM Coding Standards-compliant by
- Convert names of struct fields in Protocol.h from `camelCase` to `CamelCase`
- Enclose code in .cpp implementation files in appropriate namespaces ins
omtcyfz created this revision.
omtcyfz added reviewers: ioeric, klimek, arphaman.
omtcyfz added projects: clang, clang-tools-extra.
clang-rename was moved to the Clang repository in r306840, but documentation
was left behind, which might be confusing. The Doxygen configuration file in
clang-tool
omtcyfz updated this revision to Diff 149138.
omtcyfz retitled this revision from "[clang-rename] Move clang-rename
documentation to Clang repository" to "[clang-tools-extra] Cleanup
documentation routine".
omtcyfz edited the summary of this revision.
omtcyfz added a comment.
Herald added a subsc
omtcyfz updated this revision to Diff 150083.
omtcyfz edited the summary of this revision.
omtcyfz added a comment.
Leave clang-rename docs in cfe-tools-extra repository, keep the second part of
the patch.
https://reviews.llvm.org/D47537
Files:
clang-tools-extra/docs/Doxyfile
clang-tools-e
omtcyfz added a comment.
@ioeric makes sense to me, I have reverted the first part of the patch while
only leaving the second part of the cleanup, which might still be useful.
https://reviews.llvm.org/D47537
___
cfe-commits mailing list
cfe-commits
kbobyrev added a comment.
In https://reviews.llvm.org/D51949#1257430, @JonasToth wrote:
> @kbobyrev is it ok for you if I stick with the `Optional<>` style in the
> check?
Yes, I think it's fine. Thank you very much for working on the patch, I am
indeed very excited about this check.
Unfortu
kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D52998#1258962, @eandrews wrote:
> Yes. I understand. At the moment, exception handling flags are generated
> based on `BENCHMARK_ENABLE_EXCEPTIONS` in `utils/
kbobyrev requested changes to this revision.
kbobyrev added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D52998#1284146, @eandrews wrote:
> @kbobyrev I apologize if I was unclear in the comments. I was asking if the
> changes proposed in the comments are
kbobyrev added a comment.
In https://reviews.llvm.org/D52998#1284312, @eandrews wrote:
> > Upstreaming it first might be better, especially since the change seems to
> > be trivial. Is this line addition the only change proposed for the
> > benchmark library?
>
> Yes this line is the only chang
kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.
The patch is good to go!
Please add the commit reference to `utils/benchmark/README.LLVM`
(https://github.com/google/benchmark/commit/a9b31c51b1ee7ec7b31438c647123c2cbac5d956)
and the pat
kbobyrev updated this revision to Diff 165962.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.
Rebase on top of master, move logging to symbol index `build()` caller side.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clan
kbobyrev updated this revision to Diff 165964.
kbobyrev added a comment.
@ioeric does this format look better?
https://reviews.llvm.org/D52084
Files:
clang-tools-extra/clangd/index/dex/Iterator.h
clang-tools-extra/clangd/index/dex/PostingList.cpp
clang-tools-extra/unittests/clangd/DexTest
kbobyrev updated this revision to Diff 165973.
kbobyrev marked an inline comment as done.
https://reviews.llvm.org/D52084
Files:
clang-tools-extra/clangd/index/dex/Iterator.h
clang-tools-extra/clangd/index/dex/PostingList.cpp
clang-tools-extra/unittests/clangd/DexTests.cpp
Index: clang-to
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous.
This might be useful for benchmark construction.
https://reviews.llvm.org/D52233
Files:
clang-tools-extra/clangd
kbobyrev added a comment.
In https://reviews.llvm.org/D52084#1238537, @sammccall wrote:
> This change seems fine but...
>
> This representation with the raw DocIDs and position dumped seems mostly
> useful for debugging buggy iterator implementations, but not really useful
> for understanding q
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:69
+// This is not a *real* benchmark: it shows size of built MemIndex (in bytes).
+// Same for the next "benchmark".
ioeric wrote:
> The hack might not be obviou
kbobyrev updated this revision to Diff 166240.
kbobyrev marked 4 inline comments as done.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/index/SymbolYAML.cpp
clang-tools-extra/clangd/index/dex/Dex.cpp
Index: clang-tool
kbobyrev updated this revision to Diff 166241.
kbobyrev marked an inline comment as done.
kbobyrev retitled this revision from "[dexp] Allow users to dump JSON
representations of fuzzy find requests" to "[dexp] Dump JSON representations of
fuzzy find requests".
kbobyrev edited the summary of this
kbobyrev updated this revision to Diff 166266.
kbobyrev retitled this revision from "[clangd] Add a "benchmark" for tracking
memory" to "[clangd] Add building benchmark and memory consumption tracking".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
Add symbol index build
kbobyrev updated this revision to Diff 166269.
kbobyrev added a comment.
Add benchmark for `SymbolSlab` loading from file. This might be useful for
RIFF/YAML symbol loader optimizations.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-too
kbobyrev updated this revision to Diff 166271.
kbobyrev added a comment.
Remove `BuildMem` benchmark, which collects data about `MemIndex` building time
(which is essentially nothing and therefore not really interesting).
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchm
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny.
kbobyrev edited the summary of this revision.
This patch implements Variable-length Byte
kbobyrev updated this revision to Diff 166278.
kbobyrev added a comment.
- Update unit tests with iterator tree string representation to comply with the
new format
- Don't mark constructor `explicit` (previously it only had one parameter)
- Fix `Limits` explanation comment (`ID > Limits[I]` -> `I
kbobyrev added a comment.
Sorry, I didn't get time to review the patch properly, these are few stylistic
comments. Hopefully, I'll be able to give more feedback when I get more time.
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:21
+
+#define PRINT_DEBUG 1
+
kbobyrev added a comment.
@sammccall thank you for the comments, I'll improve it. @ilya-biryukov also
provided valuable feedback and suggested that the code looks complicated now.
We also discussed different compression approach which would require storing
`Head`s and `Payload`s separately so t
kbobyrev updated this revision to Diff 166453.
kbobyrev marked 9 inline comments as done.
kbobyrev added a comment.
I addressed the easiest issues. I'll try to implement separate storage
structure for `Head`s and `Payload`s which would potentially make the
implementation cleaner and easier to un
kbobyrev updated this revision to Diff 166462.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.
Use `llvm::Expected`, cleanup the patch.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/index/Serializa
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:81
+// Same for the next "benchmark".
+// FIXME(kbobyrev): Should this be separated into the BackingMemorySize
+// (underlying SymbolSlab size) and Symbol Index (MemIndex/Dex) overh
kbobyrev created this revision.
kbobyrev added a reviewer: ioeric.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
`Dex` should utilize `FuzzyFindRequest.RestrictForCodeCompletion` flags and
omit symbols not meant for code completion wh
kbobyrev updated this revision to Diff 166474.
kbobyrev marked an inline comment as done.
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
`RestirctForCodeCompletion` and `!RestrictForCodeCompletion` are not mutually
exclusive.
https://reviews.llvm.org/D52357
Files:
cl
kbobyrev updated this revision to Diff 166481.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.
- Be more explicit about the nature of "benchmarks" with memory tracking logic
via `State::SetLabel(...)`.
- Force single iteration for "benchmarks" with memory usage tracking
- Add
kbobyrev updated this revision to Diff 166482.
kbobyrev marked 2 inline comments as done.
https://reviews.llvm.org/D52357
Files:
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/unittests/clangd/DexTests.cpp
Index: clang-tools-extra/unittests/clangd/DexTests.cpp
kbobyrev closed this revision.
kbobyrev added a comment.
I think this one is addressed in the VByte patch, so I'm closing this revision
in order to prevent conflicts between these two.
https://reviews.llvm.org/D52084
___
cfe-commits mailing list
cf
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:29
+ DecompressedChunk = ChunkIndex->decompress();
+ InnerIndex = begin(DecompressedChunk);
+}
sammccall wrote:
> nit: we generally use members (Decompres
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342866: [clangd] Force Dex to respect symbol collector flags
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52357?vs=16
kbobyrev updated this revision to Diff 166831.
kbobyrev marked 17 inline comments as done.
kbobyrev added a comment.
- Simplify code
- Disallow empty `PostingList`s and update tests
https://reviews.llvm.org/D52300
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index
kbobyrev updated this revision to Diff 166843.
kbobyrev marked 13 inline comments as done and an inline comment as not done.
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
Address a round of comments, fallback to `std::vector`.
https://reviews.llvm.org/D52300
Files:
c
kbobyrev updated this revision to Diff 166849.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.
Address post-LG comments, remove fuzzer.
https://reviews.llvm.org/D52300
Files:
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/index/dex/PostingList.cpp
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:192
+/// Reads variable length DocID from the buffer and updates the buffer size. If
+/// the stream is terminated, return None.
+llvm::Optional readVByte(llvm::ArrayRef &Bytes) {
-
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342965: [clangd] Implement VByte PostingList compression
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52300?vs=166849
kbobyrev added inline comments.
Comment at: clangd/index/Serialization.cpp:407
+ } else {
+return makeError("Not a RIFF file and failed to parse as YAML: " +
+ llvm::toString(YAMLContents.takeError()));
nit: probably omit the last `else`
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
For consistency, functional-style code pieces are replaced with their simple
counterparts to improve readabilit
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342974: [clangd] NFC: Simplify code, enforce LLVM Coding
Standards (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52466
kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.
Ah, I see. Thanks for clarifying! I was wondering if we should also rename
`-yaml-symbol-file` flag to something like `-static-symbol-file` (but, as you
said, it's something to do in anoth
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
This patch adds `Token`'s string data to the memory estimation. While it is
only responsible for a couple of ex
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:251
Bytes += InvertedIndex.getMemorySize();
- for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+ for (const auto &TokenToPostingList : InvertedIndex)
+Bytes += Token
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:251
Bytes += InvertedIndex.getMemorySize();
- for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+ for (const auto &TokenToPostingList : InvertedIndex)
+Bytes += Token
kbobyrev updated this revision to Diff 167062.
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
- Prevent `sizeof(std::vector)` to be counted twice in the memory
estimates
- Pull memory usage logging to the caller side: it is currently incorrect
because `Dex::BackingDataSi
kbobyrev updated this revision to Diff 167067.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.
Don't include misc changes elsewhere: focus on adding more benchmarks in this
revision.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmar
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:96
-static void DexQueries(benchmark::State &State) {
+// This is not a *real* benchmark: it shows size of built MemIndex (in bytes).
+// Same for the next "benchmark".
--
kbobyrev created this revision.
kbobyrev added reviewers: sammccall, ioeric.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
According to new implementation, `readIndexFile` can also process YAML symbol
collections. Also, it might make
kbobyrev updated this revision to Diff 167079.
kbobyrev retitled this revision from "[clangd] More precise index memory usage
estimate" to "[clangd] Fix bugs with incorrect memory estimate report".
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
As discussed offline, using
kbobyrev added a comment.
In https://reviews.llvm.org/D52534#1246190, @sammccall wrote:
> The wording change is in https://reviews.llvm.org/D52531.
Oh, I didn't see that one. Thanks!
https://reviews.llvm.org/D52534
___
cfe-commits mailing list
cf
kbobyrev abandoned this revision.
kbobyrev added a comment.
This is no longer needed then.
https://reviews.llvm.org/D52534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kbobyrev resigned from this revision.
kbobyrev added a comment.
For anyone interested in the direction of posting list compression, an
implementation of Variable length Byte compression (VByte) has landed:
https://reviews.llvm.org/D52300.
Repository:
rCTE Clang Tools Extra
https://reviews.l
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous.
Because `PostingList` objects are compressed, it is now impossible to see
elements other than the current one and the do
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:251
Bytes += InvertedIndex.getMemorySize();
- for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+ for (const auto &TokenToPostingList : InvertedIndex)
+Bytes += Token
kbobyrev updated this revision to Diff 167138.
kbobyrev added a comment.
Simplify the documentation format.
https://reviews.llvm.org/D52545
Files:
clang-tools-extra/clangd/index/dex/Iterator.h
Index: clang-tools-extra/clangd/index/dex/Iterator.h
=
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343116: [docs] Update PostingList string representation
format (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52545?vs=
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343117: [clangd] Fix bugs with incorrect memory estimate
report (authored by omtcyfz, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52503?vs=167079&id=167143#toc
Repository:
rC
kbobyrev updated this revision to Diff 167267.
kbobyrev added a comment.
Pass data from I/O to `readIndexFile(StringRef)`.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
=
kbobyrev added a comment.
A lot of good improvements and many test cases, thank you!
The comments are mostly nits.
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+ auto Diag =
+ diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+ <<
kbobyrev added a comment.
Also, regarding error handling and `llvm::Option` vs `llvm::Expected`: I think
the case where the check most likely wouldn't be able to provide useful
diagnostics and perform enough analysis is when there are macro expansions
within inspected statement `SourceRange`. I
kbobyrev added inline comments.
Comment at: clangd/index/dex/Dex.cpp:143
"There must be no :: in query.");
+ trace::Span Tracer("Dex fuzzyFind");
FuzzyMatcher Filter(Req.Query);
sammccall wrote:
> We should attach the query tree to the span here.
>
kbobyrev added inline comments.
Comment at: clangd/index/dex/Dex.cpp:171
}
+ if (Req.AnyScope)
+ScopeIterators.push_back(createBoost(createTrue(Symbols.size()), 0.2));
Probably also check `!ScopeIterators.empty()`: otherwise the latency might
increase fo
kbobyrev added inline comments.
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+ auto Diag =
+ diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+ << static_cast(
aaron.ballman wrote:
> lebedev.ri wrote:
> > kbobyrev w
kbobyrev added inline comments.
Comment at: lib/Basic/VirtualFileSystem.cpp:2097
void YAMLVFSWriter::write(llvm::raw_ostream &OS) {
- llvm::sort(Mappings, [](const YAMLVFSEntry &LHS, const YAMLVFSEntry &RHS) {
+ llvm::sort(Mappings.begin(), Mappings.end(),
+ [](con
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, dexonsmith, mgrang, jkorous,
MaskRay, javed.absar, ilya-biryukov.
This patch improves readability by migrating `std::function(For
kbobyrev added a comment.
Also, I'm not sure whether I should update Clang-Tidy and other tools in the
scope of this patch. It makes sense to me, but I don't know whether the
maintainers of these projects are happy with the change. WDYT?
https://reviews.llvm.org/D52650
_
kbobyrev added a comment.
In https://reviews.llvm.org/D52650#1249556, @sammccall wrote:
> I think it's fine to update others too, this is a trivially-safe change and
> a nice readability improvement.
> No need to put everything in the same patch though.
Great, thanks! I'll submit the patch on
kbobyrev updated this revision to Diff 168589.
kbobyrev added a comment.
Rebase on top of HEAD.
https://reviews.llvm.org/D52650
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343937: [clangd] NFC: Migrate to LLVM STLExtras API where
possible (authored by omtcyfz, committed by ).
Herald added subscribers: llvm-commits, kristina.
Changed prior to commit:
https://reviews.llvm.o
kbobyrev added inline comments.
Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:24
+AST_MATCHER(DeclStmt, onlyDeclaresVariables) {
+ return std::all_of(Node.decl_begin(), Node.decl_end(),
+ [](Decl *D) { return isa(D); });
It w
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336257: [clang-tools-extra] Cleanup documentation routine
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47537?vs=15008
omtcyfz created this revision.
omtcyfz added reviewers: ioeric, sammccall, ilya-biryukov.
omtcyfz added a project: clang-tools-extra.
Herald added subscribers: jkorous, MaskRay.
Having `using qualified::name;` for some symbol is an important signal for
clangd code completion as the user is more l
301 - 400 of 1461 matches
Mail list logo
