[clang] static operators should evaluate object argument (#68485)

### Description clang don't evaluate the object argument of `static operator()` and `static operator[]` currently, for example: ```cpp #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; } ``` `getFoo()` is expected to be called, but clang don't call it currently (17.0.2). This PR fixes this issue. Fixes #67976. ### Walkthrough - **clang/lib/Sema/SemaOverload.cpp** - **`Sema::CreateOverloadedArraySubscriptExpr` & `Sema::BuildCallToObjectOfClassType`** Previously clang generate `CallExpr` for static operators, ignoring the object argument. In this PR `CXXOperatorCallExpr` is generated for static operators instead, with the object argument as the first argument. - **`TryObjectArgumentInitialization`** `const` / `volatile` objects are allowed for static methods, so that we can call static operators on them. - **clang/lib/CodeGen/CGExpr.cpp** - **`CodeGenFunction::EmitCall`** CodeGen changes for `CXXOperatorCallExpr` with static operators: emit and ignore the object argument first, then emit the operator call. - **clang/lib/AST/ExprConstant.cpp** - **`‎ExprEvaluatorBase::handleCallExpr‎`** Evaluation of static operators in constexpr also need some small changes to work, so that the arguments won't be out of position. - **clang/lib/Sema/SemaChecking.cpp** - **`Sema::CheckFunctionCall`** Code for argument checking also need to be modify, or it will fail the test `clang/test/SemaCXX/overloaded-operator-decl.cpp`. ### Tests - **Added:** - **clang/test/AST/ast-dump-static-operators.cpp** Verify the AST generated for static operators. - **clang/test/SemaCXX/cxx2b-static-operator.cpp** Static operators should be able to be called on const / volatile objects. - **Modified:** - **clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp** - **clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp** Matching the new CodeGen. ### Documentation - **clang/docs/ReleaseNotes.rst** Update release notes. --------- Co-authored-by: Shafik Yaghmour <shafik@users.noreply.github.com> Co-authored-by: cor3ntin <corentinjabot@gmail.com> Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
2024-01-31 02:09:05 +08:00
parent 9f3854a01f
commit 30155fc0ef
9 changed files with 158 additions and 42 deletions
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -178,6 +178,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
 ^^^^^^^^^^^^^^^^^^^^^^^^^
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -7951,7 +7951,8 @@ public:
      // 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 @@ public:

        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,
--- 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);
--- 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];
--- 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;
--- a/clang/test/AST/ast-dump-static-operators.cpp
+++ 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
+}
--- 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: }

--- 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: }
--- a/clang/test/SemaCXX/cxx2b-static-operator.cpp
+++ 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];
+}
+
+}