-
Notifications
You must be signed in to change notification settings - Fork 13.7k
[clang][AST] Handle implicit first argument in CallExpr::getBeginLoc() #135757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang Author: Nathan Ridge (HighCommander4) ChangesFixes #135522 Full diff: https://github.com/llvm/llvm-project/pull/135757.diff 2 Files Affected:
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 5fab2c73f214b..59c0e47c7c195 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1652,8 +1652,11 @@ SourceLocation CallExpr::getBeginLoc() const {
if (!isTypeDependent()) {
if (const auto *Method =
dyn_cast_if_present<const CXXMethodDecl>(getCalleeDecl());
- Method && Method->isExplicitObjectMemberFunction())
- return getArg(0)->getBeginLoc();
+ Method && Method->isExplicitObjectMemberFunction()) {
+ if (auto FirstArgLoc = getArg(0)->getBeginLoc(); FirstArgLoc.isValid()) {
+ return FirstArgLoc;
+ }
+ }
}
SourceLocation begin = getCallee()->getBeginLoc();
diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index 6f17ce7275456..7e392213710a4 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -1134,3 +1134,10 @@ struct S {
static_assert((S{} << 11) == a);
// expected-error@-1 {{use of undeclared identifier 'a'}}
}
+
+namespace GH135522 {
+struct S {
+ auto f(this auto) -> S;
+ bool g() { return f(); } // expected-error {{no viable conversion from returned value of type 'S' to function return type 'bool'}}
+};
+}
|
Method && Method->isExplicitObjectMemberFunction()) | ||
return getArg(0)->getBeginLoc(); | ||
Method && Method->isExplicitObjectMemberFunction()) { | ||
if (auto FirstArgLoc = getArg(0)->getBeginLoc(); FirstArgLoc.isValid()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, I took the approach of falling through if FirstArgLoc
is invalid for any reason; presumably we never want to return an invalid location when we might fall through to a valid one in getCallee()->getBeginLoc()
.
We could alternatively make the check more specific, to e.g. !getArg(0)->isImplicitCXXThis()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a changelog entry?
Otherwise LGTM
d7e4fb0
to
a156972
Compare
Added changelog entry. Should we backport this to 20.x? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, would be great to see it get backported
Filed backport issue: #135922 |
Fixes #135522