ioeric added inline comments.
================
Comment at: clangd/ClangdServer.cpp:159
+ }
+ if (SpecFuzzyReq) {
+ if (auto Filter = speculateCompletionFilter(Content, Pos)) {
----------------
ilya-biryukov wrote:
> NIT: maybe invert condition to reduce nesting?
It would become something like:
```
if (!SpecFuzzyReq)
return SpecFuzzyReq;
.... // some work
return SpecFuzzyReq;
```
Having to return the same thing twice seems a bit worse IMO.
================
Comment at: clangd/CodeComplete.cpp:1388
+ if (Len > 0)
+ St++; // Shift to the first valid character.
+ return Content.substr(St, Len);
----------------
ilya-biryukov wrote:
> This looks incorrect in case of the identifiers starting at offset 0. A
> corner case, but still.
`Offset == 0` is handled above.
================
Comment at: clangd/tool/ClangdMain.cpp:179
+ should be effective for a number of code completions.)"),
+ llvm::cl::init(true));
+
----------------
ilya-biryukov wrote:
> Maybe remove this option?
> Doing a speculative request seems like the right thing to do in all cases. It
> never hurts completion latency. Any reasons to avoid it?
Sure, was trying to be conservative :)
Removed the command-line option but still keeping the code completion option.
As async call has some overhead, I think we should only enable it when static
index is provided.
================
Comment at: unittests/clangd/CodeCompleteTests.cpp:926
- const std::vector<FuzzyFindRequest> allRequests() const { return Requests; }
+ const std::vector<FuzzyFindRequest> consumeRequests() const {
+ auto Reqs = std::move(Requests);
----------------
ilya-biryukov wrote:
> Why do we want to change this? To avoid tests getting requests from the
> previous tests?
Yes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50962
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
