[Clang][Lex] Fix parsing of nested requirement to prevent flowing off the end of token stream

[shafik]

Currently when parsing a nested requirement we attempt to balance parens if we have a parameter list. This will fail in some cases of ill-formed code and keep going until we fall off the token stream and crash. This fixes the hand parsing by using SkipUntil which will properly flag if we don't find the expected tokens.

Fixes: #73112

[llvmbot]

@llvm/pr-subscribers-clang

Author: Shafik Yaghmour (shafik)

Changes

Currently when parsing a nested requirement we attempt to balance parens if we have a parameter list. This will fail in some cases of ill-formed code and keep going until we fall off the token stream and crash. This fixes the hand parsing by using SkipUntil which will properly flag if we don't find the expected tokens.

Fixes: #73112

---

Full diff: https://github.com/llvm/llvm-project/pull/73691.diff

2 Files Affected:

• (modified) clang/lib/Parse/ParseExprCXX.cpp (+4)
• (modified) clang/test/Parser/cxx2a-concepts-requires-expr.cpp (+8)

diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 79db094e098f8e6..a0903eb3a932490 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3639,6 +3639,10 @@ ExprResult Parser::ParseRequiresExpression() {
                 //  TryParseParameterDeclarationClause).
                 unsigned Depth = 1;
                 while (Depth != 0) {
+                  bool FoundParen = SkipUntil(tok::l_paren, tok::r_paren,
+                                              SkipUntilFlags::StopBeforeMatch);
+                  if (!FoundParen)
+                    break;
                   if (Tok.is(tok::l_paren))
                     Depth++;
                   else if (Tok.is(tok::r_paren))
diff --git a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
index a18a54c7fad0690..971591afb08dba2 100644
--- a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
+++ b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
@@ -160,3 +160,11 @@ template <int N>
 requires requires {
  typename BitInt<N>; // ok
 } using r44 = void;
+
+namespace GH73112 {
+void f() {
+    requires { requires(int; } // expected-error {{expected ')'}} \
+                               // expected-error {{expected expression}} \
+                               // expected-note {{to match this '('}}
+}
+}

[shafik]

I don't think the above fixme above is needed, I don't think there is any other way to handle this, at least based on some of the other places TryParseParameterDeclarationClause() is used e.g. TryParseFunctionDeclarator(...)

[cor3ntin]

Yeah, I think we can remove that

[cor3ntin]

Missing a release note, otherwise LGTM

[cor3ntin]

Yeah, I think we can remove that