Page MenuHomePhabricator

[libclang] Fix cursors for arguments of Subscript and Call operators
ClosedPublic

Authored by nik on Nov 27 2017, 2:40 AM.

Details

Summary

The DeclRefExpr of CXXOperatorCallExpr refering to the custom operator
is visited before the arguments to the operator call. For the Call and
Subscript operator the range of this DeclRefExpr includes the whole call
expression, so that all tokens in that range were mapped to the operator
function, even the tokens of the arguments.

Fix this by ensuring that this particular DeclRefExpr is visited last.

Fixes PR25775.

Diff Detail

Repository
rL LLVM

Event Timeline

nik created this revision.Nov 27 2017, 2:40 AM
yvvan added inline comments.Nov 27 2017, 2:53 AM
tools/libclang/CIndex.cpp
6445 ↗(On Diff #124339)

4 spaces instead of 2

6535 ↗(On Diff #124339)

missing {}

6666 ↗(On Diff #124339)

4 spaces indentation instead of 2

6860 ↗(On Diff #124339)

cover with {}

6888 ↗(On Diff #124339)

4 spaces indentation instead of 2

6892 ↗(On Diff #124339)

same

nik marked 4 inline comments as done.Nov 27 2017, 2:59 AM
nik added inline comments.
tools/libclang/CIndex.cpp
6888 ↗(On Diff #124339)

That's not me, but clang-format, so I guess it's fine as is.

6892 ↗(On Diff #124339)

That's not me, but clang-format, so I guess it's fine as is.

nik updated this revision to Diff 124341.Nov 27 2017, 3:01 AM

Addressed coding style issues.

yvvan added inline comments.Nov 27 2017, 3:28 AM
tools/libclang/CIndex.cpp
6888 ↗(On Diff #124339)

oh, you're right. it's ok here and in the next one

nik added a comment.Jan 2 2018, 11:22 PM

New year, new hope - ping :)

nik added a reviewer: jbcoe.Jan 19 2018, 7:16 AM
jbcoe resigned from this revision.Jan 27 2018, 2:14 AM
jbcoe removed a reviewer: jbcoe.
nik added a comment.Apr 19 2018, 7:09 AM

This one still applies and tests pass.

Please review.

yvvan added a comment.May 17 2018, 1:15 AM

Tests run fine but the solution feels like a workaround.

Nevertheless if we are sure that extending the subscript/call operator range does not work properly or breaks too many other things than it's probably fine to have this workaround.

yvvan accepted this revision.Aug 23 2018, 1:11 AM

Let's proceed with this one. I really see that it's going to be useful.

This revision is now accepted and ready to land.Aug 23 2018, 1:11 AM
This revision was automatically updated to reflect the committed changes.