[cfe-commits] r163588 - in /cfe/trunk: lib/Lex/PPDirectives.cpp test/Index/c-index-getCursor-pp.c

[Argyrios Kyrtzidis]

On Sep 17, 2012, at 12:44 PM, James Dennett <jdennett at googlers.com> wrote:

> On Mon, Sep 10, 2012 at 7:17 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>> Author: akirtzidis
>> Date: Mon Sep 10 21:17:21 2012
>> New Revision: 163588
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=163588&view=rev
>> Log:
>> [libclang] Fix getting a cursor inside an angled #include directive.
>> 
>> Fixed by pointing the end location of the preprocessed entity for the #include
>> at the closing '>', instead of the start of '<'.
>> 
>> rdar://11113134
>> 
>> Modified:
>>    cfe/trunk/lib/Lex/PPDirectives.cpp
>>    cfe/trunk/test/Index/c-index-getCursor-pp.c
>> 
>> Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=163588&r1=163587&r2=163588&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
>> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Sep 10 21:17:21 2012
>> @@ -1296,6 +1296,9 @@
>>   case tok::string_literal:
>>     Filename = getSpelling(FilenameTok, FilenameBuffer);
>>     End = FilenameTok.getLocation();
>> +    // For an angled include, point the end location at the closing '>'.
>> +    if (FilenameTok.is(tok::angle_string_literal))
>> +      End = End.getLocWithOffset(Filename.size()-1);
>>     CharEnd = End.getLocWithOffset(Filename.size());
>>     break;
>> 
>> 
>> Modified: cfe/trunk/test/Index/c-index-getCursor-pp.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/c-index-getCursor-pp.c?rev=163588&r1=163587&r2=163588&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Index/c-index-getCursor-pp.c (original)
>> +++ cfe/trunk/test/Index/c-index-getCursor-pp.c Mon Sep 10 21:17:21 2012
>> @@ -15,6 +15,8 @@
>> 
>> const char *fname = __FILE__;
>> 
>> +#include <a.h>
>> +
>> // RUN: c-index-test -cursor-at=%s:1:11 -I%S/Inputs %s | FileCheck -check-prefix=CHECK-1 %s
>> // CHECK-1: macro definition=OBSCURE
>> // RUN: c-index-test -cursor-at=%s:2:14 -I%S/Inputs %s | FileCheck -check-prefix=CHECK-2 %s
>> @@ -31,6 +33,8 @@
>> // CHECK-7: macro expansion=B:12:9
>> // RUN: c-index-test -cursor-at=%s:16:25 -I%S/Inputs %s | FileCheck -check-prefix=CHECK-8 %s
>> // CHECK-8: macro expansion=__FILE__
>> +// RUN: c-index-test -cursor-at=%s:18:12 -I%S/Inputs %s | FileCheck -check-prefix=CHECK-9 %s
>> +// CHECK-9: inclusion directive=a.h
>> 
>> // Same tests, but with "editing" optimizations
>> // RUN: env CINDEXTEST_EDITING=1 c-index-test -cursor-at=%s:1:11 -I%S/Inputs %s | FileCheck -check-prefix=CHECK-1 %s
> 
> This change broke clients that implement
> PPCallbacks::InclusionDirective, which now receive the location of the
> closing '>' rather than the location of the token that they're
> expecting based on the spec:
>  /// \param EndLoc The location of the last token within the
> inclusion
>  /// directive.
> The last (preprocessing) token is the whole of <filename>, of type
> tok::angle_string_literal.
> 
> Could we consider reverting this change and finding an alternative way
> to fix libclang without breaking/changing the lower-level interface?

Hi James,

This is not a high-level libclang issue.
The preprocessing record needs to know the '>' location and depends on the PPCallbacks::InclusionDirective callback. Previously this callback did not provide a way to reliably get at that location.
I believe we still need to modify the bahavior of the InclusionDirective callback and focus the discussion there.

How about reverting the EndLoc adjustment but have the InclusionDirective also pass the FilenameTok token so that receivers of the callback can have more information for the inclusion directive (then the preprocessing record can do the adjustment itself).

-Argyrios

> 
> -- James