[PATCH] D40486: [clangd] Implemented logging using Context

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 11 08:50:49 PST 2017

ilya-biryukov added inline comments.

Comment at: clangd/ClangdServer.cpp:36
-  ~FulfillPromiseGuard() { Promise.set_value(); }
+  ~FulfillContextPromiseGuard() { Promise.set_value(std::move(Ctx)); }
sammccall wrote:
> Yikes, I can see how we got here, but we really don't get to move out of something we received an lvalue-reference to...
> I think a safer idiom here is turning a whole lambda into a destructor:
>    auto Fulfil = MakeDestructor([&]{
>       DonePromise.set_value(std::move(Ctx));
>    });
> I'm not sure if LLVM has a shared utility for this, I've seen it in other codebases. Either way we could just define it here.
> (Or we could copy the context to avoid the awkwardness)
Thanks for suggestion. I totally agree, callbacks on scope exit are better.

I couldn't find an utility like that in LLVM, but I think it's a useful one. I'll add this locally to this file, but maybe we should make this helper public and put it into clangd?
What are you thoughts on that?

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list