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

2020-11-25 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd95db1693cbf: [clangd] Extract common file-caching logic from ConfigProvider. (authored by sammccall). Changed prior to commit: https://reviews.ll

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

2020-11-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clang-tools-extra/clangd/support/FileCache.cpp:59 + // If the modified-time and size match, assume the content does too. + if (Size == Stat->getSize() && + ModifiedTime == Stat->getLas

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

2020-11-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/support/FileCache.cpp:59 + // If the modified-time and size match, assume the content does too. + if (Size == Stat->getSize() && + ModifiedTime == Stat->getLastModificationTime()) Is trac

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

2020-11-25 Thread 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 d

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

2020-11-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/support/FileCache.h:58 + void read(const ThreadsafeFS &TFS, +std::chrono::steady_clock::time_point FreshTime, +llvm::function_ref)> Parse, kbobyrev wrote: > So the clie

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

2020-11-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 307551. sammccall marked 2 inline comments as done. sammccall added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88172/new/ https://reviews.llvm.org/D88172 Files: clang-

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

2020-11-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 3 inline comments as done. sammccall added a comment. (oops, missed a merge conflict. New snapshot shortly) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88172/new/ https://reviews.llvm.org/D88172 _

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

2020-11-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 5 inline comments as done. sammccall added inline comments. 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 re

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

2020-11-25 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/unittests/support/FileCacheTests.cpp:15 +#include +#include + njames93 wrote: > kbobyrev wrote: > > Please sort the includes here (clangd should have a Code Action here). > The includes are so

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

2020-11-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/unittests/support/FileCacheTests.cpp:15 +#include +#include + kbobyrev wrote: > Please sort the includes here (clangd should have a Code Action here). The includes are sorted, this is a bug in

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

2020-11-25 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment. Sorry for such a tremendous delay. Mostly looks good functionally, I just have a few questions about semantics and several stylistic nits. Also, thanks for the amount of documentation - it makes it really nice to navigate around and review. Comment

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

2020-11-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D88172#2414249 , @sammccall wrote: > @kbobyrev ping... I think we do actually want to land this, for use with > `.clang-tidy` files after D91029 Yes, I was looking at copying the config cache

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

2020-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. @kbobyrev ping... I think we do actually want to land this, for use with `.clang-tidy` files after D91029 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88172/new/ https://reviews.llvm.or

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

2020-09-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. To give a bit of context here: the end-goal here is hot-reload of compile_commands.json. (This covers our *two* top bugs: #192 and #83). I'm very happy with the hot-reloading of config files (.clangd etc) and I think it's a good fit to generalize. In particular, confi

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

2020-09-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added a reviewer: kbobyrev. Herald added subscribers: cfe-commits, usaxena95, kadircet, jfb, arphaman, mgorny. Herald added a project: clang. sammccall requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. The plan is to us