clang_tokenize() returns token for one-past-end token

[llvmbot]

Bugzilla Link	9069
Resolution	FIXED
Resolved on	Nov 10, 2016 02:55
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@emilio,@tkremenek

Extended Description

The clang_tokenize() function returns all tokens falling into a range, but seems to ignore the fact that the range's endpoint is actually outside the ROI.
Thus, the generated token-list is one element too long.

[tkremenek]

Stefan: Do you have a specific example to reproduce this problem (e.g., with c-index-test)? It will be easier to understand the problem you are seeing with an example.

[llvmbot]

I was just looking at the test suite but couldn't figure out how to easily add a test that would reproduce my observations.

Let me explain in more general terms:

I'm analyzing code such as this:

enum E { e0 = 1, e1 = 2};

In a visitor callback for the CXCursor_EnumConstantDecl cursors, I call clang_visitChildren, which invokes a callback on the single child cursor of type CXCursor_UnexposedExpr. Let's call that 'e'. I then call this:

CXRange r = clang_getCursorExtent(e);

CXToken *tokens;
unsigned num_tokens;
clang_tokenize(tu, r, &tokens, &num_tokens);

For the first enumerator above, the range 'r' starts at '1', and ends at ','. For the second enumerator, it starts at '2', and ends at '}'. I took this to mean that the end point actually points at the first token after the range.
However, calling clang_tokenize() on those ranges will yield 2 tokens, instead of one. It includes the literal (initializer), as well as the following punctuation (',' and '}' respectively.
So it seems that the range and clang_tokenize() don't agree on the semantics of what the end location stands for. The former seems to mean it to lie outside the ROI, while the clang_tokenize() function assumes it to be inside.

[llvmbot]

This fixes the issue.
This patch appears to fix the issue.

[llvmbot]

Is there any chance to get this fixed soon (in particular, for LLVM 2.9 ? It seems obvious enough, and trivial to validate. Much simpler than adding enough to the testing harness to test it formally.

Thanks,
Stefan

[llvmbot]

+1 for the fix.

I ran in to this bug as well. #​17497 seems to be a duplicate of this issue.

ffi-gen worked around this issue, albeit incorrectly:
https://github.com/neelance/ffi-gen/blob/master/lib/ffi/gen.rb#L31

ffi-clang did not work around the issue, and reproducing the bug with it is trivial:
https://gist.github.com/ghazel/28aaee21fa4726d88aaf

[llvmbot]

*** Bug llvm/llvm-bugzilla-archive#17497 has been marked as a duplicate of this bug. ***

[llvmbot]

+1 Could you please merge the patch?

[emilio]

This hit me this week, so I cleaned up the patch, added tests, and submitted it for review to https://reviews.llvm.org/D26446.

If someone could review it (is my first LLVM patch), or tell me if I need to do anything else, that'd help.

[emilio]

This is now fixed upstream :)

llvm-mirror/clang@3b61b92