[clang] static operators should evaluate object argument #68485

SuperSodaSea · 2023-10-07T13:40:11Z

Description

clang don't evaluate the object argument of static operator() and static operator[] currently, for example:

#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.

SuperSodaSea · 2023-10-07T13:40:42Z

CC @dtcxzyw, @zygoloid

llvmbot · 2023-10-07T13:41:11Z

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Changes

Description

clang don't evaluate the object argument of static operator() and static operator[] currently, for example:

#include &lt;iostream&gt;

struct Foo {
    static int operator()(int x, int y) {
        std::cout &lt;&lt; "Foo::operator()" &lt;&lt; std::endl;
        return x + y;
    }
    static int operator[](int x, int y) {
        std::cout &lt;&lt; "Foo::operator[]" &lt;&lt; std::endl;
        return x + y;
    }
};
Foo getFoo() {
    std::cout &lt;&lt; "getFoo()" &lt;&lt; std::endl;
    return {};
}
int main() {
    std::cout &lt;&lt; getFoo()(1, 2) &lt;&lt; std::endl;
    std::cout &lt;&lt; getFoo()[1, 2] &lt;&lt; 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
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.
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/CodeGen/CodeGenFunction.h
CodeGen changes for CXXOperatorCallExpr with static operators. CodeGenFunction::EmitCXXStaticOperatorMemberCallExpr is added for handling this situation: emit and ignore the object argument first, then emit the operator call.
clang/lib/AST/ExprConstant.cpp
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
Code for argument checking also need to be modify, or it will fail the test clang/test/AST/ast-dump-static-operators.cpp.

Tests

Added:
- clang/test/AST/ast-dump-static-operators.cpp: verify the AST generated for static operators
Modified:
- clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp
- clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp

Full diff: https://github.com/llvm/llvm-project/pull/68485.diff

9 Files Affected:

(modified) clang/lib/AST/ExprConstant.cpp (+2-1)
(modified) clang/lib/CodeGen/CGExpr.cpp (+1-1)
(modified) clang/lib/CodeGen/CGExprCXX.cpp (+36-5)
(modified) clang/lib/CodeGen/CodeGenFunction.h (+3)
(modified) clang/lib/Sema/SemaChecking.cpp (+2-3)
(modified) clang/lib/Sema/SemaOverload.cpp (+10-23)
(added) clang/test/AST/ast-dump-static-operators.cpp (+55)
(modified) clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp (+19-7)
(modified) clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp (+9-2)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 5a33e918db8e8c0..a6c81f467fbe01c 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -7806,7 +7806,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!
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 54a1d300a9ac738..19406ff174dea14 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5070,7 +5070,7 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E,
   if (const auto *CE = dyn_cast<CXXOperatorCallExpr>(E))
     if (const auto *MD =
             dyn_cast_if_present<CXXMethodDecl>(CE->getCalleeDecl());
-        MD && MD->isImplicitObjectMemberFunction())
+        MD && !MD->isExplicitObjectMemberFunction())
       return EmitCXXOperatorMemberCallExpr(CE, MD, ReturnValue);
 
   CGCallee callee = EmitCallee(E->getCallee());
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 2e7059cc8f5b639..a580c635998510f 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -489,11 +489,42 @@ RValue
 CodeGenFunction::EmitCXXOperatorMemberCallExpr(const CXXOperatorCallExpr *E,
                                                const CXXMethodDecl *MD,
                                                ReturnValueSlot ReturnValue) {
-  assert(MD->isImplicitObjectMemberFunction() &&
-         "Trying to emit a member call expr on a static method!");
-  return EmitCXXMemberOrOperatorMemberCallExpr(
-      E, MD, ReturnValue, /*HasQualifier=*/false, /*Qualifier=*/nullptr,
-      /*IsArrow=*/false, E->getArg(0));
+  assert(!MD->isExplicitObjectMemberFunction() &&
+         "Trying to emit a member call expr on an explicit object member "
+         "function!");
+
+  if (MD->isStatic())
+    return EmitCXXStaticOperatorMemberCallExpr(E, MD, ReturnValue);
+  else
+    return EmitCXXMemberOrOperatorMemberCallExpr(
+        E, MD, ReturnValue, /*HasQualifier=*/false, /*Qualifier=*/nullptr,
+        /*IsArrow=*/false, E->getArg(0));
+}
+
+RValue CodeGenFunction::EmitCXXStaticOperatorMemberCallExpr(
+    const CXXOperatorCallExpr *E, const CXXMethodDecl *MD,
+    ReturnValueSlot ReturnValue) {
+  assert(MD->isStatic());
+
+  CGCallee Callee = EmitCallee(E->getCallee());
+
+  // Emit and ignore `this` pointer.
+  EmitIgnoredExpr(E->getArg(0));
+
+  auto ProtoType = MD->getFunctionType()->castAs<FunctionProtoType>();
+
+  // Emit the rest of the call args.
+  CallArgList Args;
+  EmitCallArgs(Args, ProtoType, drop_begin(E->arguments(), 1),
+               E->getDirectCallee());
+
+  bool Chain = E == MustTailCall;
+  const CGFunctionInfo &FnInfo =
+      CGM.getTypes().arrangeFreeFunctionCall(Args, ProtoType, Chain);
+  llvm::CallBase *CallOrInvoke = nullptr;
+
+  return EmitCall(FnInfo, Callee, ReturnValue, Args, &CallOrInvoke, Chain,
+                  E->getExprLoc());
 }
 
 RValue CodeGenFunction::EmitCUDAKernelCallExpr(const CUDAKernelCallExpr *E,
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index d5336382a2b9c95..42de125e7489911 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4163,6 +4163,9 @@ class CodeGenFunction : public CodeGenTypeCache {
   RValue EmitCXXOperatorMemberCallExpr(const CXXOperatorCallExpr *E,
                                        const CXXMethodDecl *MD,
                                        ReturnValueSlot ReturnValue);
+  RValue EmitCXXStaticOperatorMemberCallExpr(const CXXOperatorCallExpr *CE,
+                                             const CXXMethodDecl *MD,
+                                             ReturnValueSlot ReturnValue);
   RValue EmitCXXPseudoDestructorExpr(const CXXPseudoDestructorExpr *E);
 
   RValue EmitCUDAKernelCallExpr(const CUDAKernelCallExpr *E,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 446e35218bff0ad..a536f5f72811ec9 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -6942,9 +6942,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 ce78994e6553814..82b4bfe980b7963 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -14935,7 +14935,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 explicit / implicit object parameter.
         if (Method->isExplicitObjectMemberFunction()) {
           ExprResult Res =
               InitializeExplicitObjectArgument(*this, Args[0], Method);
@@ -14943,7 +14943,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())
@@ -14971,15 +14971,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();
@@ -15607,15 +15601,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 explicit / implicit 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())
@@ -15649,14 +15641,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 000000000000000..87a15403e6f8b23
--- /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':'Functor' lvalue Var {{.*}} 'functor' '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':'Functor' lvalue Var {{.*}} 'functor' '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':'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':'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 fd53649c9b06186..9cf5a7e00e7b4ef 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 5dbd2c50cc56bd2..5d8258978c50d57 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: }

clang/lib/Sema/SemaOverload.cpp

clang/lib/AST/ExprConstant.cpp

clang/lib/CodeGen/CGExpr.cpp

Co-authored-by: Shafik Yaghmour <shafik@users.noreply.github.com>

SuperSodaSea · 2023-11-13T15:49:05Z

Ping @cor3ntin @shafik

dtcxzyw · 2023-12-15T05:26:38Z

Ping.

dtcxzyw · 2024-01-04T08:19:50Z

Ping.
@SuperSodaSea Please add a release note to clang/docs/ReleaseNotes.rst so users know about the fix.

cor3ntin

I think this looks fine, once we have a release note

cor3ntin

LGTM

SuperSodaSea · 2024-01-30T13:07:41Z

Should be easy to backport using llvmbot once it is merged.

AaronBallman

LGTM with some minor nits. In terms of backporting, it's a bit more complicated than I'd like for a backport, but I don't see anything concrete to suggest this is a bad candidate. I'll give it a +1 for backporting, but I'd love a second opinion from @rjmccall or @erichkeane (or others) just to be safe.

clang/lib/CodeGen/CGExpr.cpp

Co-authored-by: Aaron Ballman <aaron@aaronballman.com>

AaronBallman · 2024-01-30T17:48:29Z

LGTM now, thank you for bearing with me! Do you need one of us to commit this on your behalf?

SuperSodaSea · 2024-01-30T17:53:29Z

Yeah, I'd be happy if anyone with write access could help. I'll create a backport issue after the commit.

AaronBallman · 2024-01-30T18:09:31Z

Yeah, I'd be happy if anyone with write access could help. I'll create a backport issue after the commit.

I've landed the changes, thank you for the fix and the offer to help backport!

nico · 2024-01-30T18:21:27Z

This might've broken clangd tests: http://45.33.8.238/linux/129484/step_9.txt

Does that look related? If so, please take a look and revert for now if it takes a while to fix.

AaronBallman · 2024-01-30T18:37:59Z

Hmmm that does look related. I'll revert so we get back to green. CC @sam-mccall in case he has opinions on how we can ease tension between making fixes in Clang that impact clangd tests (clangd is a consumer of Clang and we usually expect consumers to react when their tests break due to conforming and correct changes in Clang). I mostly want to be mindful of the new contributor experience, as in this case.

)" This reverts commit 30155fc. It seems to have broken some tests in clangd: http://45.33.8.238/linux/129484/step_9.txt

erichkeane · 2024-01-30T18:59:33Z

I have absolutely no idea how to fix it as I don't understand the syntax of the clangd tests, but it SEEMS like it is just a source-location type thing that is wrong?

https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/unittests/InlayHintTests.cpp#L810

From my read of the test output, it is 'l2' on line 831 (which is line 20 of the test best i can tell?) where the 'suggestion' is chars 6-8 instead of 9-10.

I have no idea what any of that means however, so hopefully someone can stop by and help out @SuperSodaSea .

SuperSodaSea · 2024-01-30T19:16:23Z

It seems that clangd doesn't like the new usage for the static operator of the CXXOperatorCallExpr, so the hint is misplaced. I'm looking into clang-tools-extra/clangd/InlayHints.cpp now...

AaronBallman · 2024-01-30T19:16:35Z

CC @zyn0217 as the original author of that test case and @HighCommander4 as the original reviewer.

SuperSodaSea · 2024-01-30T19:32:57Z

https://github.com/SuperSodaSea/llvm-project/blob/1ceaae47b2b43fd8fa5512e20e0b32a7e8f4ab5b/clang-tools-extra/clangd/InlayHints.cpp#L660-L664

      if (const CXXMethodDecl *Method =
              dyn_cast_or_null<CXXMethodDecl>(Callee.Decl))
-       if (Method->isInstance() &&
-           (IsFunctor || Method->hasCXXExplicitFunctionObjectParameter()))
+       if (IsFunctor || (Method->isInstance() &&
+                         Method->hasCXXExplicitFunctionObjectParameter()))
          Args = Args.drop_front(1);

Here is the code need to be modified (!Method->isInstance() && IsFunctor also need to drop the first argument). Passed check-clangd on my own build.

Diff: 1ceaae4...910ae40

SuperSodaSea · 2024-01-30T19:38:00Z

By the way, maybe we should add a clangd label to this PR (and #80041)?

zyn0217 · 2024-01-31T02:53:26Z

clang/lib/Sema/SemaOverload.cpp

-          TheCall =
-              CallExpr::Create(Context, FnExpr.get(), MethodArgs, ResultTy, VK,
-                               RLoc, CurFPFeatureOverrides());
+        CallExpr *TheCall = CXXOperatorCallExpr::Create(


While I haven’t had time to go through this PR, these lines appear to be the reason for the test failure. We had been relying on these different CallExprs to tell whether we should drop the explicit object parameter at the clangd site (IsFunctor). And this seemingly breaks that expectation as well as changes the AST?

Yeah, with this change the CXXOperatorCall will always have an operator argument, even if the operator is static, this is intended in this PR.

In short, for the following code

// struct Foo { static int operator()(int a, int b); }; // Foo foo; foo(1, 2);

, the AST is changed from

CallExpr |- operator() |- a \- b

to

CXXOperatorCallExpr |- operator() |- foo |- a \- b

And other situations are not affected.

Thanks for the explanation! I think this is reasonable and makes it much more intuitive for static operator call expressions. So, if I understand the change correctly, I think we can simplify the condition testing at the clangd side: we can avoid the check isInstance on expressions. That means, can we just reduce the condition to IsFunctor && hasCXXExplicitFunctionObjectParameter?

I’m on my phone now, so I couldn't validate my thoughts. Could you please help me for it? I’m willing to help you reland the patch if that works!

You are right, hasCXXExplicitFunctionObjectParameter() should imply isInstance(), so isInstance() is no longer needed. Passed check-clangd on my own build.

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);

Diff: 1ceaae4...910ae40

Thanks. Given that the commit has been reverted, can you please submit a new relanding PR?

SuperSodaSea · 2024-01-31T07:04:57Z

Relanding at #80108.

…0108) This re-applies 30155fc with a fix for clangd. ### 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.6). This PR fixes this issue. Fixes #67976, reland #68485. ### 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`. - **clang-tools-extra/clangd/InlayHints.cpp** - **`InlayHintVisitor::VisitCallExpr`** Now that the `CXXOperatorCallExpr` for static operators also have object argument, we should also take care of this situation in clangd. ### 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>

…eland)' to release/18.x (cherry picked from commit ee01a2c) This re-applies 30155fc with a fix for clangd. clang don't evaluate the object argument of `static operator()` and `static operator[]` currently, for example: ```cpp 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.6). This PR fixes this issue. Fixes llvm#67976, reland llvm#68485. - **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`. - **clang-tools-extra/clangd/InlayHints.cpp** - **`InlayHintVisitor::VisitCallExpr`** Now that the `CXXOperatorCallExpr` for static operators also have object argument, we should also take care of this situation in clangd. - **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. - **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>

[clang] static operators should evaluate object argument

0327626

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Oct 7, 2023

dtcxzyw requested review from cor3ntin, AaronBallman and royjacobson October 7, 2023 13:46

SuperSodaSea mentioned this pull request Oct 7, 2023

Too aggresive C++23 static call operator optimization #67976

Closed

dtcxzyw requested a review from erichkeane October 7, 2023 13:52

Deal with static operator in EmitCall

63a3627

cor3ntin reviewed Oct 10, 2023

View reviewed changes

clang/lib/Sema/SemaOverload.cpp Outdated Show resolved Hide resolved

SuperSodaSea added 2 commits October 10, 2023 15:37

Apply suggestions from cor3ntin

1269bc3

Minor changes

755bcaa

shafik reviewed Oct 10, 2023

View reviewed changes

clang/lib/AST/ExprConstant.cpp Show resolved Hide resolved

clang/lib/CodeGen/CGExpr.cpp Outdated Show resolved Hide resolved

clang/lib/CodeGen/CGExpr.cpp Outdated Show resolved Hide resolved

SuperSodaSea and others added 2 commits October 11, 2023 17:41

Apply suggestions from shafik

12d3ea2

Co-authored-by: Shafik Yaghmour <shafik@users.noreply.github.com>

Ignore this in constexpr evaluation

e725a8f

SuperSodaSea requested review from cor3ntin and shafik October 23, 2023 14:51

cor3ntin reviewed Jan 4, 2024

View reviewed changes

SuperSodaSea added 4 commits January 4, 2024 22:56

Merge remote-tracking branch 'upstream/main' into fix/static-operator

441ca98

Update clang/docs/ReleaseNotes.rst

5accc5d

Merge remote-tracking branch 'upstream/main' into fix/static-operator

389d6d4

Update ast-dump-static-operators.cpp

6bcc590

cor3ntin approved these changes Jan 4, 2024

View reviewed changes

Merge branch 'main' into fix/static-operator

ab1dc2c

AaronBallman approved these changes Jan 30, 2024

View reviewed changes

clang/lib/CodeGen/CGExpr.cpp Outdated Show resolved Hide resolved

clang/lib/CodeGen/CGExpr.cpp Outdated Show resolved Hide resolved

SuperSodaSea and others added 3 commits January 30, 2024 22:02

Apply suggestion from @AaronBallman

5c3dfb3

Co-authored-by: Aaron Ballman <aaron@aaronballman.com>

Apply suggestion from @AaronBallman

c560b38

Use upper case

1ceaae4

AaronBallman merged commit 30155fc into llvm:main Jan 30, 2024
5 checks passed

SuperSodaSea deleted the fix/static-operator branch January 30, 2024 18:12

SuperSodaSea mentioned this pull request Jan 30, 2024

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

Closed

AaronBallman added a commit that referenced this pull request Jan 30, 2024

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

201eb2b

)" This reverts commit 30155fc. It seems to have broken some tests in clangd: http://45.33.8.238/linux/129484/step_9.txt

SuperSodaSea restored the fix/static-operator branch January 30, 2024 18:43

zyn0217 reviewed Jan 31, 2024

View reviewed changes

SuperSodaSea mentioned this pull request Jan 31, 2024

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang] static operators should evaluate object argument #68485

[clang] static operators should evaluate object argument #68485

SuperSodaSea commented Oct 7, 2023 •

edited

Loading

SuperSodaSea commented Oct 7, 2023

llvmbot commented Oct 7, 2023 •

edited

Loading

Description

Walkthrough

Tests

SuperSodaSea commented Nov 13, 2023

dtcxzyw commented Dec 15, 2023

dtcxzyw commented Jan 4, 2024 •

edited

Loading

cor3ntin left a comment

cor3ntin left a comment

SuperSodaSea commented Jan 30, 2024

AaronBallman left a comment

AaronBallman commented Jan 30, 2024

SuperSodaSea commented Jan 30, 2024

AaronBallman commented Jan 30, 2024

nico commented Jan 30, 2024

AaronBallman commented Jan 30, 2024

erichkeane commented Jan 30, 2024

SuperSodaSea commented Jan 30, 2024

AaronBallman commented Jan 30, 2024

SuperSodaSea commented Jan 30, 2024 •

edited

Loading

SuperSodaSea commented Jan 30, 2024 •

edited

Loading

zyn0217 Jan 31, 2024

SuperSodaSea Jan 31, 2024

SuperSodaSea Jan 31, 2024 •

edited

Loading

zyn0217 Jan 31, 2024

SuperSodaSea Jan 31, 2024

zyn0217 Jan 31, 2024

SuperSodaSea Jan 31, 2024

SuperSodaSea commented Jan 31, 2024

[clang] static operators should evaluate object argument #68485

[clang] static operators should evaluate object argument #68485

Conversation

SuperSodaSea commented Oct 7, 2023 • edited Loading

Description

Walkthrough

Tests

Documentation

SuperSodaSea commented Oct 7, 2023

llvmbot commented Oct 7, 2023 • edited Loading

Description

Walkthrough

Tests

SuperSodaSea commented Nov 13, 2023

dtcxzyw commented Dec 15, 2023

dtcxzyw commented Jan 4, 2024 • edited Loading

cor3ntin left a comment

Choose a reason for hiding this comment

cor3ntin left a comment

Choose a reason for hiding this comment

SuperSodaSea commented Jan 30, 2024

AaronBallman left a comment

Choose a reason for hiding this comment

AaronBallman commented Jan 30, 2024

SuperSodaSea commented Jan 30, 2024

AaronBallman commented Jan 30, 2024

nico commented Jan 30, 2024

AaronBallman commented Jan 30, 2024

erichkeane commented Jan 30, 2024

SuperSodaSea commented Jan 30, 2024

AaronBallman commented Jan 30, 2024

SuperSodaSea commented Jan 30, 2024 • edited Loading

SuperSodaSea commented Jan 30, 2024 • edited Loading

zyn0217 Jan 31, 2024

Choose a reason for hiding this comment

SuperSodaSea Jan 31, 2024

Choose a reason for hiding this comment

SuperSodaSea Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

zyn0217 Jan 31, 2024

Choose a reason for hiding this comment

SuperSodaSea Jan 31, 2024

Choose a reason for hiding this comment

zyn0217 Jan 31, 2024

Choose a reason for hiding this comment

SuperSodaSea Jan 31, 2024

Choose a reason for hiding this comment

SuperSodaSea commented Jan 31, 2024

SuperSodaSea commented Oct 7, 2023 •

edited

Loading

llvmbot commented Oct 7, 2023 •

edited

Loading

dtcxzyw commented Jan 4, 2024 •

edited

Loading

SuperSodaSea commented Jan 30, 2024 •

edited

Loading

SuperSodaSea commented Jan 30, 2024 •

edited

Loading

SuperSodaSea Jan 31, 2024 •

edited

Loading