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

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

Reply via email to