[clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no parameter list

[curdeius]

Summary

This fixes http://llvm.org/PR49736 caused by implementing http://wg21.link/P1102 (https://reviews.llvm.org/rG0620e6f4b76a9725dbd82454d58c5a68a7e47074), by correctly allowing requires-clause only:

1. directly after template-parameter-list
2. after lambda-specifiers iff parameter-declaration-clause is present (2nd kind of lambda-declarator)

[aaron.ballman]

clang/lib/Parse/ParseExprCXX.cpp

1435

Rather than add this in both branches, I'd say to hoist it out of the branches and put it after the else if below (before the declaration of ScopeFlags).

[curdeius]

clang/lib/Parse/ParseExprCXX.cpp

1435

True. Will do soon.

[curdeius]

• Hoist common code.

[aaron.ballman]

Mostly looks good, just a few things with the tests.

clang/test/Parser/cxx-concepts-requires-clause.cpp

160–162

This warning seems less than ideal (same issue happens below).

clang/test/Parser/cxx2a-template-lambdas.cpp

15

You should spell out the full diagnostic the first time it's used in a file.

17

This seems grammatically valid to me, was there a reason for the // ??

33–36

It seems odd to warn the user about use of an extension they're not really using, but I don't think this is strictly wrong as opposed to just not being ideal. I don't think this will be trivial to improve the behavior, so I think it's fine for the moment.

[rsmith]

Only a nit, please go ahead once Aaron is happy :)

clang/test/Parser/cxx2a-template-lambdas.cpp

25–29

I'd find these examples easier to read with a space between true and ().

[curdeius]

• Clean up tests.
• Fix grammar comment.

clang/test/Parser/cxx2a-template-lambdas.cpp

17

Ooops, a WIP remnant.

25–29

I've just clang-formatted the code. Isn't it the way to go?
I can format manually if desired though.

33–36

I agree that having an error and a warning is strange, but IMO the extension is used here.
Without the extension, you couldn't have noexcept without ().

[curdeius]

• Rebase to fix CI.

[aaron.ballman]

LGTM aside from the test case commenting nit. Thank you!

clang/test/Parser/cxx2a-template-lambdas.cpp

17

Please spell out the full diagnostic wording in this instance.

clang/lib/Parse/ParseExprCXX.cpp

ExprResult Parser::ParseCXXIdExpression(bool isAddressOfOperand) {
   return Result;
 }
 
 /// ParseLambdaExpression - Parse a C++11 lambda expression.
 ///
 ///       lambda-expression:
 ///         lambda-introducer lambda-declarator compound-statement
 ///         lambda-introducer '<' template-parameter-list '>'
-///             lambda-declarator compound-statement
+///             requires-clause[opt] lambda-declarator compound-statement
 ///
 ///       lambda-introducer:
 ///         '[' lambda-capture[opt] ']'
 ///
 ///       lambda-capture:
 ///         capture-default
 ///         capture-list
 ///         capture-default ',' capture-list
auto ParseLambdaSpecifiers =
                 /*RefQualifierLoc=*/NoLoc, MutableLoc, ESpecType, ESpecRange,
                 DynamicExceptions.data(), DynamicExceptionRanges.data(),
                 DynamicExceptions.size(),
                 NoexceptExpr.isUsable() ? NoexceptExpr.get() : nullptr,
                 /*ExceptionSpecTokens*/ nullptr,
                 /*DeclsInPrototype=*/None, LParenLoc, FunLocalRangeEnd, D,
                 TrailingReturnType, TrailingReturnTypeLoc, &DS),
             std::move(Attr), DeclEndLoc);
-
-        // Parse requires-clause[opt].
-        if (Tok.is(tok::kw_requires))
-          ParseTrailingRequiresClause(D);
-
-        WarnIfHasCUDATargetAttr();
       };
 
   if (Tok.is(tok::l_paren)) {
     ParseScope PrototypeScope(this, Scope::FunctionPrototypeScope |
                                         Scope::FunctionDeclarationScope |
                                         Scope::DeclScope);
 
     BalancedDelimiterTracker T(*this, tok::l_paren);
if (Tok.isNot(tok::r_paren)) {
         CurTemplateDepthTracker.setAddedDepth(1);
     }
 
     T.consumeClose();
 
     // Parse lambda-specifiers.
     ParseLambdaSpecifiers(LParenLoc, /*DeclEndLoc=*/T.getCloseLocation(),
                           ParamInfo, EllipsisLoc);
+
+    // Parse requires-clause[opt].
+    if (Tok.is(tok::kw_requires))
+      ParseTrailingRequiresClause(D);
   } else if (Tok.isOneOf(tok::kw_mutable, tok::arrow, tok::kw___attribute,
                          tok::kw_constexpr, tok::kw_consteval,
                          tok::kw___private, tok::kw___global, tok::kw___local,
                          tok::kw___constant, tok::kw___generic,
                          tok::kw_requires, tok::kw_noexcept) ||
              (Tok.is(tok::l_square) && NextToken().is(tok::l_square))) {
     if (!getLangOpts().CPlusPlus2b)
       // It's common to forget that one needs '()' before 'mutable', an
       // attribute specifier, the result type, or the requires clause. Deal with
       // this.
       Diag(Tok, diag::ext_lambda_missing_parens)
           << FixItHint::CreateInsertion(Tok.getLocation(), "() ");
 
     SourceLocation NoLoc;
     // Parse lambda-specifiers.
     std::vector<DeclaratorChunk::ParamInfo> EmptyParamInfo;
     ParseLambdaSpecifiers(/*LParenLoc=*/NoLoc, /*RParenLoc=*/NoLoc,
                           EmptyParamInfo, /*EllipsisLoc=*/NoLoc);
   }
 
+  WarnIfHasCUDATargetAttr();
+
   // FIXME: Rename BlockScope -> ClosureScope if we decide to continue using
   // it.
   unsigned ScopeFlags = Scope::BlockScope | Scope::FnScope | Scope::DeclScope |
                         Scope::CompoundStmtScope;
   ParseScope BodyScope(this, ScopeFlags);
 
   Actions.ActOnStartOfLambdaDefinition(Intro, D, getCurScope());
 

clang/test/Parser/cxx-concepts-requires-clause.cpp

struct B {
   static void baz(int y) requires (this, true);
   // expected-error@-1{{'this' cannot be used in a static member function declaration}}
 };
 
 auto lambda1 = [] (auto x) requires (sizeof(decltype(x)) == 1) { };
 
 auto lambda2 = [] (auto x) constexpr -> int requires (sizeof(decltype(x)) == 1) { return 0; };
 
-auto lambda3 = [] requires (sizeof(char) == 1) { };
+auto lambda3 = []<auto> requires(sizeof(char) == 1){};
+
+auto lambda4 = [] requires(sizeof(char) == 1){}; // expected-error {{expected body of lambda expression}}
 #if __cplusplus <= 202002L
-// expected-warning@-2{{is a C++2b extension}}
+// expected-warning@-2{{lambda without a parameter clause is a C++2b extension}}
 #endif

clang/test/Parser/cxx2a-template-lambdas.cpp

 // RUN: %clang_cc1 -std=c++2b %s -verify
 // RUN: %clang_cc1 -std=c++2a %s -verify
 
 auto L0 = []<> { }; //expected-error {{cannot be empty}}
 
 auto L1 = []<typename T1, typename T2> { };
 auto L2 = []<typename T1, typename T2>(T1 arg1, T2 arg2) -> T1 { };
 auto L3 = []<typename T>(auto arg) { T t; };
 auto L4 = []<int I>() { };
+
+// http://llvm.org/PR49736
+auto L5 = []<auto>(){};
+auto L6 = []<auto>{};
+auto L7 = []<auto>() noexcept {};
+auto L8 = []<auto> noexcept {};
+#if __cplusplus <= 202002L
+// expected-warning@-2 {{is a C++2b extension}}
+#endif
+auto L9 = []<auto> requires true {};
+auto L10 = []<auto> requires true(){};
+auto L11 = []<auto> requires true() noexcept {};
+auto L12 = []<auto> requires true noexcept {};
+#if __cplusplus <= 202002L
+// expected-warning@-2 {{is a C++2b extension}}
+#endif
+auto L13 = []<auto>() noexcept requires true {};
+auto L14 = []<auto> requires true() noexcept requires true {};
+
+auto XL0 = []<auto> noexcept requires true {};               // expected-error {{expected body of lambda expression}}
+auto XL1 = []<auto> requires true noexcept requires true {}; // expected-error {{expected body}}
+#if __cplusplus <= 202002L
+// expected-warning@-3 {{lambda without a parameter clause is a C++2b extension}}
+// expected-warning@-3 {{is a C++2b extension}}
+#endif

clang/test/Parser/cxx2b-lambdas.cpp

 auto L5 = [] constexpr mutable noexcept {};
 auto L6 = [s = 1] mutable {};
 auto L7 = [s = 1] constexpr mutable noexcept {};
 auto L8 = [] -> bool { return true; };
 auto L9 = []<typename T> { return true; };
 auto L10 = []<typename T> noexcept { return true; };
 auto L11 = []<typename T> -> bool { return true; };
 auto L12 = [] consteval {};
-auto L13 = [] requires requires() { true; }
-{};
-auto L15 = [] [[maybe_unused]]{};
+auto L13 = []() requires true {};
+auto L14 = []<auto> requires true() requires true {};
+auto L15 = []<auto> requires true noexcept {};
+auto L16 = [] [[maybe_unused]]{};
 
 auto XL0 = [] mutable constexpr mutable {};    // expected-error{{cannot appear multiple times}}
 auto XL1 = [] constexpr mutable constexpr {};  // expected-error{{cannot appear multiple times}}
-auto XL2 = []) constexpr mutable constexpr {}; // expected-error{{expected body}}
+auto XL2 = []) constexpr mutable constexpr {}; // expected-error{{expected body of lambda expression}}
 auto XL3 = []( constexpr mutable constexpr {}; // expected-error{{invalid storage class specifier}} \
                                                // expected-error{{function parameter cannot be constexpr}} \
                                                // expected-error{{C++ requires}} \
                                                // expected-error{{expected ')'}} \
                                                // expected-note{{to match this '('}} \
                                                // expected-error{{expected body}} \
                                                // expected-warning{{duplicate 'constexpr'}}
+
+// http://llvm.org/PR49736
+auto XL4 = [] requires true {}; // expected-error{{expected body}}
+auto XL5 = []<auto> requires true requires true {}; // expected-error{{expected body}}
+auto XL6 = []<auto> requires true noexcept requires true {}; // expected-error{{expected body}}