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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin, lichray.
aaron.ballman added a comment.

In D133659#3808095 <https://reviews.llvm.org/D133659#3808095>, @royjacobson wrote:

> Thanks everyone! So if no one else has comments I'm planning to merge this tomorrow.

FWIW, you shouldn't merge something that has nobody signed on as the reviewer (and has only mild acceptance from people subscribing) -- a better approach is to add some reviewers, get them to accept, then land. Otherwise, coming back to this review in the future when doing code archeology leads to confusion about whether the changes should have even gone in at all, etc. I've added some more reviewers so we can make sure this gets the pretty green checkmark before landing. :-)



================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1033
+def err_static_lambda: Error<
+  "static lambdas is a C++2b extension">;
+def warn_cxx20_compat_static_lambda: Warning<
----------------
This is a bit confusing -- we're saying it's an extension but then making it an error? I've reworded based on how I think you're intending it to be issued.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041
+def err_static_mutable_lambda : Error<
+  "lambda cannot be both mutable and static">;
+def err_static_lambda_captures : Error<
+  "a static lambda cannot have any captures">;
+def note_lambda_captures : Note<"captures declared here">;
----------------
These are semantic errors, not parsing ones. This means these will be diagnosed when parsing the lambda rather than when instantiating it. I don't think that matters for the cast of combining `mutable` and `static`, but I'm less certain about "have any captures" because of cases like:
```
template <typename... Types>
auto func(Types... Ts) {
  return [Ts...] { return 1; };
}

int main() {
  auto lambda = func();
}
```
I'm pretty sure that lambda has no captures for that call, but it could have captures depending on the instantiation.

Actually, from some off-list discussion with @erichkeane, even mutable and static are a problem in code like:
```
template <typename Ty>
void func(T t) {
  if constexpr (Something<T>) {
    [](){};
  } else {
    [t](){};
  }
```
where the lambda is in a discarded statement.

So I think these might need to change to be Sema diagnostics (and we should add some additional test coverage).


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



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15938-15944
+        Diag(FnDecl->getLocation(),
+             (LangOpts.CPlusPlus2b ? diag::ext_operator_overload_static
+                                   : diag::err_call_operator_overload_static))
+            << FnDecl->getDeclName();
+      else
+        return Diag(FnDecl->getLocation(), diag::err_operator_overload_static)
+               << FnDecl->getDeclName();
----------------
The diagnostic formatter knows how to format anything that inherits from `NamedDecl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133659