Skip to content
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

Backport '[clang] static operators should evaluate object argument (reland)' to release/18.x #80109

Merged
merged 2 commits into from
Feb 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions clang-tools-extra/clangd/InlayHints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,9 @@ Bug Fixes to C++ Support
Fixes (`#78830 <https://github.com/llvm/llvm-project/issues/78830>`_)
Fixes (`#60085 <https://github.com/llvm/llvm-project/issues/60085>`_)

- 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
^^^^^^^^^^^^^^^^^^^^^^^^^
- Fixed an import failure of recursive friend class template.
Expand Down
9 changes: 7 additions & 2 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand All @@ -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,
Expand Down
17 changes: 15 additions & 2 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CXXOperatorCallExpr>(E)) {
if (OCE->isAssignmentOp())
Order = EvaluationOrder::ForceRightToLeft;
Expand All @@ -5863,10 +5864,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);
Expand Down
5 changes: 2 additions & 3 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
44 changes: 18 additions & 26 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -15061,15 +15066,15 @@ 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);
if (Res.isInvalid())
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())
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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;
Expand Down
55 changes: 55 additions & 0 deletions clang/test/AST/ast-dump-static-operators.cpp
Original file line number Diff line number Diff line change
@@ -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
}
26 changes: 19 additions & 7 deletions clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{{.*}}
Expand All @@ -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: }

Expand Down Expand Up @@ -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: }

Expand Down
11 changes: 9 additions & 2 deletions clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{{.*}}
Expand All @@ -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: }

Expand Down Expand Up @@ -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: }
31 changes: 31 additions & 0 deletions clang/test/SemaCXX/cxx2b-static-operator.cpp
Original file line number Diff line number Diff line change
@@ -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];
}

}
Loading