[Clang] Missing source location in diagnostic for invalid conversion in function w/ explicit object parameter

[Sirraide]

Consider (https://godbolt.org/z/aa475YbW6):

struct X{};

template <typename T = void>
struct A {
    auto Error(this auto&& This) -> T {
        return T();
    }
};

struct B : A<X> {
    bool f();
};

bool B::f() {
    return Error();
}

Obviously, X isn’t convertible to bool, so we print an error, but the error is missing a source location:

error: no viable conversion from returned value of type 'X' to function return type 'bool'

If I remove the explicit object parameter, we instead print

<source>:15:12: error: no viable conversion from returned value of type 'X' to function return type 'bool'
   15 |     return Error();
      |            ^~~~~~~

This appears to be a regression since the source location is present in Clang 19.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (Sirraide)

Consider (https://godbolt.org/z/aa475YbW6): ```c++ struct X{};

template <typename T = void>
struct A {
auto Error(this auto&& This) -> T {
return T();
}
};

struct B : A<X> {
bool f();
};

bool B::f() {
return Error();
}

Obviously, `X` isn’t convertible to `bool`, so we print an error, but the error is missing a source location:
```console
error: no viable conversion from returned value of type 'X' to function return type 'bool'

If I remove the explicit object parameter, we instead print

<source>:15:12: error: no viable conversion from returned value of type 'X' to function return type 'bool'
   15 |     return Error();
      |            ^~~~~~~

This appears to be a regression since the source location is present in Clang 19.

[tbaederr]

Debug hint:

diff --git i/clang/lib/Sema/SemaOverload.cpp w/clang/lib/Sema/SemaOverload.cpp
index f46ef2c7f5bd..377747195e4d 100644
--- i/clang/lib/Sema/SemaOverload.cpp
+++ w/clang/lib/Sema/SemaOverload.cpp
@@ -15782,6 +15782,9 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE,
     TheCall =
         CallExpr::Create(Context, FnExpr.get(), Args, ResultType, VK, RParenLoc,
                          CurFPFeatureOverrides(), Proto->getNumParams());
+    TheCall->dumpColor();
+    assert(!TheCall->getExprLoc().isInvalid());
+
   } else {
     // Convert the object argument (for a non-static member function call).
     ExprResult ObjectArg = PerformImplicitObjectArgumentInitialization(

[tbaederr]

Actually, this makes it more obvious:

diff --git i/clang/lib/Sema/SemaOverload.cpp w/clang/lib/Sema/SemaOverload.cpp
index f46ef2c7f5bd..482f4d2231bd 100644
--- i/clang/lib/Sema/SemaOverload.cpp
+++ w/clang/lib/Sema/SemaOverload.cpp
@@ -6543,6 +6543,10 @@ static Expr *GetExplicitObjectExpr(Sema &S, Expr *Obj,
     Obj = UnaryOperator::Create(S.getASTContext(), Obj, UO_Deref, ObjType,
                                 VK_LValue, OK_Ordinary, SourceLocation(),
                                 /*CanOverflow=*/false, FPOptionsOverride());
+
+    assert(!Obj->getExprLoc().isInvalid());
+
+
   }
   return Obj;
 }

we're explicitly passing an invalid source location above.

[Ankur-0429]

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

Line 15191 in cbe8f3a

if (!S.getLangOpts().CPlusPlus23)

If I set this conditional to be true, the source location shows up now in Clang 23.

[zyn0217]

I reverted these lines and the source location comes back:

llvm-project/clang/lib/AST/Expr.cpp

Lines 1648 to 1660 in 150e7b1

	// A non-dependent call to a member function with an explicit object parameter
	// is modelled with the object expression being the first argument, e.g. in
	// `o.f(x)`, the callee will be just `f`, and `o` will be the first argument.
	// Since the first argument is written before the callee, the expression's
	// begin location should come from the first argument.
	// This does not apply to dependent calls, which are modelled with `o.f`
	// being the callee.
	if (!isTypeDependent()) {
	if (const auto *Method =
	dyn_cast_if_present<const CXXMethodDecl>(getCalleeDecl());
	Method && Method->isExplicitObjectMemberFunction())
	return getArg(0)->getBeginLoc();
	}

@HighCommander4

[HighCommander4]

Thanks for the ping. Investigating.

[HighCommander4]

A more minimal testcase:

struct A {
  auto Error(this auto) -> A;
  bool f() { return Error(); }
};

Analysis: the CallExpr for Error() is a call to an explicit object member function, and it has a first argument (so we take the branch of CallExpr::getBeginLoc() linked to in this comment), but that argument is the implicit *this expression, so it does not have a valid begin location. In such a case, we should fall through and return the begin location of the callee.

[HighCommander4]

Proposed fix: #135757