Skip to content

Commit

Permalink
Revert "[clang] static operators should evaluate object argument (#68485
Browse files Browse the repository at this point in the history
)"

This reverts commit 30155fc.

It seems to have broken some tests in clangd:
http://45.33.8.238/linux/129484/step_9.txt
  • Loading branch information
AaronBallman committed Jan 30, 2024
1 parent 3477bcf commit 201eb2b
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 158 deletions.
2 changes: 0 additions & 2 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ 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
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
9 changes: 2 additions & 7 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7951,8 +7951,7 @@ 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() || (OCE && MD->isStatic()))) {
if (MD && MD->isImplicitObjectMemberFunction()) {
// 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 @@ -7961,11 +7960,7 @@ class ExprEvaluatorBase

if (!EvaluateObjectArgument(Info, Args[0], ThisVal))
return false;

// If we are calling a static operator, the 'this' argument needs to be
// ignored after being evaluated.
if (MD->isInstance())
This = &ThisVal;
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: 2 additions & 15 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5848,7 +5848,6 @@ 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 @@ -5866,22 +5865,10 @@ 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;
}

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);
EmitCallArgs(Args, dyn_cast<FunctionProtoType>(FnType), E->arguments(),
E->getDirectCallee(), /*ParamsToSkip*/ 0, Order);

const CGFunctionInfo &FnInfo = CGM.getTypes().arrangeFreeFunctionCall(
Args, FnType, /*ChainCall=*/Chain);
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7611,8 +7611,9 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
unsigned NumArgs = TheCall->getNumArgs();

Expr *ImplicitThis = nullptr;
if (IsMemberOperatorCall && !FDecl->hasCXXExplicitFunctionObjectParameter()) {
// If this is a call to a member operator, hide the first
if (IsMemberOperatorCall && !FDecl->isStatic() &&
!FDecl->hasCXXExplicitFunctionObjectParameter()) {
// If this is a call to a non-static 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: 26 additions & 18 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5664,15 +5664,10 @@ static ImplicitConversionSequence TryObjectArgumentInitialization(
assert(FromType->isRecordType());

QualType ClassType = S.Context.getTypeDeclType(ActingContext);
// 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).
// [class.dtor]p2: A destructor can be invoked for a const, volatile or
// const volatile object.
Qualifiers Quals = Method->getMethodQualifiers();
if (isa<CXXDestructorDecl>(Method) || Method->isStatic()) {
if (isa<CXXDestructorDecl>(Method)) {
Quals.addConst();
Quals.addVolatile();
}
Expand Down Expand Up @@ -15066,15 +15061,15 @@ ExprResult Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
CXXMethodDecl *Method = cast<CXXMethodDecl>(FnDecl);
SmallVector<Expr *, 2> MethodArgs;

// Initialize the object parameter.
// Handle 'this' parameter if the selected function is not static.
if (Method->isExplicitObjectMemberFunction()) {
ExprResult Res =
InitializeExplicitObjectArgument(*this, Args[0], Method);
if (Res.isInvalid())
return ExprError();
Args[0] = Res.get();
ArgExpr = Args;
} else {
} else if (Method->isInstance()) {
ExprResult Arg0 = PerformImplicitObjectArgumentInitialization(
Args[0], /*Qualifier=*/nullptr, Best->FoundDecl, Method);
if (Arg0.isInvalid())
Expand Down Expand Up @@ -15102,9 +15097,15 @@ ExprResult Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
ExprValueKind VK = Expr::getValueKindForType(ResultTy);
ResultTy = ResultTy.getNonLValueExprType(Context);

CallExpr *TheCall = CXXOperatorCallExpr::Create(
Context, OO_Subscript, FnExpr.get(), MethodArgs, ResultTy, VK, RLoc,
CurFPFeatureOverrides());
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());

if (CheckCallReturnType(FnDecl->getReturnType(), LLoc, TheCall, FnDecl))
return ExprError();
Expand Down Expand Up @@ -15732,13 +15733,15 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,

bool IsError = false;

// Initialize the object parameter.
// 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.
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 {
} else if (Method->isInstance()) {
ExprResult ObjRes = PerformImplicitObjectArgumentInitialization(
Object.get(), /*Qualifier=*/nullptr, Best->FoundDecl, Method);
if (ObjRes.isInvalid())
Expand Down Expand Up @@ -15772,9 +15775,14 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
ExprValueKind VK = Expr::getValueKindForType(ResultTy);
ResultTy = ResultTy.getNonLValueExprType(Context);

CallExpr *TheCall = CXXOperatorCallExpr::Create(
Context, OO_Call, NewFn.get(), MethodArgs, ResultTy, VK, RParenLoc,
CurFPFeatureOverrides());
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());

if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
return true;
Expand Down
55 changes: 0 additions & 55 deletions clang/test/AST/ast-dump-static-operators.cpp

This file was deleted.

26 changes: 7 additions & 19 deletions clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,16 @@ void CallsTheLambda() {

// CHECK: define {{.*}}CallsTheLambda{{.*}}
// CHECK-NEXT: entry:
// CHECK: {{.*}}call {{.*}}GetALambda{{.*}}()
// CHECK-NEXT: {{.*}} = call noundef i32 {{.*}}(i32 noundef 1, i32 noundef 2)
// CHECK-NEXT: %call = 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 @@ -43,8 +37,6 @@ 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 @@ -114,16 +106,12 @@ void test_dep_functors() {

// CHECK: define {{.*}}test_dep_functors{{.*}}
// CHECK-NEXT: entry:
// 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: %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: ret void
// CHECK-NEXT: }

Expand Down
11 changes: 2 additions & 9 deletions clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,12 @@ 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 @@ -26,8 +21,6 @@ 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 @@ -67,7 +60,7 @@ void test_dep_functors() {

// CHECK: define {{.*}}test_dep_functors{{.*}}
// CHECK-NEXT: entry:
// CHECK: {{.*}} = call noundef i32 {{.*}}DepFunctor{{.*}}(float noundef 1.000000e+00)
// CHECK: {{.*}} = call noundef i32 {{.*}}DepFunctor{{.*}}(i1 noundef zeroext true)
// CHECK: %call = call noundef i32 {{.*}}DepFunctor{{.*}}(float noundef 1.000000e+00)
// CHECK: %call1 = call noundef i32 {{.*}}DepFunctor{{.*}}(i1 noundef zeroext true)
// CHECK: ret void
// CHECK-NEXT: }
31 changes: 0 additions & 31 deletions clang/test/SemaCXX/cxx2b-static-operator.cpp

This file was deleted.

0 comments on commit 201eb2b

Please sign in to comment.