[PATCH] D88172: [clangd] Extract common file-caching logic from ConfigProvider.

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.

LGTM with `std::numeric_limits` in the second place :) Thanks!



================
Comment at: clang-tools-extra/clangd/support/FileCache.cpp:21
+// The cached value reflects that the file doesn't exist.
+static constexpr uint64_t FileNotFound = -2;
+
----------------
I think makes sense to do `std::numeric_limits()` here, too for consistency?


================
Comment at: clang-tools-extra/clangd/support/FileCache.cpp:18
+// The cached value does not reflect the current content on disk.
+static constexpr uint64_t CacheDiskMismatch = -1;
+// The cached value reflects that the file doesn't exist.
----------------
sammccall wrote:
> kbobyrev wrote:
> > Uhhh. Doesn't Clang-Tidy think this is a bad idea? IIUC this is not UB but it looks quite scary regardless. Maybe use `std::numeric_limits<uint64_t>::max()` here?
> Done.
> (FWIW I prefer the -1 idiom as you don't have to repeat the type so there's no chance of a mismatch)
Yeah I guess one could use reflection for that but that but that's more code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88172