[PATCH] D16216: Fix infinite loop when ::new or ::delete are found in member initializer list

[Denis Zobnin via cfe-commits]

d.zobnin.bugzilla updated this revision to Diff 45295.
d.zobnin.bugzilla added a comment.

Thank you for the review! Updated the patch: parser now skips the rest of the initializers if the previous one was invalid, added several checks to the test.


http://reviews.llvm.org/D16216

Files:
  lib/Parse/ParseDeclCXX.cpp
  test/Parser/cxx-invalid-function-decl.cpp

Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3187,28 +3187,30 @@
       Actions.CodeCompleteConstructorInitializer(ConstructorDecl,
                                                  MemInitializers);
       return cutOffParsing();
-    } else {
-      MemInitResult MemInit = ParseMemInitializer(ConstructorDecl);
-      if (!MemInit.isInvalid())
-        MemInitializers.push_back(MemInit.get());
-      else
-        AnyErrors = true;
     }
-    
+
+    MemInitResult MemInit = ParseMemInitializer(ConstructorDecl);
+    if (!MemInit.isInvalid())
+      MemInitializers.push_back(MemInit.get());
+    else
+      AnyErrors = true;
+
     if (Tok.is(tok::comma))
       ConsumeToken();
     else if (Tok.is(tok::l_brace))
       break;
-    // If the next token looks like a base or member initializer, assume that
-    // we're just missing a comma.
-    else if (Tok.isOneOf(tok::identifier, tok::coloncolon)) {
+    // If the previous initializer was valid and the next token looks like a
+    // base or member initializer, assume that we're just missing a comma.
+    else if (!MemInit.isInvalid() &&
+             Tok.isOneOf(tok::identifier, tok::coloncolon)) {
       SourceLocation Loc = PP.getLocForEndOfToken(PrevTokLocation);
       Diag(Loc, diag::err_ctor_init_missing_comma)
         << FixItHint::CreateInsertion(Loc, ", ");
     } else {
       // Skip over garbage, until we get to '{'.  Don't eat the '{'.
-      Diag(Tok.getLocation(), diag::err_expected_either) << tok::l_brace
-                                                         << tok::comma;
+      if (!MemInit.isInvalid())
+        Diag(Tok.getLocation(), diag::err_expected_either) << tok::l_brace
+                                                           << tok::comma;
       SkipUntil(tok::l_brace, StopAtSemi | StopBeforeMatch);
       break;
     }
Index: test/Parser/cxx-invalid-function-decl.cpp
===================================================================
--- test/Parser/cxx-invalid-function-decl.cpp
+++ test/Parser/cxx-invalid-function-decl.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Check that "::new" and "::delete" in member initializer list are diagnosed
+// correctly and don't lead to infinite loop on parsing.
+
+// Error: X() (initializer on non-constructor), "::new" is skipped.
+void f1() : X() ::new{}; // expected-error{{only constructors take base initializers}}
+
+// Errors: first "::delete" and initializer on non-constructor, others skipped.
+void f2() : ::delete, ::new, X() ::new ::delete{} // expected-error{{expected class member or base class name}}
+                                                  // expected-error at -1{{only constructors take base initializers}}
+
+// Errors: the '::' token, "::delete" and initializer on non-constructor, others skipped.
+void f3() : ::, ::delete X(), ::new {}; // expected-error2{{expected class member or base class name}}
+                                        // expected-error at -1{{only constructors take base initializers}}
+
+template <class T>
+struct Base1 {
+  T x1;
+  Base1(T a1) : x1(a1) {}
+};
+
+template <class T>
+struct Base2 {
+  T x2;
+  Base2(T a2) : x2(a2) {}
+};
+
+struct S : public Base1<int>, public Base2<float> {
+  int x;
+
+  // 1-st initializer is correct (just missing ','), 2-nd incorrect, skip other.
+  S() : ::Base1<int>(0) ::new, ::Base2<float>(1.0) ::delete x(2) {} // expected-error{{expected class member or base class name}}
+                                                                    // expected-error at -1{{missing ',' between base or member initializers}}
+
+  // 1-st and 2-nd are correct, errors: '::' and "::new", others skipped.
+  S(int a) : Base1<int>(a), ::Base2<float>(1.0), ::, // expected-error{{expected class member or base class name}}
+             ::new, ! ::delete, ::Base2<() x(3) {}   // expected-error{{expected class member or base class name}}
+
+  // All initializers are correct, nothing to skip, diagnose 2 missing commas.
+  S(const S &) : Base1<int>(0) ::Base2<float>(1.0) x(2) {} // expected-error2{{missing ',' between base or member initializers}}
+};


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16216.45295.patch
Type: text/x-patch
Size: 4250 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160119/6319535c/attachment.bin>