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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
