[PATCH] D59605: [clangd] Introduce background-indexer

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 20 10:53:28 PDT 2019


ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/background-indexer/BackgroundIndexer.cpp:13
+
+#include "/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Context.h"
+#include "/usr/local/google/home/kadircet/repos/llvm/clang-tools-extra/clangd/Logger.h"
----------------
Eugene.Zelenko wrote:
> Please use relative path. Same below.
Heh, where do these come from?
Does our include insertion prefer to add global paths in some cases? Dynamic index?


================
Comment at: clang-tools-extra/clangd/index/Background.cpp:622
+    IndexedTUs += UpToDateTUs;
+    log("[{0}/{1}] Loaded shards from storage", IndexedTUs, EnqueuedTUs);
+  }
----------------
Everything else is `vlog` (or even `dlog`), what's the rationale of using `log` here?


================
Comment at: clang-tools-extra/clangd/index/Background.h:149
+  // For logging
+  size_t EnqueuedTUs = 0;
+  size_t IndexedTUs = 0;
----------------
Maybe use `std::atomic` instead?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59605/new/

https://reviews.llvm.org/D59605





More information about the cfe-commits mailing list