[clang] Provide a more specific diagnostic for a misplaced lambda capture-default.

[riccibruno]

Summary

Currently a capture-default which is not the first element in the lambda-capture is diagnosed with a generic expected variable name or 'this' in lambda capture list, which is true but not very helpful.

If we don't have already parsed a capture-default then a lone "&" or "=" is likely to be a misplaced capture-default, so diagnose it as such.

[riccibruno]

clang-format the tests

[riccibruno]

Get rid of the note which doesn't add anything.

[aaron.ballman]

clang/lib/Parse/ParseExprCXX.cpp

937

Would it make sense to provide a fix-it to move the capture default to the start of the list automatically, or is that a pain?

clang/test/Parser/lambda-misplaced-capture-default.cpp

12

Can you also add a test showing that [=, &i] continues to behave properly (to test the &] checking logic)?

[riccibruno]

clang-format again, sigh...

[riccibruno]

Add some tests showing that a by-ref capture is still parsed properly, including some tests with a capture containing a pack expansion.

[aaron.ballman]

Thanks for the additional tests. This LGTM, though I still wonder about the fix-it (not certain if you saw the comment).

[riccibruno]

clang/lib/Parse/ParseExprCXX.cpp

937

I have thought about doing that, but my understanding is that a fix-it must fix the error. But here the capture-default might conflict with one of the other captures (both before and after).

To make sure that the fix-it is safe we would have to look at the other captures (before and after). This is certainly not impossible but a bit more tricky.

[aaron.ballman]

LGTM!

clang/lib/Parse/ParseExprCXX.cpp

937

That's a good reason to not do it right now, thanks!

clang/include/clang/Basic/DiagnosticParseKinds.td

def err_lambda_missing_parens : Error<
   "attribute specifier|'constexpr'|'consteval'|'requires' clause}0">;
 def err_lambda_decl_specifier_repeated : Error<
   "%select{'mutable'|'constexpr'|'consteval'}0 cannot appear multiple times in a lambda declarator">;
 def err_lambda_capture_misplaced_ellipsis : Error<
   "ellipsis in pack %select{|init-}0capture must appear %select{after|before}0 "
   "the name of the capture">;
 def err_lambda_capture_multiple_ellipses : Error<
   "multiple ellipses in pack capture">;
+def err_capture_default_first : Error<
+  "capture default must be first">;
 // C++17 lambda expressions
 def err_expected_star_this_capture : Error<
   "expected 'this' following '*' in lambda capture list">;
 
 // C++17 constexpr lambda expressions
 def warn_cxx14_compat_constexpr_on_lambda : Warning<
   "constexpr on lambda expressions is incompatible with C++ standards before C++17">,
   InGroup<CXXPre17Compat>, DefaultIgnore;

clang/lib/Parse/ParseExprCXX.cpp

if (Tok.is(tok::star)) {
       } else {
         return Invalid([&] {
           Diag(Tok.getLocation(), diag::err_expected_star_this_capture);
         });
       }
     } else if (Tok.is(tok::kw_this)) {
       Kind = LCK_This;
       Loc = ConsumeToken();
+    } else if (Tok.isOneOf(tok::amp, tok::equal) &&
+               NextToken().isOneOf(tok::comma, tok::r_square) &&
+               Intro.Default == LCD_None) {
+      // We have a lone "&" or "=" which is either a misplaced capture-default
+      // or the start of a capture (in the "&" case) with the rest of the
+      // capture missing. Both are an error but a misplaced capture-default
+      // is more likely if we don't already have a capture default.
+      return Invalid(
+          [&] { Diag(Tok.getLocation(), diag::err_capture_default_first); });
     } else {
       TryConsumeToken(tok::ellipsis, EllipsisLocs[0]);
 
       if (Tok.is(tok::amp)) {
         Kind = LCK_ByRef;
         ConsumeToken();
 
         if (Tok.is(tok::code_completion)) {

clang/test/Parser/lambda-misplaced-capture-default.cpp

+// RUN: %clang_cc1 -std=c++20 -Wno-unused-value -fsyntax-only -verify %s
+
+namespace misplaced_capture_default {
+void Test() {
+  int i = 0;
+  [&, i, &] {};   // expected-error {{expected variable name or 'this' in lambda capture list}}
+  [&, i, = ] {};  // expected-error {{expected variable name or 'this' in lambda capture list}}
+  [=, &i, &] {};  // expected-error {{expected variable name or 'this' in lambda capture list}}
+  [=, &i, = ] {}; // expected-error {{expected variable name or 'this' in lambda capture list}}
+
+  [i, &] {};   // expected-error {{capture default must be first}}
+  [i, = ] {};  // expected-error {{capture default must be first}}
+  [i, = x] {}; // expected-error {{expected variable name or 'this' in lambda capture list}}
+  [=, &i] {};  // ok
+  [&, &i] {};  // expected-error {{'&' cannot precede a capture when the capture default is '&'}}
+  [&x = i] {}; // ok
+  [=, &x = i] {};  // ok
+  [x = &i] {};     // ok
+  [=, &x = &i] {}; // expected-error {{non-const lvalue reference to type 'int *' cannot bind to a temporary of type 'int *'}}
+  [&, this] {}; // expected-error {{'this' cannot be captured in this context}}
+
+  [i, &, x = 2] {}; // expected-error {{capture default must be first}}
+  [i, =, x = 2] {}; // expected-error {{capture default must be first}}
+}
+} // namespace misplaced_capture_default
+
+namespace misplaced_capture_default_pack {
+template <typename... Args> void Test(Args... args) {
+  [&, args...] {};         // ok
+  [args..., &] {};         // expected-error {{capture default must be first}}
+  [=, &args...] {};        // ok
+  [&, ... xs = &args] {};  // ok
+  [&, ... xs = &] {};      // expected-error {{expected expression}}
+  [... xs = &] {};         // expected-error {{expected expression}}
+  [... xs = &args, = ] {}; // expected-error {{capture default must be first}}
+  [... xs = &args, &] {};  // expected-error {{capture default must be first}}
+}
+} // namespace misplaced_capture_default_pack