Skip to content

Conversation

MitalAshok
Copy link
Contributor

Fixes #78355

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 16, 2024

@llvm/pr-subscribers-clang

Author: Mital Ashok (MitalAshok)

Changes

Fixes #78355


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

2 Files Affected:

  • (modified) clang/lib/Sema/SemaOverload.cpp (+13-7)
  • (modified) clang/test/SemaCXX/overloaded-operator.cpp (+27-1)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 37c62b306b3cd3..cde0f3c13e4d9c 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -15089,10 +15089,12 @@ ExprResult Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
           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());
+        else {
+          if (Base->isPRValue())
+            Base = CreateMaterializeTemporaryExpr(Base->getType().getNonReferenceType(), Base, /*BoundToLvalueReference=*/false);
+          ExprResult OperatorAccess = MemberExpr::Create(Context, Base, /*IsArrow=*/false, LLoc, NestedNameSpecifierLoc(), SourceLocation(), FnDecl, Best->FoundDecl, OpLocInfo, nullptr, FnDecl->getType(), ExprValueKind::VK_LValue, ExprObjectKind::OK_Ordinary, NonOdrUseReason::NOUR_None);
+          TheCall = CallExpr::Create(Context, CallExprUnaryConversions(OperatorAccess.get()).get(), MethodArgs, ResultTy, VK, RLoc, CurFPFeatureOverrides());
+        }
 
         if (CheckCallReturnType(FnDecl->getReturnType(), LLoc, TheCall, FnDecl))
           return ExprError();
@@ -15767,9 +15769,13 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
     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());
+  else {
+    Expr *Base = Object.get();
+    if (Base->isPRValue())
+      Base = CreateMaterializeTemporaryExpr(Base->getType().getNonReferenceType(), Base, /*BoundToLvalueReference=*/false);
+    ExprResult OperatorAccess = MemberExpr::Create(Context, Base, /*IsArrow=*/false, LParenLoc, NestedNameSpecifierLoc(), SourceLocation(), Method, Best->FoundDecl, OpLocInfo, nullptr, Method->getType(), ExprValueKind::VK_LValue, ExprObjectKind::OK_Ordinary, NonOdrUseReason::NOUR_None);
+    TheCall = CallExpr::Create(Context, CallExprUnaryConversions(OperatorAccess.get()).get(), MethodArgs, ResultTy, VK, RParenLoc, CurFPFeatureOverrides());
+  }
 
   if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
     return true;
diff --git a/clang/test/SemaCXX/overloaded-operator.cpp b/clang/test/SemaCXX/overloaded-operator.cpp
index 83a7e65b43dd01..361adda58e48b6 100644
--- a/clang/test/SemaCXX/overloaded-operator.cpp
+++ b/clang/test/SemaCXX/overloaded-operator.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s 
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+
 class X { };
 
 X operator+(X, X);
@@ -598,3 +600,27 @@ namespace B {
 }
 void g(B::X x) { A::f(x); }
 }
+
+namespace static_operator {
+#if __cplusplus >= 201703L
+struct X {
+  static constexpr int operator()() { return 0; }
+  // expected-warning@-1 {{declaring overloaded 'operator()' as 'static' is a C++23 extension}}
+  static constexpr int operator[](int) { return 0; }
+  // expected-warning@-1 {{declaring overloaded 'operator[]' as 'static' is a C++23 extension}}
+};
+
+constexpr int f(int x) {
+  return (++x, X())(), (++x, X())[1], x;
+}
+
+static_assert(f(0) == 2, "");
+
+constexpr int g(int x) {
+  return (++x, []() static { return 0; })(), x;
+  // expected-warning@-1 {{static lambdas are a C++23 extension}}
+}
+
+static_assert(g(0) == 1, "");
+#endif
+}

@MitalAshok MitalAshok force-pushed the static-call-operator-fix branch from 08c7087 to c6d4aca Compare January 16, 2024 21:59
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that more things are being done #68485. Have you double checked that PR?

@@ -598,3 +600,27 @@ namespace B {
}
void g(B::X x) { A::f(x); }
}

namespace static_operator {
#if __cplusplus >= 201703L
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why C++17?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No way to modify things in a C++11/14 constexpr function (++x is not a constant expression), so no way to make a positive test that the expression was evaluated. Though I guess I should have done (non_constexpr_fn(), X())() and see if that failed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No way to modify things in a C++11/14 constexpr function (++x is not a constant expression), so no way to make a positive test that the expression was evaluated. Though I guess I should have done (non_constexpr_fn(), X())() and see if that failed.

Oh I see. It's inconvenient that runtime results can't be easily asserted in this test file.

I think the related changes of constexpr were made in C++14 (N3652).

@MitalAshok MitalAshok marked this pull request as draft January 19, 2024 11:27
@MitalAshok
Copy link
Contributor Author

This pull request changes E(...) to produce the same AST as E.operator()(...), which is similar to E.f(...) for a static member function f.

But the linked #68485 changes this to CXXOperatorCallExpr, which is more appropriate since it is used for non-member call operators like anyways

@cor3ntin
Copy link
Contributor

@MitalAshok Can we close this?

@cor3ntin cor3ntin closed this Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Clang] E[...]/E(...) does not evaluate E if it calls a static overload
4 participants