[PATCH] D42517: [clangd] Pass Context implicitly using TLS.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 31 05:12:46 PST 2018


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(Replacement, currentContext());
----------------
Maybe call it `swapCurrent`. It's longer, but won't allow to mistake it for a member `swap` function. 
Since it's almost never used, having a longer name is not a big deal.


================
Comment at: clangd/Context.h:196
+  ~WithContext() {
+    if (Restore)
+      Context::swap(std::move(*Restore));
----------------
I think it can't be `null` after move ctor is deleted.
Maybe always store a `Context` instead of `unique_ptr` and call `swap` unconditionally?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42517





More information about the cfe-commits mailing list