[clang] 22e95e0 - [clang] Fix assertion while parsing an invalid for loop

[Corentin Jabot via cfe-commits]

Author: Corentin Jabot
Date: 2023-06-07T10:31:11+02:00
New Revision: 22e95e0bf375660dd3d083b16a5d92c33559a4a3

URL: https://github.com/llvm/llvm-project/commit/22e95e0bf375660dd3d083b16a5d92c33559a4a3
DIFF: https://github.com/llvm/llvm-project/commit/22e95e0bf375660dd3d083b16a5d92c33559a4a3.diff

LOG: [clang] Fix assertion while parsing an invalid for loop

with multiple declarations followed by a colon.

Fixes #63010

Reviewed By: shafik

Differential Revision: https://reviews.llvm.org/D152009

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Parse/ParseExprCXX.cpp
    clang/lib/Parse/ParseStmt.cpp
    clang/test/Parser/cxx0x-for-range.cpp
    clang/test/Parser/objc-foreach-syntax.m

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d6b498ced6ca2..7ec9b3911ad5d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -267,7 +267,7 @@ Attribute Changes in Clang
   the compilation of the foreign language sources (e.g. Swift).
 - The ``__has_attribute``, ``__has_c_attribute`` and ``__has_cpp_attribute``
   preprocessor operators now return 1 also for attributes defined by plugins.
-- Improve the AST fidelity of ``alignas`` and ``_Alignas`` attribute. Before, we 
+- Improve the AST fidelity of ``alignas`` and ``_Alignas`` attribute. Before, we
   model ``alignas(type-id)`` as though the user wrote ``alignas(alignof(type-id))``,
   now we directly use ``alignas(type-id)``.
 
@@ -321,7 +321,7 @@ Improvements to Clang's diagnostics
   (`#62850: <https://github.com/llvm/llvm-project/issues/62850>`_).
 - Clang now warns when any predefined macro is undefined or redefined, instead
   of only some of them.
-- Clang now correctly diagnoses when the argument to ``alignas`` or ``_Alignas`` 
+- Clang now correctly diagnoses when the argument to ``alignas`` or ``_Alignas``
   is an incomplete type.
   (`#55175: <https://github.com/llvm/llvm-project/issues/55175>`_, and fixes an
   incorrect mention of ``alignof`` in a diagnostic about ``alignas``).
@@ -478,6 +478,9 @@ Bug Fixes in This Version
   (`#63008 <https://github.com/llvm/llvm-project/issues/63008>`_).
 - Reject increment of bool value in unevaluated contexts after C++17.
   (`#47517 <https://github.com/llvm/llvm-project/issues/47517>`_).
+- Fix assertion and quality of diagnostic messages in a for loop
+  containing multiple declarations and a range specifier
+  (`#63010 <https://github.com/llvm/llvm-project/issues/63010>`_).
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 037dc923c47eb..981345a7d55ca 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -2140,8 +2140,6 @@ Parser::ParseCXXCondition(StmtResult *InitStmt, SourceLocation Loc,
     DeclGroupPtrTy DG = ParseSimpleDeclaration(
         DeclaratorContext::ForInit, DeclEnd, attrs, DeclSpecAttrs, false, FRI);
     FRI->LoopVar = Actions.ActOnDeclStmt(DG, DeclStart, Tok.getLocation());
-    assert((FRI->ColonLoc.isValid() || !DG) &&
-           "cannot find for range declaration");
     return Sema::ConditionResult();
   }
 

diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index aea810e8cf45c..224df67b47f8f 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -2202,9 +2202,7 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
     if (Tok.isNot(tok::semi)) {
       if (!SecondPart.isInvalid())
         Diag(Tok, diag::err_expected_semi_for);
-      else
-        // Skip until semicolon or rparen, don't consume it.
-        SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
+      SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
     }
 
     if (Tok.is(tok::semi)) {

diff  --git a/clang/test/Parser/cxx0x-for-range.cpp b/clang/test/Parser/cxx0x-for-range.cpp
index c3276ebeaabbc..a265c4764af56 100644
--- a/clang/test/Parser/cxx0x-for-range.cpp
+++ b/clang/test/Parser/cxx0x-for-range.cpp
@@ -60,3 +60,22 @@ void f() {
   }
 }
 }
+
+namespace GH63010 {
+void foo(int n) {
+    int a[] = {1, 2, 3, 4, 5};
+    {
+        for (auto x = n ? 1 : 2 : a); // expected-error {{expected ';' in 'for' statement specifier}} \
+                                    // expected-error {{expected expression}}
+        for (int i = 1; auto x = n ? 1 : 2 : a); // expected-error {{expected ';' in 'for' statement specifier}}
+    }
+    {
+        for (auto x = n ? 1 : 2 : a)  // expected-error {{expected ';' in 'for' statement specifier}} \
+                                      // expected-error {{expected expression}}
+
+    }  // expected-error {{expected statement}}
+    {
+        for (int i = 1; auto x = n ? 1 : 2 : a) // expected-error {{expected ';' in 'for' statement specifier}}
+    } // expected-error {{expected statement}}
+}
+}

diff  --git a/clang/test/Parser/objc-foreach-syntax.m b/clang/test/Parser/objc-foreach-syntax.m
index e446eb576c1aa..2158d8062f6cd 100644
--- a/clang/test/Parser/objc-foreach-syntax.m
+++ b/clang/test/Parser/objc-foreach-syntax.m
@@ -21,8 +21,6 @@ - (void)compilerTestAgainst {
 
 
 static int test7(id keys) {
-  // FIXME: would be nice to suppress the secondary diagnostics.
   for (id key; in keys) ;  // expected-error {{use of undeclared identifier 'in'}} \
-                           // expected-error {{expected ';' in 'for' statement specifier}} \
-                           // expected-warning {{expression result unused}}
+                           // expected-error {{expected ';' in 'for' statement specifier}}
 }