From f68238ee9375714f9616f9c401b5af45f11c0d93 Mon Sep 17 00:00:00 2001 From: Tianlan Zhou Date: Sun, 4 Feb 2024 02:35:40 +0800 Subject: [PATCH] Backport '[clang] static operators should evaluate object argument (reland)' to release/18.x (#80109) Cherry picked from commit ee01a2c3996f9647f3158f5acdb921a6ede94dc1. Closes #80041, backport #80108. Co-authored-by: Shafik Yaghmour Co-authored-by: cor3ntin Co-authored-by: Aaron Ballman --- clang-tools-extra/clangd/InlayHints.cpp | 6 +- clang/docs/ReleaseNotes.rst | 3 + clang/lib/AST/ExprConstant.cpp | 9 ++- clang/lib/CodeGen/CGExpr.cpp | 17 +++++- clang/lib/Sema/SemaChecking.cpp | 5 +- clang/lib/Sema/SemaOverload.cpp | 44 ++++++--------- clang/test/AST/ast-dump-static-operators.cpp | 55 +++++++++++++++++++ .../CodeGenCXX/cxx2b-static-call-operator.cpp | 26 ++++++--- .../cxx2b-static-subscript-operator.cpp | 11 +++- clang/test/SemaCXX/cxx2b-static-operator.cpp | 31 +++++++++++ 10 files changed, 160 insertions(+), 47 deletions(-) create mode 100644 clang/test/AST/ast-dump-static-operators.cpp create mode 100644 clang/test/SemaCXX/cxx2b-static-operator.cpp diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 5722ca8f66eb72..5f3687ac581017 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -651,16 +651,12 @@ class InlayHintVisitor : public RecursiveASTVisitor { // 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 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(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 2f4fe8bf7556e7..3ec187058db42a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -1054,6 +1054,9 @@ Bug Fixes to C++ Support Fixes (`#78830 `_) Fixes (`#60085 `_) +- Fix incorrect code generation caused by the object argument of ``static operator()`` and ``static operator[]`` calls not being evaluated. + Fixes (`#67976 `_) + Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed an import failure of recursive friend class template. diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index f1d07d022b2584..edf9b5e2d52bb3 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(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 c5f6b6d3a99f0b..f8f9979099775f 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -5846,6 +5846,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(E)) { if (OCE->isAssignmentOp()) Order = EvaluationOrder::ForceRightToLeft; @@ -5863,10 +5864,22 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee break; } } + + if (const auto *MD = + dyn_cast_if_present(OCE->getCalleeDecl()); + MD && MD->isStatic()) + StaticOperator = true; } - EmitCallArgs(Args, dyn_cast(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(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 7833d5a2ea20ee..25e9af1ea3f362 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -7643,9 +7643,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 c9eb6789835633..940bcccb9e261b 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(Method)) { + if (isa(Method) || Method->isStatic()) { Quals.addConst(); Quals.addVolatile(); } @@ -15061,7 +15066,7 @@ ExprResult Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc, CXXMethodDecl *Method = cast(FnDecl); SmallVector 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 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 00000000000000..e8454bdac02f7b --- /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 {{.*}} 'int (*)(int, int)' + // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'int (int, int)' lvalue CXXMethod {{.*}} 'operator()' 'int (int, int)' + // CHECK-NEXT: |-DeclRefExpr {{.*}} 'Functor' lvalue Var {{.*}} 'functor' 'Functor' + // CHECK-NEXT: |-IntegerLiteral {{.*}} 'int' 1 + // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2 + + int z2 = functor[1, 2]; + // CHECK: CXXOperatorCallExpr {{.*}} 'int' '[]' + // CHECK-NEXT: |-ImplicitCastExpr {{.*}} 'int (*)(int, int)' + // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'int (int, int)' lvalue CXXMethod {{.*}} 'operator[]' 'int (int, int)' + // CHECK-NEXT: |-DeclRefExpr {{.*}} 'Functor' lvalue Var {{.*}} 'functor' 'Functor' + // CHECK-NEXT: |-IntegerLiteral {{.*}} 'int' 1 + // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2 + + int z3 = get_functor()(1, 2); + // CHECK: CXXOperatorCallExpr {{.*}} 'int' '()' + // CHECK-NEXT: |-ImplicitCastExpr {{.*}} 'int (*)(int, int)' + // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'int (int, int)' lvalue CXXMethod {{.*}} 'operator()' 'int (int, int)' + // CHECK-NEXT: |-CallExpr {{.*}} 'Functor' lvalue + // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'Functor &(*)()' + // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'Functor &()' lvalue Function {{.*}} 'get_functor' 'Functor &()' + // CHECK-NEXT: |-IntegerLiteral {{.*}} 'int' 1 + // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2 + + int z4 = get_functor()[1, 2]; + // CHECK: CXXOperatorCallExpr {{.*}} 'int' '[]' + // CHECK-NEXT: |-ImplicitCastExpr {{.*}} 'int (*)(int, int)' + // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'int (int, int)' lvalue CXXMethod {{.*}} 'operator[]' 'int (int, int)' + // CHECK-NEXT: |-CallExpr {{.*}} 'Functor' lvalue + // CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'Functor &(*)()' + // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'Functor &()' lvalue Function {{.*}} 'get_functor' 'Functor &()' + // CHECK-NEXT: |-IntegerLiteral {{.*}} 'int' 1 + // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2 +} diff --git a/clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp b/clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp index fd53649c9b0618..9cf5a7e00e7b4e 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 5dbd2c50cc56bd..5d8258978c50d5 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 00000000000000..4d6f1f76d13157 --- /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]; +} + +}