[cfe-dev] patch for improving Clang's virtual file handling

[Zhanyong Wan (λx.x x)]

Thanks, Doug!

2011/2/10 Douglas Gregor <dgregor at apple.com>:
> Hi Zhanyong,
>
> Your patch looks good to me. Did you consider using the functionality in llvm::sys::path to handle the parsing of path names, rather than hand-coding string algorithm such as
>
> +void FileManager::addAncestorsAsVirtualDirs(llvm::StringRef Path) {
> +  size_t SlashPos = Path.size();
> +
> +  // Find the beginning of the last segment in Path.
> +  while (SlashPos != 0 && !IS_DIR_SEPARATOR_CHAR(Path[SlashPos-1]))
> +    --SlashPos;
> +
> +  // Ignore repeated //'s.
> +  while (SlashPos != 0 && IS_DIR_SEPARATOR_CHAR(Path[SlashPos-1]))
> +    --SlashPos;
> +
> +  if (SlashPos == 0)
> +    return;

I wasn't aware of llvm::sys::path, so I just followed what the
existing code in FileManager was doing.  I like the idea of switching
to llvm::sys::path.

May I commit the patch as is and send you a separate one for using
llvm::sys::path?  Thanks,

>
> ?
>
>        - Doug
>
> On Feb 7, 2011, at 2:17 PM, Zhanyong Wan (λx.x x) wrote:
>
>> Hi, Doug,
>>
>> Per our chat, please review the attached patch Manuel and I wrote for
>> improving Clang's virtual file handling.  I also uploaded the patch to
>> http://codereview.appspot.com/4126060 to make it easier to comment on
>> the individual lines.
>>
>> This patch contains:
>>
>> - making some of the existing comments more accurate in the presence
>> of virtual files/directories.
>>
>> - renaming some private data members of FileManager to match their roles better.
>>
>> - creating 'DirectorEntry's for the parent directories of virtual
>> files, such that we can tell whether two virtual files are from the
>> same directory.  This is useful for injecting virtual files whose
>> directories don't exist in the real file system.
>>
>> - minor clean-ups and adding comments for class
>> FileManager::UniqueDirContainer and FileManager::UniqueFileContainer.
>>
>> - adding statistics on virtual files to FileManager::PrintStats().
>>
>> I didn't add new tests, but all existing tests pass.  I plan to add
>> the tests in a new patch, as I'm still figuring out how to do that.
>>
>> Thanks a lot!
>> --
>> Zhanyong
>> <virtual-dir.patch>
>
>



-- 
Zhanyong