ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1141
+ ReceivedRequestCV.wait(Lock,
+ [this, Num] { return NumRequestsReceived == Num; });
+ NumRequestsReceived = 0;
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > Could we wait with timeout here (a really high one, e.g. 10 seconds) here
> > and assert we did not hit the timeout?
> > The `wait` helper from `Threading.h` is a nice API to do this.
> >
> > This would ensure the test never actually runs into an infinite loop in
> > practice.
> I don't think it is that important, but sure I suppose a failing test is
> better than a hanging one :D
Totally agree! Thanks!
================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:1161
completions(Code, {}, Opts);
- return Requests.consumeRequests();
+ const auto Reqs = Requests.consumeRequests(Num);
+ EXPECT_EQ(Reqs.size(), Num);
----------------
This code got me thinking whether compiler is allowed to do NRVO if the local
variable is const.
Given that the object is not const, we definitely cannot call a move
constructor to go from `Reqs` to the return value.
OTOH, if NRVO applies here, this shouldn't matter.
No need to do anything, just thinking out loud.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68273/new/
https://reviews.llvm.org/D68273
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
