sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
One down!
I'd like to know what you think about a generic "block the call and capture the
result" mechanism rather than method-specific wrappers.
But if you're not convinced or just want to land this, I don't want to block
the review.
================
Comment at: unittests/clangd/SyncAPI.h:1
+//===--- SyncAPI.h - Sync version of ClangdServer's API ----------*-
C++-*-===//
+//
----------------
Being able to call synchronously is really nice for tests.
It's a bit unfortunate that to do this for each function we want to call
synchronously (giving them a name, writing some boilerplate that repeats the
args a few times).
It would be nice if we had a primitive we could compose.
Here's an idea that might work:
```Tagged<CompletionList> CompletionResults;
Server.codeComplete(File, Pos, Opts, capture(CompletionResults),
OverriddenContents);```
`capture()` returns a callback that writes into CompletionResults. It also
magically causes the call to block!
(brief pause for you to consider the API before I suggest a shameful
implementation)
Actually capture() returns a proxy object that converts to UniqueFunction. The
proxy object itself has a destructor that blocks on the UF being invoked. The
proxy will be destroyed at the end of the full-expression, i.e. the line. e.g.
```template <typename T> struct CaptureProxy {
T &Target;
std::promise<T> Promise;
std::future<T> Future;
CaptureProxy(T &Target) : Target(Target) {}
operator UniqueFunction<void(T)>() {
return BindWithForward([](T Value, std::promise<T> Promise) {
Promise.set_value(std::move(Value));
}, std::move(Promise));
}
~CaptureProxy() { Target = Future.get(); }
};
template <typename T> CaptureProxy<T> capture(T &Target) {
return CaptureProxy<T>(Target);
}```
This is a little bit magic, but it's just for tests :-)
(One caveat - this needs the return value to be default-constructible. We could
easily use Optional instead, but these are default-constructible in practice)
================
Comment at: unittests/clangd/SyncAPI.h:8
+//
+//===---------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
----------------
ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > ioeric wrote:
> > > > > Any reason not to put sync APIs in ClangdServer by their async
> > > > > versions, so that they are easier to find? I might be missing
> > > > > context. But if there is a good reason, please explain in the file
> > > > > doc.
> > > > That's a good question. I don't think we want anyone using the sync
> > > > versions of the API. The tests are somewhat special, though, as they'll
> > > > get really complicated if we move everything to callbacks and we don't
> > > > want that.
> > > >
> > > > This probably deserves a comment.
> > > If these are expected to be used by tests only, I'd suggest moving the
> > > file to the test directory.
> > I think it's in the unittest directory already, so we're covered here :-)
> Sorry! My bad!
Yep, can you add a file comment describing why these exist?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43068
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
