[PATCH] Use -ivfsoverlay in ASTUnit

Argyrios Kyrtzidis kyrtzidis at apple.com
Mon Apr 14 09:16:31 PDT 2014


On Apr 14, 2014, at 8:45 AM, Ben Langmuir <blangmuir at apple.com> wrote:

> 
> On Apr 11, 2014, at 5:45 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com> 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 <blangmuir at apple.com> 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>
>> 
> 





More information about the cfe-commits mailing list