On Apr 14, 2014, at 8:45 AM, Ben Langmuir <[email protected]> wrote:
> > On Apr 11, 2014, at 5:45 PM, Argyrios Kyrtzidis <[email protected]> wrote: > >> IntrusiveRefCntPtr<LangOptions> LangOpts; >> IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics; >> + IntrusiveRefCntPtr<vfs::FileSystem> VFS; >> IntrusiveRefCntPtr<FileManager> FileMgr; >> IntrusiveRefCntPtr<SourceManager> SourceMgr; >> >> <…> >> >> DiagnosticsEngine &Diag, LangOptions &LangOpts, >> SourceManager &SourceMgr, FileManager &FileMgr, >> + vfs::FileSystem &VFS, >> >> Why do we need to keep the VFS separately, isn’t it owned by the FileManager >> ? >> > > No, it is conceptually owned by the CompilerInstance. The FileManager is a > user (albeit the only user right now). I say conceptually owned, because it > is a ref-counted object that doesn’t really have a single owner. So taking into account that it is a ref-counted object with no single owner, and FileManager already has reference to it (I think considering the FileManager as one of its owners makes sense IMO), why do we need it as a field in ASTUnit class and passing around as parameter when a FileManager parameter is already there ? > >> >> Would it be better if a >> IntrusiveRefCntPtr<vfs::FileSystem> FS; >> is part of FileSystemOptions ? And created at the time with get the >> FileSystemOptions for the compiler invocation ? >> >> It seems it would simplify a bunch of code. > > This would confuse the division of responsibilities between the compiler > invocation (which owns the FileSystemOptions) and the compiler instance. I > don’t think that the constructed VFS belongs in the compiler invocation. I’m > also not sure we should be reading and parsing VFS files during command-line > argument parsing. Fair enough, but on a related note, doesn’t the “VFSOverlayFiles” belong to the FileSystemOptions and not the HeaderSearchOptions ? > > Ben > >> >> On Apr 11, 2014, at 2:14 PM, Ben Langmuir <[email protected]> wrote: >> >>> Hi Dmitri and Argyrios, >>> >>> Could one (or both) of you take a look at my changes to the ASTUnit to >>> support the VFS? The VFS needs to be created for most/all of the >>> FileManagers that get created, and I’m a bit worried by the sheer number of >>> FileManager and SourceManager creations that I needed to plug up. >>> >>> Ben >>> >>> <astunit.patch> >> > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
