[cfe-commits] r164874 - in /cfe/trunk: include/clang/Lex/PPCallbacks.h include/clang/Lex/PreprocessingRecord.h lib/Frontend/DependencyFile.cpp lib/Frontend/DependencyGraph.cpp lib/Lex/PPDirectives.cpp lib/Lex/PreprocessingRecord.cpp lib/Lex/Prepr

[Richard Smith]

On Mon, Oct 1, 2012 at 11:01 AM, Chandler Carruth <chandlerc at google.com>wrote:

> On Mon, Oct 1, 2012 at 10:51 AM, Argyrios Kyrtzidis <kyrtzidis at apple.com>wrote:
>
>> Hi Kim,
>>
>> On Sep 30, 2012, at 12:29 AM, Kim Gräsman <kim.grasman at gmail.com> wrote:
>>
>> > On Sat, Sep 29, 2012 at 11:50 PM, Kim Gräsman <kim.grasman at gmail.com>
>> wrote:
>> >>
>> >> I would expect either '<stdio.h>' or
>> >> 'HEADER_NAME', but I think getting the expanded range ('<stdio.h>')
>> >> would be the most natural.
>> >
>> > Having looked into our callback implementation, we already handle
>> > macro expansion, and I think passing down the macro name range
>> > unexpanded gives more fidelity. So I change my mind -- I think getting
>> > the range as-written would be better ('HEADER_NAME'). Of course,
>> > assuming that FilenameRange.getBegin().isMacroID() is still true so
>> > callback sinks can expand it.
>>
>> Your suggestion is a bit contradictory, if we get the range
>> 'HEADER_NAME', then FilenameRange.getBegin().isMacroID() will be false
>> (since it's pointing at the beginning of the macro expansion).
>> The most fidelity option would be to give a range for  '<stdio.h>'
>> (MacroIDs for begin and end), from which a callback sink can get at the
>> 'HEADER_NAME' file-level range.
>> Is this your recommendation ?
>>
>
> FWIW, I think it is, and it makes sense. ;] The source location structure
> is... confusing though.
>
> Richard, thoughts?
>

Yes, I think we should maintain SourceLocation fidelity wherever possible.
Clients can convert to the expansion location themselves if they need to.

It looks like the CharEnd calculation for the string_literal and
angle_string_literal cases is wrong:

    Filename = getSpelling(FilenameTok, FilenameBuffer);
    End = FilenameTok.getLocation();
    CharEnd = End.getLocWithOffset(Filename.size());

You can't use getLocWithOffset to find the end of a token this way (it
won't work for trigraphs, at least). This should be using
getLocForEndOfToken, except that that apparently doesn't work inside a
macro expansion. (That looks like the same problem which is breaking Kim's
angled-include-macro case).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121001/87f06f4b/attachment.html>