[clang] static operators should evaluate object argument (reland) #80108
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Shafik Yaghmour <[email protected]>
Co-authored-by: cor3ntin <[email protected]>
dtcxzyw
requested review from
shafik,
efriedma-quic,
zyn0217,
nico and
erichkeane
dtcxzyw
added
clangd
clang:codegen
|
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang-codegen Author: Tianlan Zhou (SuperSodaSea) ChangesDescriptionclang don't evaluate the object argument of #include <iostream>
struct Foo {
static int operator()(int x, int y) {
std::cout << "Foo::operator()" << std::endl;
return x + y;
}
static int operator[](int x, int y) {
std::cout << "Foo::operator[]" << std::endl;
return x + y;
}
};
Foo getFoo() {
std::cout << "getFoo()" << std::endl;
return {};
}
int main() {
std::cout << getFoo()(1, 2) << std::endl;
std::cout << getFoo()[1, 2] << std::endl;
}
Fixes #67976, reland #68485. Walkthrough
Tests
Documentation
Full diff: https://github.com/llvm/llvm-project/pull/80108.diff 10 Files Affected:
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index c7dce041474a1..671a9c30ffa95 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -651,16 +651,12 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
// implied object argument ([over.call.func]), the list of provided
// arguments is preceded by the implied object argument for the purposes of
// this correspondence...
- //
- // However, we don't have the implied object argument
- // for static operator() per clang::Sema::BuildCallToObjectOfClassType.
llvm::ArrayRef<const Expr *> Args = {E->getArgs(), E->getNumArgs()};
// We don't have the implied object argument through a function pointer
// either.
if (const CXXMethodDecl *Method =
dyn_cast_or_null<CXXMethodDecl>(Callee.Decl))
- if (Method->isInstance() &&
- (IsFunctor || Method->hasCXXExplicitFunctionObjectParameter()))
+ if (IsFunctor || Method->hasCXXExplicitFunctionObjectParameter())
Args = Args.drop_front(1);
processCall(Callee, Args);
return true;
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f0dea0c9bc89b..891623eb13c45 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -147,6 +147,8 @@ Bug Fixes to C++ Support
- Fix for crash when using a erroneous type in a return statement.
Fixes (`#63244 <https://github.com/llvm/llvm-project/issues/63244>`_)
and (`#79745 <https://github.com/llvm/llvm-project/issues/79745>`_)
+- Fix incorrect code generation caused by the object argument of ``static operator()`` and ``static operator[]`` calls not being evaluated.
+ Fixes (`#67976 <https://github.com/llvm/llvm-project/issues/67976>`_)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index a41bdac46c814..63453890d9879 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -7951,7 +7951,8 @@ class ExprEvaluatorBase
// Overloaded operator calls to member functions are represented as normal
// calls with '*this' as the first argument.
const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD);
- if (MD && MD->isImplicitObjectMemberFunction()) {
+ if (MD &&
+ (MD->isImplicitObjectMemberFunction() || (OCE && MD->isStatic()))) {
// FIXME: When selecting an implicit conversion for an overloaded
// operator delete, we sometimes try to evaluate calls to conversion
// operators without a 'this' parameter!
@@ -7960,7 +7961,11 @@ class ExprEvaluatorBase
if (!EvaluateObjectArgument(Info, Args[0], ThisVal))
return false;
- This = &ThisVal;
+
+ // If we are calling a static operator, the 'this' argument needs to be
+ // ignored after being evaluated.
+ if (MD->isInstance())
+ This = &ThisVal;
// If this is syntactically a simple assignment using a trivial
// assignment operator, start the lifetimes of union members as needed,
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 8c8c937b512ac..4a2f3caad6588 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5848,6 +5848,7 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
// destruction order is not necessarily reverse construction order.
// FIXME: Revisit this based on C++ committee response to unimplementability.
EvaluationOrder Order = EvaluationOrder::Default;
+ bool StaticOperator = false;
if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(E)) {
if (OCE->isAssignmentOp())
Order = EvaluationOrder::ForceRightToLeft;
@@ -5865,10 +5866,22 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
break;
}
}
+
+ if (const auto *MD =
+ dyn_cast_if_present<CXXMethodDecl>(OCE->getCalleeDecl());
+ MD && MD->isStatic())
+ StaticOperator = true;
}
- EmitCallArgs(Args, dyn_cast<FunctionProtoType>(FnType), E->arguments(),
- E->getDirectCallee(), /*ParamsToSkip*/ 0, Order);
+ auto Arguments = E->arguments();
+ if (StaticOperator) {
+ // If we're calling a static operator, we need to emit the object argument
+ // and ignore it.
+ EmitIgnoredExpr(E->getArg(0));
+ Arguments = drop_begin(Arguments, 1);
+ }
+ EmitCallArgs(Args, dyn_cast<FunctionProtoType>(FnType), Arguments,
+ E->getDirectCallee(), /*ParamsToSkip=*/0, Order);
const CGFunctionInfo &FnInfo = CGM.getTypes().arrangeFreeFunctionCall(
Args, FnType, /*ChainCall=*/Chain);
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 502b24bcdf8b4..fc91f5d3f8f88 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -7611,9 +7611,8 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
unsigned NumArgs = TheCall->getNumArgs();
Expr *ImplicitThis = nullptr;
- if (IsMemberOperatorCall && !FDecl->isStatic() &&
- !FDecl->hasCXXExplicitFunctionObjectParameter()) {
- // If this is a call to a non-static member operator, hide the first
+ if (IsMemberOperatorCall && !FDecl->hasCXXExplicitFunctionObjectParameter()) {
+ // If this is a call to a member operator, hide the first
// argument from checkCall.
// FIXME: Our choice of AST representation here is less than ideal.
ImplicitThis = Args[0];
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index c9eb678983563..940bcccb9e261 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -5664,10 +5664,15 @@ static ImplicitConversionSequence TryObjectArgumentInitialization(
assert(FromType->isRecordType());
QualType ClassType = S.Context.getTypeDeclType(ActingContext);
- // [class.dtor]p2: A destructor can be invoked for a const, volatile or
- // const volatile object.
+ // C++98 [class.dtor]p2:
+ // A destructor can be invoked for a const, volatile or const volatile
+ // object.
+ // C++98 [over.match.funcs]p4:
+ // For static member functions, the implicit object parameter is considered
+ // to match any object (since if the function is selected, the object is
+ // discarded).
Qualifiers Quals = Method->getMethodQualifiers();
- if (isa<CXXDestructorDecl>(Method)) {
+ if (isa<CXXDestructorDecl>(Method) || Method->isStatic()) {
Quals.addConst();
Quals.addVolatile();
}
@@ -15061,7 +15066,7 @@ ExprResult Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
CXXMethodDecl *Method = cast<CXXMethodDecl>(FnDecl);
SmallVector<Expr *, 2> MethodArgs;
- // Handle 'this' parameter if the selected function is not static.
+ // Initialize the object parameter.
if (Method->isExplicitObjectMemberFunction()) {
ExprResult Res =
InitializeExplicitObjectArgument(*this, Args[0], Method);
@@ -15069,7 +15074,7 @@ ExprResult Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
return ExprError();
Args[0] = Res.get();
ArgExpr = Args;
- } else if (Method->isInstance()) {
+ } else {
ExprResult Arg0 = PerformImplicitObjectArgumentInitialization(
Args[0], /*Qualifier=*/nullptr, Best->FoundDecl, Method);
if (Arg0.isInvalid())
@@ -15097,15 +15102,9 @@ ExprResult Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
ExprValueKind VK = Expr::getValueKindForType(ResultTy);
ResultTy = ResultTy.getNonLValueExprType(Context);
- CallExpr *TheCall;
- if (Method->isInstance())
- TheCall = CXXOperatorCallExpr::Create(
- Context, OO_Subscript, FnExpr.get(), MethodArgs, ResultTy, VK,
- RLoc, CurFPFeatureOverrides());
- else
- TheCall =
- CallExpr::Create(Context, FnExpr.get(), MethodArgs, ResultTy, VK,
- RLoc, CurFPFeatureOverrides());
+ CallExpr *TheCall = CXXOperatorCallExpr::Create(
+ Context, OO_Subscript, FnExpr.get(), MethodArgs, ResultTy, VK, RLoc,
+ CurFPFeatureOverrides());
if (CheckCallReturnType(FnDecl->getReturnType(), LLoc, TheCall, FnDecl))
return ExprError();
@@ -15733,15 +15732,13 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
bool IsError = false;
- // Initialize the implicit object parameter if needed.
- // Since C++23, this could also be a call to a static call operator
- // which we emit as a regular CallExpr.
+ // Initialize the object parameter.
llvm::SmallVector<Expr *, 8> NewArgs;
if (Method->isExplicitObjectMemberFunction()) {
// FIXME: we should do that during the definition of the lambda when we can.
DiagnoseInvalidExplicitObjectParameterInLambda(Method);
PrepareExplicitObjectArgument(*this, Method, Obj, Args, NewArgs);
- } else if (Method->isInstance()) {
+ } else {
ExprResult ObjRes = PerformImplicitObjectArgumentInitialization(
Object.get(), /*Qualifier=*/nullptr, Best->FoundDecl, Method);
if (ObjRes.isInvalid())
@@ -15775,14 +15772,9 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
ExprValueKind VK = Expr::getValueKindForType(ResultTy);
ResultTy = ResultTy.getNonLValueExprType(Context);
- CallExpr *TheCall;
- if (Method->isInstance())
- TheCall = CXXOperatorCallExpr::Create(Context, OO_Call, NewFn.get(),
- MethodArgs, ResultTy, VK, RParenLoc,
- CurFPFeatureOverrides());
- else
- TheCall = CallExpr::Create(Context, NewFn.get(), MethodArgs, ResultTy, VK,
- RParenLoc, CurFPFeatureOverrides());
+ CallExpr *TheCall = CXXOperatorCallExpr::Create(
+ Context, OO_Call, NewFn.get(), MethodArgs, ResultTy, VK, RParenLoc,
+ CurFPFeatureOverrides());
if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
return true;
diff --git a/clang/test/AST/ast-dump-static-operators.cpp b/clang/test/AST/ast-dump-static-operators.cpp
new file mode 100644
index 0000000000000..e8454bdac02f7
--- /dev/null
+++ b/clang/test/AST/ast-dump-static-operators.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++23 %s -ast-dump -triple x86_64-unknown-unknown -o - | FileCheck -strict-whitespace %s
+
+struct Functor {
+ static int operator()(int x, int y) {
+ return x + y;
+ }
+ static int operator[](int x, int y) {
+ return x + y;
+ }
+};
+
+Functor& get_functor() {
+ static Functor functor;
+ return functor;
+}
+
+void call_static_operators() {
+ Functor functor;
+
+ int z1 = functor(1, 2);
+ // CHECK: CXXOperatorCallExpr {{.*}} 'int' '()'
+ // CHECK-NEXT: |-ImplicitCastExpr {{.*}} <col:19, col:24> 'int (*)(int, int)' <FunctionToPointerDecay>
+ // CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:19, col:24> 'int (int, int)' lvalue CXXMethod {{.*}} 'operator()' 'int (int, int)'
+ // CHECK-NEXT: |-DeclRefExpr {{.*}} <col:12> 'Functor' lvalue Var {{.*}} 'functor' 'Functor'
+ // CHECK-NEXT: |-IntegerLiteral {{.*}} <col:20> 'int' 1
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} <col:23> 'int' 2
+
+ int z2 = functor[1, 2];
+ // CHECK: CXXOperatorCallExpr {{.*}} 'int' '[]'
+ // CHECK-NEXT: |-ImplicitCastExpr {{.*}} <col:19, col:24> 'int (*)(int, int)' <FunctionToPointerDecay>
+ // CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:19, col:24> 'int (int, int)' lvalue CXXMethod {{.*}} 'operator[]' 'int (int, int)'
+ // CHECK-NEXT: |-DeclRefExpr {{.*}} <col:12> 'Functor' lvalue Var {{.*}} 'functor' 'Functor'
+ // CHECK-NEXT: |-IntegerLiteral {{.*}} <col:20> 'int' 1
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} <col:23> 'int' 2
+
+ int z3 = get_functor()(1, 2);
+ // CHECK: CXXOperatorCallExpr {{.*}} 'int' '()'
+ // CHECK-NEXT: |-ImplicitCastExpr {{.*}} <col:25, col:30> 'int (*)(int, int)' <FunctionToPointerDecay>
+ // CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:25, col:30> 'int (int, int)' lvalue CXXMethod {{.*}} 'operator()' 'int (int, int)'
+ // CHECK-NEXT: |-CallExpr {{.*}} <col:12, col:24> 'Functor' lvalue
+ // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:12> 'Functor &(*)()' <FunctionToPointerDecay>
+ // CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:12> 'Functor &()' lvalue Function {{.*}} 'get_functor' 'Functor &()'
+ // CHECK-NEXT: |-IntegerLiteral {{.*}} <col:26> 'int' 1
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} <col:29> 'int' 2
+
+ int z4 = get_functor()[1, 2];
+ // CHECK: CXXOperatorCallExpr {{.*}} 'int' '[]'
+ // CHECK-NEXT: |-ImplicitCastExpr {{.*}} <col:25, col:30> 'int (*)(int, int)' <FunctionToPointerDecay>
+ // CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:25, col:30> 'int (int, int)' lvalue CXXMethod {{.*}} 'operator[]' 'int (int, int)'
+ // CHECK-NEXT: |-CallExpr {{.*}} <col:12, col:24> 'Functor' lvalue
+ // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:12> 'Functor &(*)()' <FunctionToPointerDecay>
+ // CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:12> 'Functor &()' lvalue Function {{.*}} 'get_functor' 'Functor &()'
+ // CHECK-NEXT: |-IntegerLiteral {{.*}} <col:26> 'int' 1
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} <col:29> 'int' 2
+}
diff --git a/clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp b/clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
index fd53649c9b061..9cf5a7e00e7b4 100644
--- a/clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
+++ b/clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
@@ -19,16 +19,22 @@ void CallsTheLambda() {
// CHECK: define {{.*}}CallsTheLambda{{.*}}
// CHECK-NEXT: entry:
-// CHECK-NEXT: %call = call noundef i32 {{.*}}(i32 noundef 1, i32 noundef 2)
+// CHECK: {{.*}}call {{.*}}GetALambda{{.*}}()
+// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}(i32 noundef 1, i32 noundef 2)
// CHECK-NEXT: ret void
// CHECK-NEXT: }
+Functor GetAFunctor() {
+ return {};
+}
+
void call_static_call_operator() {
Functor f;
f(101, 102);
f.operator()(201, 202);
Functor{}(301, 302);
Functor::operator()(401, 402);
+ GetAFunctor()(501, 502);
}
// CHECK: define {{.*}}call_static_call_operator{{.*}}
@@ -37,6 +43,8 @@ void call_static_call_operator() {
// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 201, i32 noundef 202)
// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 301, i32 noundef 302)
// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 401, i32 noundef 402)
+// CHECK: {{.*}}call {{.*}}GetAFunctor{{.*}}()
+// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 501, i32 noundef 502)
// CHECK-NEXT: ret void
// CHECK-NEXT: }
@@ -106,12 +114,16 @@ void test_dep_functors() {
// CHECK: define {{.*}}test_dep_functors{{.*}}
// CHECK-NEXT: entry:
-// CHECK: %call = call noundef i32 {{.*}}DepFunctor{{.*}}(float noundef 1.000000e+00)
-// CHECK: %call1 = call noundef i32 {{.*}}DepFunctor{{.*}}(i1 noundef zeroext true)
-// CHECK: %call2 = call noundef i32 {{.*}}dep_lambda1{{.*}}(float noundef 1.000000e+00)
-// CHECK: %call3 = call noundef i32 {{.*}}dep_lambda1{{.*}}(i1 noundef zeroext true)
-// CHECK: %call4 = call noundef i32 {{.*}}dep_lambda2{{.*}}(float noundef 1.000000e+00)
-// CHECK: %call5 = call noundef i32 {{.*}}dep_lambda2{{.*}}(i1 noundef zeroext true)
+// CHECK: {{.*}} = call noundef i32 {{.*}}DepFunctor{{.*}}(float noundef 1.000000e+00)
+// CHECK: {{.*}} = call noundef i32 {{.*}}DepFunctor{{.*}}(i1 noundef zeroext true)
+// CHECK: {{.*}}call {{.*}}dep_lambda1{{.*}}()
+// CHECK: {{.*}} = call noundef i32 {{.*}}dep_lambda1{{.*}}(float noundef 1.000000e+00)
+// CHECK: {{.*}}call {{.*}}dep_lambda1{{.*}}()
+// CHECK: {{.*}} = call noundef i32 {{.*}}dep_lambda1{{.*}}(i1 noundef zeroext true)
+// CHECK: {{.*}}call {{.*}}dep_lambda2{{.*}}()
+// CHECK: {{.*}} = call noundef i32 {{.*}}dep_lambda2{{.*}}(float noundef 1.000000e+00)
+// CHECK: {{.*}}call {{.*}}dep_lambda2{{.*}}()
+// CHECK: {{.*}} = call noundef i32 {{.*}}dep_lambda2{{.*}}(i1 noundef zeroext true)
// CHECK: ret void
// CHECK-NEXT: }
diff --git a/clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp b/clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp
index 5dbd2c50cc56b..5d8258978c50d 100644
--- a/clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp
+++ b/clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp
@@ -7,12 +7,17 @@ struct Functor {
}
};
+Functor GetAFunctor() {
+ return {};
+}
+
void call_static_subscript_operator() {
Functor f;
f[101, 102];
f.operator[](201, 202);
Functor{}[301, 302];
Functor::operator[](401, 402);
+ GetAFunctor()[501, 502];
}
// CHECK: define {{.*}}call_static_subscript_operator{{.*}}
@@ -21,6 +26,8 @@ void call_static_subscript_operator() {
// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 201, i32 noundef 202)
// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 301, i32 noundef 302)
// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 401, i32 noundef 402)
+// CHECK: {{.*}}call {{.*}}GetAFunctor{{.*}}()
+// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}Functor{{.*}}(i32 noundef 501, i32 noundef 502)
// CHECK-NEXT: ret void
// CHECK-NEXT: }
@@ -60,7 +67,7 @@ void test_dep_functors() {
// CHECK: define {{.*}}test_dep_functors{{.*}}
// CHECK-NEXT: entry:
-// CHECK: %call = call noundef i32 {{.*}}DepFunctor{{.*}}(float noundef 1.000000e+00)
-// CHECK: %call1 = call noundef i32 {{.*}}DepFunctor{{.*}}(i1 noundef zeroext true)
+// CHECK: {{.*}} = call noundef i32 {{.*}}DepFunctor{{.*}}(float noundef 1.000000e+00)
+// CHECK: {{.*}} = call noundef i32 {{.*}}DepFunctor{{.*}}(i1 noundef zeroext true)
// CHECK: ret void
// CHECK-NEXT: }
diff --git a/clang/test/SemaCXX/cxx2b-static-operator.cpp b/clang/test/SemaCXX/cxx2b-static-operator.cpp
new file mode 100644
index 0000000000000..4d6f1f76d1315
--- /dev/null
+++ b/clang/test/SemaCXX/cxx2b-static-operator.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++23 %s
+
+// expected-no-diagnostics
+
+namespace A {
+
+struct Foo {
+ static int operator()(int a, int b) { return a + b; }
+ static int operator[](int a, int b) { return a + b; }
+};
+
+void ok() {
+ // Should pass regardless of const / volatile
+ Foo foo;
+ foo(1, 2);
+ foo[1, 2];
+
+ const Foo fooC;
+ fooC(1, 2);
+ fooC[1, 2];
+
+ const Foo fooV;
+ fooV(1, 2);
+ fooV[1, 2];
+
+ const volatile Foo fooCV;
+ fooCV(1, 2);
+ fooCV[1, 2];
+}
+
+}
|
cor3ntin
reviewed
Jan 31, 2024
Comment on lines
-663
to
+659
| (IsFunctor || Method->hasCXXExplicitFunctionObjectParameter())) | ||
| if (IsFunctor || Method->hasCXXExplicitFunctionObjectParameter()) |
There was a problem hiding this comment.
I don;t understand where the change is. isInstance is implied by hasCXXExplicitFunctionObjectParameter so isn't that change doing... nothing?
There was a problem hiding this comment.
IsFunctor can be true when isInstance() is false, this is the situation (static functor) we are dealing with.
There was a problem hiding this comment.
See https://github.com/llvm/llvm-project/pull/68485/files#diff-19c518dbc68b30c66e1a2b6bd523c005fb2050dcf1a0e92305df7ab3e1b9e9f3L15744-L15751. The patch actually changes the behavior when we put an implied object argument. (I was replying at wrong place in that PR...)
There was a problem hiding this comment.
So we may have an implied object argument for static operator() calls without an explicit object parameter, while it was only applied previously to non-static (isInstance()) operator() calls.
zyn0217
approved these changes
Jan 31, 2024
There was a problem hiding this comment.
Thanks again for working on it. The clangd part LGTM, and since the other part was left as-is and had been accepted in our previous attempt #68485, I believe we can now reland the patch.
Please keep an eye on the CI in case we have missed anything else.
tstellar
pushed a commit
that referenced
this pull request
Feb 3, 2024
…eland)' to release/18.x (#80109) Cherry picked from commit ee01a2c. Closes #80041, backport #80108. Co-authored-by: Shafik Yaghmour <[email protected]> Co-authored-by: cor3ntin <[email protected]> Co-authored-by: Aaron Ballman <[email protected]>
tstellar
pushed a commit
to tstellar/llvm-project
that referenced
this pull request
Feb 14, 2024
…eland)' to release/18.x (llvm#80109) Cherry picked from commit ee01a2c. Closes llvm#80041, backport llvm#80108. Co-authored-by: Shafik Yaghmour <[email protected]> Co-authored-by: cor3ntin <[email protected]> Co-authored-by: Aaron Ballman <[email protected]>
tstellar
pushed a commit
to tstellar/llvm-project
that referenced
this pull request
Feb 14, 2024
…eland)' to release/18.x (llvm#80109) Cherry picked from commit ee01a2c. Closes llvm#80041, backport llvm#80108. Co-authored-by: Shafik Yaghmour <[email protected]> Co-authored-by: cor3ntin <[email protected]> Co-authored-by: Aaron Ballman <[email protected]>
tstellar
pushed a commit
to tstellar/llvm-project
that referenced
this pull request
Feb 14, 2024
…eland)' to release/18.x (llvm#80109) Cherry picked from commit ee01a2c. Closes llvm#80041, backport llvm#80108. Co-authored-by: Shafik Yaghmour <[email protected]> Co-authored-by: cor3ntin <[email protected]> Co-authored-by: Aaron Ballman <[email protected]>
tstellar
pushed a commit
to tstellar/llvm-project
that referenced
this pull request
Feb 14, 2024
…eland)' to release/18.x (llvm#80109) Cherry picked from commit ee01a2c. Closes llvm#80041, backport llvm#80108. Co-authored-by: Shafik Yaghmour <[email protected]> Co-authored-by: cor3ntin <[email protected]> Co-authored-by: Aaron Ballman <[email protected]>
xgupta
pushed a commit
to xgupta/llvm-project
that referenced
this pull request
Sep 9, 2024
…eland)' to release/18.x (llvm#80109) Cherry picked from commit ee01a2c. Closes llvm#80041, backport llvm#80108. Co-authored-by: Shafik Yaghmour <[email protected]> Co-authored-by: cor3ntin <[email protected]> Co-authored-by: Aaron Ballman <[email protected]>
xgupta
pushed a commit
to xgupta/llvm-project
that referenced
this pull request
Oct 10, 2024
…eland)' to release/18.x (llvm#80109) Cherry picked from commit ee01a2c. Closes llvm#80041, backport llvm#80108. Co-authored-by: Shafik Yaghmour <[email protected]> Co-authored-by: cor3ntin <[email protected]> Co-authored-by: Aaron Ballman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
clang don't evaluate the object argument of
static operator()andstatic operator[]currently, for example:getFoo()is expected to be called, but clang don't call it currently (17.0.6). This PR fixes this issue.Fixes #67976, reland #68485.
Walkthrough
Sema::CreateOverloadedArraySubscriptExpr&Sema::BuildCallToObjectOfClassTypePreviously clang generate
CallExprfor static operators, ignoring the object argument. In this PRCXXOperatorCallExpris generated for static operators instead, with the object argument as the first argument.TryObjectArgumentInitializationconst/volatileobjects are allowed for static methods, so that we can call static operators on them.CodeGenFunction::EmitCallCodeGen changes for
CXXOperatorCallExprwith static operators: emit and ignore the object argument first, then emit the operator call.ExprEvaluatorBase::handleCallExprEvaluation of static operators in constexpr also need some small changes to work, so that the arguments won't be out of position.
Sema::CheckFunctionCallCode for argument checking also need to be modify, or it will fail the test
clang/test/SemaCXX/overloaded-operator-decl.cpp.InlayHintVisitor::VisitCallExprNow that the
CXXOperatorCallExprfor static operators also have object argument, we should also take care of this situation in clangd.Tests
Verify the AST generated for static operators.
Static operators should be able to be called on const / volatile objects.
Matching the new CodeGen.
Documentation
Update release notes.