[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

[Argyrios Kyrtzidis]

On Oct 29, 2012, at 5:38 AM, Kim Gräsman <kim.grasman at gmail.com> wrote:

> Could somebody take a look at this? Getting this behavior fixed and
> fixated would make our lives as PPCallbacks implementors much more
> comfortable :-)

I'll review it soon(ish).

> 
> Thanks,
> - Kim
> 
> On Sun, Oct 21, 2012 at 10:45 AM, Kim Gräsman <kim.grasman at gmail.com> wrote:
>> Ping.
>> 
>> On Mon, Oct 15, 2012 at 7:18 AM, Kim Gräsman <kim.grasman at gmail.com> wrote:
>>> Hello,
>>> 
>>> On Mon, Oct 8, 2012 at 11:13 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>> 
>>>> On Mon, Oct 8, 2012 at 1:21 PM, Kim Gräsman <kim.grasman at gmail.com> wrote:
>>>>> 
>>>>> There seems to be two problems: [...]
>>>> 
>>>> Both of those changes look correct to me. [IIRC, the 'spelling location' is
>>>> what you get by flattening a (hierarchical) source location down to a
>>>> (non-hierarchical) location where the character literally appeared in a
>>>> source file, and that's not quite what you mean here, but close enough.]
>>>> 
>>>>> Is there test coverage for this stuff somewhere? I'm not familiar with
>>>>> Clang's testing tools yet, but I would make an effort to get this
>>>>> under test if I knew where to start...
>>>> 
>>>> I can't find any tests for this. I think the best way to proceed would be to
>>>> add a PPCallbacksTest.cpp to unittests/Lex.
>>> 
>>> Attached is a patch for PPDirectives.cpp that implements these
>>> changes, as well as a new PPCallbacksTest.cpp that checks the
>>> FilenameRange in all these cases.
>>> 
>>> The tests use FilenameRange.begin/end to get pointers directly into
>>> SourceManager using getCharacterData(). This range is then formed into
>>> a string and I assert its contents. To me, this was the clearest way
>>> of demonstrating how it works, but I'm not sure if it's
>>> idiomatic/correct. Any comments welcome.
>>> 
>>> I've run this on Windows/VC10 and Ubuntu/GCC 4.6.3 -- the latter has a
>>> failure in Index/crash-recovery-modules.m with and without this patch,
>>> so I don't think it's related.
>>> 
>>> Thanks,
>>> - Kim