Page MenuHomePhabricator

[libclang, bindings]: add spelling location
Needs ReviewPublic

Authored by frutiger on Oct 23 2017, 4:40 PM.

Details

Summary
  • Add a 'Location' class that represents the four properties of a physical location
  • Enhance 'SourceLocation' to provide 'expansion' and 'spelling' locations, maintaining backwards compatibility with existing code by forwarding the four properties to 'expansion'.
  • Update the implementation to use 'clang_getExpansionLocation' instead of the deprecated 'clang_getInstantiationLocation', which has been present since 2011.
  • Update the implementation of 'clang_getSpellingLocation' to actually obtain spelling location instead of file location.
  • Update test cases to account for changes to the result of 'clang_getSpellingLocation'.

Note: this commit is a reapplication of r316278 along with fixes to the
test cases.

Diff Detail

Event Timeline

frutiger updated this revision to Diff 119971.Oct 23 2017, 4:40 PM
frutiger created this revision.

Add context to the patch.

I would very much appreciate some guidance on whether or not this kind of a change in behaviour for clang_getSpellingLocation is an acceptable thing to do.

compnerd edited edge metadata.Oct 23 2017, 11:09 PM
compnerd added a subscriber: akyrtzi.

I think that this is reasonable given that it is addressing a long standing issue. CC'ing @akyrtzi for his opinion as well.

jklaehn added inline comments.
test/Index/annotate-tokens.c
226

Those look like an improvement to me since the MemberRefs of y and z no longer refer to the same line?

test/Index/c-index-getCursor-test.m
167

This does not seem to refer to a valid location? (line 2 only contains a comment)

frutiger added inline comments.Oct 24 2017, 2:43 PM
test/Index/c-index-getCursor-test.m
167

I don't really understand this output. What is -test-file-scan supposed to show?

jklaehn added inline comments.Oct 25 2017, 1:54 AM
test/Index/c-index-getCursor-test.m
167

Given this argument c-index-test will use clang_getCursor to iterate through all cursors in the file (by calling it on each character location and checking if it is equal to the previous cursor).
The numbers in brackets (e.g. [58:4 - 58:8]) indicate the locations for which clang_getCursor returned the same cursor. VarDecl=my_var:2:1 (Definition) is the output of PrintCursor. In this case :2:1 is IIUC produced by

Referenced = clang_getCursorReferenced(Cursor);
if (!clang_equalCursors(Referenced, clang_getNullCursor())) {
  if (clang_getCursorKind(Referenced) == CXCursor_OverloadedDeclRef) {
    // [...]
  } else {
    CXSourceLocation Loc = clang_getCursorLocation(Referenced);
    clang_getSpellingLocation(Loc, 0, &line, &column, 0);
    printf(":%d:%d", line, column);
  }

  // [...]
}

So it is affected by the change to clang_getSpellingLocation. I'm not sure what the referenced cursor is in this case. Maybe it would be helpful to also print its kind.

It looks like the my_var:2:1 refers to the result from clang_getCursorReferenced: https://github.com/llvm-mirror/clang/blob/d454549fce04dfedda6cc2825b66efca94effe3f/tools/c-index-test/c-index-test.c#L709-L711. I'm not sure what a referenced cursor is in this context, but the line and column appear to be related to the referenced cursor. As you say, there's nothing relevant at that location, so I'm not sure why it's returning 2 and 1.

@jklaehn do you know why the referenced cursor would point to line 2?