sammccall marked 2 inline comments as done.
sammccall added inline comments.
Comment at: clangd/Context.h:196
+ ~WithContext() {
+if (Restore)
+ Context::swap(std::move(*Restore));
ilya-biryukov wrote:
> I think it can't be `null` after move ctor is del
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM modulo two NITs.
Really excited to see this land.
Comment at: clangd/Context.cpp:32
+
+Context Context::swap(Context Replacement) {
+ std::swap(Replacemen
sammccall added a comment.
In https://reviews.llvm.org/D42517#993131, @ilya-biryukov wrote:
> I don't see a patch corresponding to the fixed comments. `arc diff` didn't go
> through?
Indeed, sorry. Should be up to date now.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D4251
ilya-biryukov added inline comments.
Comment at: clangd/JSONRPCDispatcher.h:60
+/// Current context must derive from JSONRPCDispatcher::Handler.
+void reply(json::Expr &&Result);
+/// Sends an error response to the client, and logs it.
sammccall wrote:
> ilya-bir
ilya-biryukov added a comment.
I don't see a patch corresponding to the fixed comments. `arc diff` didn't go
through?
Comment at: clangd/Trace.cpp:131
+return Context::current().clone();
+ WithContextValue WithArgs{std::unique_ptr(Args)};
+ return T->beginSpan(Name, Args
sammccall marked 2 inline comments as done.
sammccall added inline comments.
Comment at: clangd/JSONRPCDispatcher.h:60
+/// Current context must derive from JSONRPCDispatcher::Handler.
+void reply(json::Expr &&Result);
+/// Sends an error response to the client, and logs it.
sammccall marked 3 inline comments as done.
sammccall added a comment.
Thanks!
Comment at: clangd/Context.h:77
+ /// See WithContext and WithContextValue for how to do this safely.
+ static Context ¤t();
ilya-biryukov wrote:
> Maybe we could return `const C
ilya-biryukov added inline comments.
Comment at: clangd/Context.h:77
+ /// See WithContext and WithContextValue for how to do this safely.
+ static Context ¤t();
Maybe we could return `const Context&` here and have a separate `setCurrent()`
method for replac
