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 replacing the current context?
Most of the clients shouldn't mutate the contexts anyway, and finding the few
that do seems easier if there's a separate method for that.
================
Comment at: clangd/Context.h:201
+ WithContext &operator=(const WithContext &) = delete;
+ WithContext(WithContext &&) = default; // Allow return-by-value.
+ WithContext &operator=(WithContext &&) = delete;
----------------
Where do we use it? Can't we make move ctor deleted too?
================
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.
----------------
`reply`'s signature is scary now.
Should we change it to accept something request-specific that can be found in
the `Context` by the clients? WDYT?
PS we could change it after the patch lands, just wanted to get your opinion on
this
================
Comment at: clangd/Threading.cpp:37
// addToFront).
- Request = std::move(RequestQueue.front());
+ Request = std::move(RequestQueue.front().first);
+ Ctx = std::move(RequestQueue.front().second);
----------------
Maybe replace with `std::tie(Request, Ctx) = std::move(RequestQueue.front())`?
================
Comment at: clangd/Trace.cpp:131
+ return Context::current().clone();
+ WithContextValue WithArgs{std::unique_ptr<json::obj>(Args)};
+ return T->beginSpan(Name, Args);
----------------
Maybe use parentheses instead of braces for initialization?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42517
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
