This revision was automatically updated to reflect the committed changes.
Closed by commit rL312917: [PCH] Allow VFS to be used for tests that generate
PCH files (authored by cameron314).
Changed prior to commit:
https://reviews.llvm.org/D37474?vs=114604=114612#toc
Repository:
rL LLVM
cameron314 added inline comments.
Comment at: include/clang/Frontend/PrecompiledPreamble.h:100
/// PreambleBounds used to build the preamble
PreambleBounds getBounds() const;
ilya-biryukov wrote:
> Not introduced by this change, but could you also add a
cameron314 updated this revision to Diff 114604.
cameron314 added a comment.
Final diff, will commit soon. Thanks!
https://reviews.llvm.org/D37474
Files:
include/clang/Frontend/PrecompiledPreamble.h
lib/Frontend/ASTUnit.cpp
unittests/Frontend/CMakeLists.txt
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
Looks good, just a few minor style comments.
Comment at: include/clang/Frontend/PrecompiledPreamble.h:100
/// PreambleBounds used to build the preamble
cameron314 updated this revision to Diff 114414.
cameron314 added a comment.
The latest patch. I think this one should do the trick :-)
https://reviews.llvm.org/D37474
Files:
include/clang/Frontend/PrecompiledPreamble.h
lib/Frontend/ASTUnit.cpp
unittests/Frontend/CMakeLists.txt
cameron314 added inline comments.
Comment at: lib/Frontend/ASTUnit.cpp:1014
+/// with another virtual file system.
+class PCHOverlayFileSystem : public vfs::FileSystem
+{
ilya-biryukov wrote:
> Maybe create a combination of `InMemoryFileSystem` and
ilya-biryukov added a comment.
In https://reviews.llvm.org/D37474#863335, @cameron314 wrote:
> Looking at the way remapped buffers are handled, I just remembered that they
> must exist on the file system (at the very least, in a directory that exists)
> or the remapping is not taken into
cameron314 added a comment.
Looking at the way remapped buffers are handled, I just remembered that they
must exist on the file system (at the very least, in a directory that exists)
or the remapping is not taken into account. So that pretty much rules out the
other approach, I think.
ilya-biryukov added a comment.
In https://reviews.llvm.org/D37474#862187, @cameron314 wrote:
> I'll change the overlay to only allow access to the PCH.
>
> I agree that it would be nice to only read the PCH using real filesystem APIs
> in order to be symmetrical, but this seems non-trivial. It
cameron314 updated this revision to Diff 114016.
cameron314 added a comment.
Here's an updated patch that allows only the PCH to be accessed from the real
FS when a VSF is present. Tests still pass.
https://reviews.llvm.org/D37474
Files:
lib/Frontend/ASTUnit.cpp
cameron314 added a comment.
I'll change the overlay to only allow access to the PCH.
I agree that it would be nice to only read the PCH using real filesystem APIs
in order to be symmetrical, but this seems non-trivial. It also feels like a
step in the wrong direction -- ideally the VFS would
ilya-biryukov added a comment.
In https://reviews.llvm.org/D37474#861206, @cameron314 wrote:
> I suppose we could do the overlay manually in all the tests that need it; I
> guess it depends on what the goal of the VFS support is. To my mind, it
> doesn't make sense that a file is placed in the
cameron314 added a comment.
I suppose we could do the overlay manually in all the tests that need it; I
guess it depends on what the goal of the VFS support is. To my mind, it doesn't
make sense that a file is placed in the RealFS in the first place if a VFS is
used, but this is quite
ilya-biryukov added a comment.
Could we fix this in tests instead by providing an overlay over `RealFS` there
instead of doing that in `ASTUnit`?
If I'm passing `vfs::FileSystem`, I specifically **do not want** any file
accesses to silently go through `RealFS`, ignoring the `vfs::FileSystem`
cameron314 created this revision.
Herald added a subscriber: mgorny.
When using a virtual file-system (VFS) and a preamble file (PCH) is generated,
it is generated on-disk in the real file-system instead of in the VFS (which I
guess makes sense, since the VFS is read-only). However, when
15 matches
Mail list logo
