CallExpr has incorrect begin location with "deducing this"

[HighCommander4]

In the following code:

struct S
{
    int bar(this S&);
};

int main()
{
    S waldo;
    int x = waldo.bar();
}

The AST dump of main(), taken with a recent trunk, is:

`-FunctionDecl 0x55ac3a911f78 <line:6:1, line:10:1> line:6:5 main 'int ()'
  `-CompoundStmt 0x55ac3a912988 <line:7:1, line:10:1>
    |-DeclStmt 0x55ac3a9127c8 <line:8:5, col:12>
    | `-VarDecl 0x55ac3a9120a8 <col:5, col:7> col:7 used waldo 'S' callinit
    |   `-CXXConstructExpr 0x55ac3a9127a0 <col:7> 'S' 'void () noexcept'
    `-DeclStmt 0x55ac3a912970 <line:9:5, col:24>
      `-VarDecl 0x55ac3a9127f8 <col:5, col:23> col:9 x 'int' cinit
        `-CallExpr 0x55ac3a912948 <col:19, col:23> 'int'
          |-ImplicitCastExpr 0x55ac3a912930 <col:19> 'int (*)(S &)' <FunctionToPointerDecay>
          | `-DeclRefExpr 0x55ac3a9128b0 <col:19> 'int (S &)' lvalue CXXMethod 0x55ac3a911e58 'bar' 'int (S &)'
          `-DeclRefExpr 0x55ac3a912860 <col:13> 'S' lvalue Var 0x55ac3a9120a8 'waldo' 'S'

The CallExpr for waldo.bar() is reporting col:19 as its begin location, even though the descendant DeclRefExpr for waldo is at col:13.

This is misleading, and causes problems for tooling like clangd whose hit-testing algorithm (used for go-to-definition) skips the CallExpr node when hit-testing the waldo token because it falls outside of the CallExpr's reported source range (#116335).

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Nathan Ridge (HighCommander4)

In the following code:

struct S
{
    int bar(this S&);
};

int main()
{
    S waldo;
    int x = waldo.bar();
}

The AST dump of main(), taken with a recent trunk, is:

`-FunctionDecl 0x55ac3a911f78 <line:6:1, line:10:1> line:6:5 main 'int ()'
  `-CompoundStmt 0x55ac3a912988 <line:7:1, line:10:1>
    |-DeclStmt 0x55ac3a9127c8 <line:8:5, col:12>
    | `-VarDecl 0x55ac3a9120a8 <col:5, col:7> col:7 used waldo 'S' callinit
    |   `-CXXConstructExpr 0x55ac3a9127a0 <col:7> 'S' 'void () noexcept'
    `-DeclStmt 0x55ac3a912970 <line:9:5, col:24>
      `-VarDecl 0x55ac3a9127f8 <col:5, col:23> col:9 x 'int' cinit
        `-CallExpr 0x55ac3a912948 <col:19, col:23> 'int'
          |-ImplicitCastExpr 0x55ac3a912930 <col:19> 'int (*)(S &)' <FunctionToPointerDecay>
          | `-DeclRefExpr 0x55ac3a9128b0 <col:19> 'int (S &)' lvalue CXXMethod 0x55ac3a911e58 'bar' 'int (S &)'
          `-DeclRefExpr 0x55ac3a912860 <col:13> 'S' lvalue Var 0x55ac3a9120a8 'waldo' 'S'

The CallExpr for waldo.bar() is reporting col:19 as its begin location, even though the descendant DeclRefExpr for waldo is at col:13.

This is misleading, and causes problems for tooling like clangd whose hit-testing algorithm (used for go-to-definition) skips the CallExpr node when hit-testing the waldo token because it falls outside of the CallExpr's reported source range.

[zyn0217]

With a cursory look, I presume in the following code,

llvm-project/clang/lib/Sema/SemaOverload.cpp

Lines 15566 to 15568 in bca846d

	ExprResult FnExpr =
	CreateFunctionRefExpr(*this, Method, FoundDecl, MemExpr,
	HadMultipleCandidates, MemExpr->getExprLoc());

we should use MemExpr->getBeginLoc() as the location of the DeclRefExpr.

@HighCommander4 Do you want to work on this? Otherwise I think this is a good-first issue

[AaronBallman]

CC @cor3ntin

[HighCommander4]

I don't think #117345 is the right fix for this. The AST for the CallExpr is now:

        `-CallExpr 0x5572cbcf4f98 <col:13, col:23> 'int'
          |-ImplicitCastExpr 0x5572cbcf4f80 <col:13> 'int (*)(S &)' <FunctionToPointerDecay>
          | `-DeclRefExpr 0x5572cbcf4f00 <col:13> 'int (S &)' lvalue CXXMethod 0x5572cbcf4498 'bar' 'int (S &)'
          `-DeclRefExpr 0x5572cbcf4eb0 <col:13> 'S' lvalue Var 0x5572cbcf46e8 'waldo' 'S'

The begin location of the CallExpr is now col:13, but so is the begin location of the DeclRefExpr for bar, even though the token bar only starts at col:19.

From a hit-testing point of view, we could say the DeclRefExpr for bar "occludes" the DeclRefExpr for waldo, which is in a later child of the CallExpr.

I would expect the AST to be:

        `-CallExpr 0x5572cbcf4f98 <col:13, col:23> 'int'
          |-ImplicitCastExpr 0x5572cbcf4f80 <col:19> 'int (*)(S &)' <FunctionToPointerDecay>
          | `-DeclRefExpr 0x5572cbcf4f00 <col:19> 'int (S &)' lvalue CXXMethod 0x5572cbcf4498 'bar' 'int (S &)'
          `-DeclRefExpr 0x5572cbcf4eb0 <col:13> 'S' lvalue Var 0x5572cbcf46e8 'waldo' 'S'

i.e. only the begin location of the CallExpr changes to col:13, to reflect the smaller of the begin locations of its children.

[HighCommander4]

Proposed alternative fix: #117841

[HighCommander4]

Fixed by #117841.