[PATCH] Some infrastructure work for virtual file system

Ben Langmuir blangmuir at apple.com
Tue Feb 11 08:20:44 PST 2014


On Feb 11, 2014, at 2:16 AM, Manuel Klimek <klimek at google.com> wrote:

> Very cool. A few high level questions:
> - shouldn't the virtual file system go into llvm, not clang?

I’m fine with putting this in llvm/Support instead of clang/Basic, but we’ll need to check with llvm-dev and the libsupport code owner.  If you feel strongly about this I’ll send something to Chandler and llvm-dev.

> - seems to me like there is only one AbstractFileSystem that should be owned pretty top-level, so why use an IntrusiveRefCntPtr?

This is based on an off-list discussion I had with Doug Gregor.  To ensure modules get built with a consistent environment we want to share the file system among compiler instances.  Also, the AbstractFileSystem may composed of several sub file-systems that can refer to each other.  IntrusiveRefCntPtr seemed like a safer choice based on that.

> - 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...
> 

Do you have any functions in mind that could benefit?  Most of the non-virtual functions are built on top of the virtual status() operation.  I would be concerned that an implementor might think it was a good idea to return something different from one of those query operations than what is in the Status object, which would certainly lead to problems.

> 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 :)
> 

No particular reason - I’ll install arcanist and try to get you a phab review later today :)

Ben

> 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/ac98ebb0/attachment.html>


More information about the cfe-commits mailing list