Skip to content

Commit

Permalink
Make simple requirements starting with requires ill-formed in in requ…
Browse files Browse the repository at this point in the history
…irement body

This patch implements P2092

Simple requirements in requirement body shall not start with requires.
A warning was already in place so we just turn this warning into an error.

In addition, we add tests to make sure typename is optional in
requirement-parameter-list as per the same paper.
  • Loading branch information
cor3ntin authored and AaronBallman committed Aug 3, 2021
1 parent 43ff058 commit 977bdf6
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 8 deletions.
6 changes: 3 additions & 3 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -806,10 +806,10 @@ def err_requires_expr_expected_type_constraint : Error<
def err_requires_expr_simple_requirement_noexcept : Error<
"'noexcept' can only be used in a compound requirement (with '{' '}' around "
"the expression)">;
def warn_requires_expr_in_simple_requirement : Warning<
"this requires expression will only be checked for syntactic validity; did "
def err_requires_expr_in_simple_requirement : Error<
"requires expression in requirement body; did "
"you intend to place it in a nested requirement? (add another 'requires' "
"before the expression)">, InGroup<DiagGroup<"requires-expression">>;
"before the expression)">;

def err_missing_dependent_template_keyword : Error<
"use 'template' keyword to treat '%0' as a dependent template name">;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3602,7 +3602,7 @@ ExprResult Parser::ParseRequiresExpression() {
break;
}
if (!Expression.isInvalid() && PossibleRequiresExprInSimpleRequirement)
Diag(StartLoc, diag::warn_requires_expr_in_simple_requirement)
Diag(StartLoc, diag::err_requires_expr_in_simple_requirement)
<< FixItHint::CreateInsertion(StartLoc, "requires");
if (auto *Req = Actions.ActOnSimpleRequirement(Expression.get()))
Requirements.push_back(Req);
Expand Down
6 changes: 3 additions & 3 deletions clang/test/Parser/cxx2a-concepts-requires-expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,13 @@ void r37(bool b) requires requires { 1 } {}
// expected-error@-1 {{expected ';' at end of requirement}}

bool r38 = requires { requires { 1; }; };
// expected-warning@-1 {{this requires expression will only be checked for syntactic validity; did you intend to place it in a nested requirement? (add another 'requires' before the expression)}}
// expected-error@-1 {{requires expression in requirement body; did you intend to place it in a nested requirement? (add another 'requires' before the expression)}}

bool r39 = requires { requires () { 1; }; };
// expected-warning@-1 {{this requires expression will only be checked for syntactic validity; did you intend to place it in a nested requirement? (add another 'requires' before the expression)}}
// expected-error@-1 {{requires expression in requirement body; did you intend to place it in a nested requirement? (add another 'requires' before the expression)}}

bool r40 = requires { requires (int i) { i; }; };
// expected-warning@-1 {{this requires expression will only be checked for syntactic validity; did you intend to place it in a nested requirement? (add another 'requires' before the expression)}}
// expected-error@-1 {{requires expression in requirement body; did you intend to place it in a nested requirement? (add another 'requires' before the expression)}}

bool r41 = requires { requires (); };
// expected-error@-1 {{expected expression}}
4 changes: 3 additions & 1 deletion clang/www/cxx_status.html
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ <h2 id="cxx20">C++20 implementation status</h2>
</tr>
<tr> <!-- from Belfast -->
<td><a href="https://wg21.link/p1972r0">P1972R0</a></td>
<td rowspan="5" class="none" align="center">No</td>
<td rowspan="3" class="none" align="center">No</td>
</tr>
<tr>
<td><a href="https://wg21.link/p1980r0">P1980R0</a></td>
Expand All @@ -944,9 +944,11 @@ <h2 id="cxx20">C++20 implementation status</h2>
</tr>
<tr>
<td><a href="https://wg21.link/p2092r0">P2092R0</a></td>
<td rowspan="1" class="partial" align="center">Clang 13</td>

This comment has been minimized.

Copy link
@tambry

tambry Aug 3, 2021

Contributor

Will this be backported? main is already Clang 14.

This comment has been minimized.

Copy link
@AaronBallman

AaronBallman Aug 3, 2021

Collaborator

Good catch -- I don't plan to backport this to 13. I'll correct the documentation.

</tr>
<tr>
<td><a href="https://wg21.link/p2113r0">P2113R0</a></td>
<td rowspan="1" class="none" align="center">No</td>
</tr>
<!-- Albuquerque papers -->
<tr>
Expand Down

0 comments on commit 977bdf6

Please sign in to comment.