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

[Erik Verbruggen]

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.