sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clangd/Function.h:9
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FUNCTION_H
----------------
Maybe add a file comment "provides analogues to std::function that supports
move semantics"?
================
Comment at: clangd/Function.h:36
+ template <class Callable>
+ UniqueFunction(Callable Func)
+ : CallablePtr(llvm::make_unique<
----------------
Do you want this constructor to be explicit?
If not, I think you should be able to simplify the callsites in ClangdServer.h
================
Comment at: clangd/Function.h:77
+/// `operator()` can only be called once, as some of the arguments could be
+/// std::move'ed into the callable on first call.
+template <class Func, class... Args> struct ForwardBinder {
----------------
nit: just 'moved'? std::move is just a cast...
================
Comment at: clangd/Function.h:117
+
+/// Creates an object that stores a callable (\p F) and first arguments to the
+/// callable (\p As) and allows to call \p F with \Args at a later point.
----------------
I find these "first arg" APIs a bit awkward, and can lead to writing confusing
APIs for easier binding. Not sure there's a good alternative, though.
================
Comment at: clangd/Function.h:121
+///
+/// The returned object can only be called once, as \p As are std::forwarded'ed
+/// (therefore can be std::move`d) into \p F for the call.
----------------
nit: can -> must?
https://reviews.llvm.org/D38627
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
