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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to