[cfe-dev] Module maps, __FILE__ and lazy stat'ing of headers

[Manuel Klimek]

Richard, as this patch doesn't apply cleanly any more - can you outline
what you were trying to do in SourceManager.h? I have a hard time figuring
out which places that use FileEntry now need to use FileName...

Cheers,
/Manuel


On Tue, Aug 5, 2014 at 4:51 PM, Manuel Klimek <klimek at google.com> wrote:

> As this is blocking us, I'm taking a stab at it...
>
>
> On Wed, Jul 30, 2014 at 12:04 AM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Mon, Jul 28, 2014 at 11:40 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>>
>>>
>>> On 28 Jul 2014 17:57, "Ben Langmuir" <blangmuir at apple.com> wrote:
>>> >
>>> >
>>> >> On Jul 28, 2014, at 4:37 PM, Richard Smith <richard at metafoo.co.uk>
>>> wrote:
>>> >>
>>> >> On Mon, Jul 28, 2014 at 3:10 PM, Ben Langmuir <blangmuir at apple.com>
>>> wrote:
>>> >>>
>>> >>>
>>> >>> > On Jul 28, 2014, at 4:31 AM, Manuel Klimek <klimek at google.com>
>>> wrote:
>>> >>> >
>>> >>> > Hi Richard,
>>> >>> >
>>> >>> > while working with module maps for layering checks a problem with
>>> __FILE__ we noticed triggered a couple of questions.
>>> >>> >
>>> >>> > The problem is that __FILE__ uses the path of the first 'stat'
>>> call (as that is how FileManager::getFile() works).
>>> >>>
>>> >>> I was thinking about this a while ago and independently of this
>>> issue I would really like to change this behaviour at some point.  The name
>>> of a file is a property of how you look it up, not an intrinsic part of the
>>> file itself.
>>> >>
>>> >>
>>> >> I agree. We have incorrectly conflated the notion of the file
>>> identity (inode) with the directory entriy, and we can't tell the two
>>> apart. This leads to weird behavior in a number of places. For instance:
>>> >>
>>> >>  a/
>>> >>   x.h: #include "y.h"
>>> >>   y.h: int a = 0;
>>> >>  b/
>>> >>   x.h: symlink to a/x.h
>>> >>   y.h: int b = 0;
>>> >>  main.c:
>>> >>   #include "a/x.h"
>>> >>   #include "b/x.h"
>>> >>   int main() { return a + b; }
>>> >>
>>> >> On a correct compiler, this would work. For clang, it fails, because
>>> b/x.h's #include finds a/y.h, because we use the path by which we first
>>> found x.h as the One True Path to x.h. (This also leads to wrong __FILE__,
>>> etc.)
>>> >>
>>> >> I tried fixing this ~2 years ago by splitting FileEntry into separate
>>> dentry and inode classes, but this rapidly snowballed and exposed the same
>>> design error being made in various other components.
>>> >
>>> >
>>> > Interesting.  My motivation was keeping track of virtual and “real”
>>> paths for the VFS, which is a special case of the above.  Maybe we can sink
>>> the dentry/inode down to the VFS layer eventually?  Getting symlinks and
>>> “..” entries to work in the VFS make a lot more sense when we have explicit
>>> dentries rather than inferring from the file path.
>>> >
>>> > I’d be interested in what issues you ran into here if you remember.
>>>
>>> I'll see if I can dig out my patch tomorrow.
>>>
>> Attached is my WIP patch from (as it turns out) over 2 years ago. My
>> (very) hazy memories were that we had quite a few different places where
>> people were using FileEntry*s for things, and neither a dentry nor an inode
>> seemed like the "right" thing. I don't remember any more details than that.
>> There were also places where we would need to just make a decision, such
>> as: what should #pragma once use as its key? (I think dentry is the right
>> answer here, since the same file found in different directories might mean
>> different things, but that answer may break some people who use #pragma
>> once and don't also have include guards. Conversely, it fixes some builds
>> on content-addressed file systems.)
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140805/d058b145/attachment.html>