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

lg



================
Comment at: clangd/ClangdUnit.cpp:664
+  std::lock_guard<std::mutex> Lock(Mutex);
+  return ASTMemUsage + PreambleMemUsage;
+}
----------------
ilya-biryukov wrote:
> ioeric wrote:
> > Would it be possible to calculate the usages lazily on request, so that you 
> > don't need to maintain two states?
> It's really complicated since we store `std::future`s. This is one of the 
> caveats that I'm trying to fix with threading revamp, implementation will 
> change accordingly. For now, having two states is much easier.
Thanks for the explanation! A `FIXME` would be appropriate here then? In 
general, I wouldn't expect this API to be called very often.


================
Comment at: unittests/clangd/ClangdTests.cpp:428
 
+TEST_F(ClangdVFSTest, MemoryUsage) {
+  MockFSProvider FS;
----------------
ilya-biryukov wrote:
> ioeric wrote:
> > Can we add a test for non-empty files and check that the usage is 
> > reasonable (e.g. `>0`)?
> We do exactly that. 
My bad! 
```
struct Something {
  int method();
};
```
looks like real code on phabricator!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42480



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to