[PATCH] Record ranges skipped by the preprocessor and expose them with libclang.

[Erik Verbruggen]

Ping?

Also, attached is a updated version of the patch with the promised changes against svn rev. 194350.

-- Erik.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Record-ranges-skipped-by-the-preprocessor-and-expose.patch
Type: application/octet-stream
Size: 9101 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131112/603e0dc5/attachment.obj>
-------------- next part --------------


On 31 Oct 2013, at 17:36, Erik Verbruggen <erikjv at me.com> wrote:

> 
> On 31 Oct 2013, at 0:57, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
> 
>> Hi Erik, sorry for the delay!
>> 
>> On Oct 12, 2013, at 4:25 AM, Erik Verbruggen <erikjv at me.com> wrote:
>> 
>>> It has been some time since the last time I did a patch...
>>> 
>>> Record ranges skipped by the preprocessor and expose them with libclang.
>> 
>> Cool!
>> 
>>> 
>>> This requires the use of a detailed preprocessing record. Also bumbed the cindex minor version to reflect adding new functionality (and to be able to detect that during built-time).
>>> 
>>> The patch is against r192531, which has the same amount of failures for me on MacOS as trunk (two, both with OpenCL).
>>> 
>>> Feedback please! :)
>> 
>> Some nitpicks:
>> 
>> +/**
>> + * \brief Retrieve all ranges that were skipped by the preprocessor.
>> + */
>> +CINDEX_LINKAGE CXSkippedRanges *clang_getSkippedRanges(CXTranslationUnit tu);
>> 
>> Should we have a function to get all skipped ranges, and another to get the skipped ranges of a particular file ? IMO the latter is much more useful.
> 
> You're right of course. So for API design: add a const char *filename as second parameter?
> 
>> +
>> +    /// \brief Retrieve all ranges that got skipped while preprocessing.
>> +    const std::vector<SourceRange> &getSkippedRanges() const {
>> +      return SkippedRanges;
>> +    }
>> 
>> Please use ArrayRef here.
>> 
>> Also we would need to serialize the skipped ranges to the preprocessing record of a PCH as well but this can be done later.
> 
> Do we need that? I mean, my use-case is to grey-out the skipped ranges/lines in an IDE. When a file is opened that's part of a PCH, it will probably need a reparse anyway... But I might be missing something here.
> 
>> +// RUN: env CINDEXTEST_SHOW_SKIPPED_RANGES=1 c-index-test -test-annotate-tokens=%s:1:1:16:1 %s | FileCheck %s
>> +// CHECK: Skipping: [5:2 - 6:7]
>> +// CHECK: Skipping: [8:2 - 12:7]
>> +// CHECK: Skipping: [14:2 - 20:7]
>> 
>> Should we just unconditionally (I mean without the CINDEXTEST_SHOW_SKIPPED_RANGES env variable) show the skipped ranges for the '-test-annotate-tokens' option (and update any test if it breaks) ?
> 
> Will do that.
> 
>> Also the test is not also testing if tokens are properly annotated/not-annotated, could you add some CHECK/CHECK-NOT lines for some of the tokens ?
> 
> Sorry, I don't understand your question. Can you give an example?
> 
> I just noticed that my rebase removed the CINDEX_VERSION_MINOR bump. I'll add it back in, if that's okay?
> 
> Thanks!
> -- Erik.
>