[PATCH] D88172: [clangd] Extract common file-caching logic from ConfigProvider.
Kirill Bobyrev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 25 02:21:30 PST 2020
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
More information about the cfe-commits
mailing list