[PATCH] D133659: [Clang] P1169R4: static operator()

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

Thanks a lot for working on that!

I think this is the right direction but i would like to see more tests.
Notably:

- converting a lambda with a static operator to a pointer
- tests in dependant contexts
- classes having both static and non-static `operator()` . especially the example in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1169r4.html#overload-resolution deserves a test.
- unevaluated lambda tests / constexpr tests

There is the question of whether we'd want the call operator to be implicitly static but I'm don't think it's currently allowed, it may have abi impact and it doesn't to be dealt with in this PR so that should not hold us.
The other question is whether we want to make static operator() an extension in older language modes. I don;t see a reason to prohibit that (with a warning)



================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1041
+def err_static_lambda_captures : Error<
+  "a static lambda cannot capture">;
+
----------------



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9075
+def ext_operator_overload_static : ExtWarn<
+  "making overloaded %0 static is a C++2b extension">, InGroup<CXXPre2bCompat>, DefaultIgnore;
+def err_call_operator_overload_static : Error<
----------------
For consistency and clarity, I'd say 'static overloaded %0'


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9077
+def err_call_operator_overload_static : Error<
+  "overloaded %0 cannot be a static member function before C++2b">;
 def err_operator_overload_static : Error<
----------------



================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:1265
+  if (Intro.hasLambdaCapture())
+    P.Diag(StaticLoc, diag::err_static_lambda_captures);
+}
----------------
We might want to add a note showing where the capture list is.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:5840
+  if (IsMemberOperatorCall && !FDecl->isStatic()) {
+    // If this is a call to an instance member operator, hide the first argument
     // from checkCall.
----------------



================
Comment at: clang/lib/Sema/SemaLambda.cpp:954-955
     //   by mutable. It is neither virtual nor declared volatile. [...]
-    if (!FTI.hasMutableQualifier()) {
+    if (!FTI.hasMutableQualifier() &&
+        ParamInfo.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static) {
       FTI.getOrCreateMethodQualifiers().SetTypeQual(DeclSpec::TQ_const,
----------------
Do we actually need that check ? It should be diagnosed already.


================
Comment at: clang/lib/Sema/SemaLambda.cpp:983-985
+      ParamInfo.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static
+          ? SC_Static
+          : SC_None,
----------------
Do we want an assert to make sure it's not something else than static or none?


================
Comment at: clang/test/CXX/over/over.oper/over.literal/p7.cpp:23-25
+struct Functor {
+  static int operator()(int a, int b);  // cxx11-error {{cannot be a static member function}}
+};
----------------
This doesn't really seem to be the right file for this test - I'd expect that to be in a test file where we do sema check on operators, not literal operators.
Do you need help looking for a better place?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133659/new/

https://reviews.llvm.org/D133659