[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/TestTU.h:71
+  // required for use of modules.
+  // FIXME: Intercept the write of implicitly build module to disk and update
+  // TestFS with that content to avoid using real file system in tests.
----------------
sammccall wrote:
> nit: I'm not sure the (V)FS should be involved at all in the final test setup.
> The way I see it, the module storage is basically a cache from (module name + validity-stuff) ==> bytes, and *reads* could be abstracted just like writes.
> For tests this could just be an in-memory map of course, but I think even in production we'd benefit a bit from not having to express this map through the filesystem. (E.g the way it's versioned doesn't have much to do with the way versioned filesystems are)
> 
> Only actionable thing for now is to maybe tweak this comment slightly, but wanted to mention it.
Updated comment, is this better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80525