[PATCH] Some infrastructure work for virtual file system

Manuel Klimek klimek at google.com
Tue Feb 11 02:16:24 PST 2014


Very cool. A few high level questions:
- shouldn't the virtual file system go into llvm, not clang?
- seems to me like there is only one AbstractFileSystem that should be
owned pretty top-level, so why use an IntrusiveRefCntPtr?
- why not make all the function virtual? Doesn't seem to have real
downsides, and would immediately enable fully virtualized file system
implementations (seems fine to have default implementations for all the
functions); if the plan is to have the functions state now to provide an
easier migration path, I think it'd make sense to add a comment describing
the end-state...

And a meta-thing:
- it would help me a lot if I was able to comment via phabricator - is
there a specific reason you're not using it? (I'd be happy to help fix that
reason if it makes reviews simpler - if the reason is not fixable, I'll
just keep my mouth shut and download the patches from now on :)

Otherwise looks like a nice first step...

Cheers,
/Manuel






On Mon, Feb 10, 2014 at 11:32 PM, Ben Langmuir <blangmuir at apple.com> wrote:

> Hi Doug et al.
>
> This is the first patch in the implementation of the virtual file system
> discussed here:
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-February/035188.html.  It
> is intended to include enough of the AbstractFileSystem interface to
> replace the use of llvm::sys::fs in FileManager. Incidentally, I’ve
> included a number of the convenience methods that are trivially implemented
> on top of the status object.  When created by the CompilerInstance,
> FileManager will use a virtual file system created by the CI, but to avoid
> modifying every other user of the FileManager, in the absence of an
> explicit AbstractFileSystem to use, it will default to the real file system
> during construction.
>
> Additionally, I’ve included a stub implementation of the
> OverlayFileSystem, which the CompilerInstance uses.  It is able to overlay
> files from a second file system, but will not merge directories correctly
> yet.  There are no users that actually push a second file system yet (other
> than the included unit test).
>
> This patch is notably missing: the interfaces for recursive directory
> traversal, file system modification (create dir, rename, etc.), and any
> notion of thread safety.
>
> Ben
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140211/317724e3/attachment.html>


More information about the cfe-commits mailing list