ilya-biryukov added inline comments.
================ Comment at: clangd/ClangdServer.cpp:51 +template <class Ret, class... Args> +std::future<Ret> makeFutureAPIFromCallback( ---------------- sammccall wrote: > I'm not sure we need a template here rather than just writing the code > directly. Removed the template, inlined its usage. ================ Comment at: clangd/ClangdServer.cpp:257 + // A task that will be run asynchronously. + auto Task = + // 'mutable' to reassign Preamble variable. ---------------- sammccall wrote: > Isn't this exactly where you'd want to use ForwardBinder? > (Yeah, it looks like the work scheduler itself has this functionality too) Yes, `ClangdServer` uses `ForwardBinder` internally. We could refactor it to accept `UniqueFunction` instead, but I suggest we do that in a separate commit since there are more call sites that we'll have to change. ================ Comment at: clangd/ClangdServer.h:261 + void codeComplete( + UniqueFunction<void(Tagged<std::vector<CompletionItem>>)> Callback, + PathRef File, Position Pos, ---------------- sammccall wrote: > Hmm, generally having a callback as the last parameter is nicer, but this > doesn't play nicely with optional params... Right, so I opted for making it a first parameter. I don't know if there's a better alternative. Do you think making it a last non-optional parameter would make things better? (Probably it will, at least calls without non-optional params would look nicer). https://reviews.llvm.org/D38629 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
