<div dir="ltr">Very cool. A few high level questions:<div>- shouldn't the virtual file system go into llvm, not clang?<br></div><div>- seems to me like there is only one AbstractFileSystem that should be owned pretty top-level, so why use an IntrusiveRefCntPtr?</div>
<div>- 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...</div>
<div><br></div><div>And a meta-thing:</div><div>- 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 :)</div>
<div><br></div><div>Otherwise looks like a nice first step...</div><div><br></div><div>Cheers,</div><div>/Manuel</div><div><br></div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Mon, Feb 10, 2014 at 11:32 PM, Ben Langmuir <span dir="ltr"><<a href="mailto:blangmuir@apple.com" target="_blank">blangmuir@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Doug et al.<br>
<br>
This is the first patch in the implementation of the virtual file system discussed here: <a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-February/035188.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-February/035188.html</a>.  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.<br>

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

<br>
This patch is notably missing: the interfaces for recursive directory traversal, file system modification (create dir, rename, etc.), and any notion of thread safety.<br>
<span class="HOEnZb"><font color="#888888"><br>
Ben<br>
<br>
</font></span><br><br>
<br>
<br></blockquote></div><br></div>