Skip to content

[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

Merged
merged 2 commits into from
Apr 15, 2025

Conversation

HighCommander4
Copy link
Collaborator

Fixes #135522

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 15, 2025

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

Changes

Fixes #135522


Full diff: https://github.com/llvm/llvm-project/pull/135757.diff

2 Files Affected:

  • (modified) clang/lib/AST/Expr.cpp (+5-2)
  • (modified) clang/test/SemaCXX/cxx2b-deducing-this.cpp (+7)
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()) {
Copy link
Collaborator Author

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().

Copy link
Contributor

@cor3ntin cor3ntin left a 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

@HighCommander4
Copy link
Collaborator Author

Added changelog entry.

Should we backport this to 20.x?

Copy link
Contributor

@zyn0217 zyn0217 left a 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

@HighCommander4 HighCommander4 merged commit 289baf1 into llvm:main Apr 15, 2025
11 of 12 checks passed
@HighCommander4
Copy link
Collaborator Author

Yes, would be great to see it get backported

Filed backport issue: #135922

var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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