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.

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

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

Reply via email to