[PATCH] Fix to PR8880 (clang dies processing a for loop).

[Richard Smith]

@@ -1291,6 +1289,7 @@
   // See comments in ParseIfStatement for why we create a scope for the
   // condition and a new scope for substatement in C++.
   //
+  getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
   ParseScope InnerScope(this, Scope::DeclScope,
                         C99orCXX && Tok.isNot(tok::l_brace));

@@ -1342,6 +1341,7 @@

   // Pop the body scope if needed.
   InnerScope.Exit();
+  getCurScope()->RemoveFlags(Scope::BreakScope | Scope::ContinueScope);

These changes look redundant: you're adding and removing the flags to the
outer scope for exactly the period where the outer scope is not active.


+  assert ((Flags & ControlScope) != 0 || "Must be control scope");

No space after 'assert'.


+void Scope::RemoveFlags(unsigned FlagsToClear) {
+  assert((FlagsToClear & ~(BreakScope | ContinueScope)) == 0 ||
+         "Unsupported scope flags");
+  if (FlagsToClear & BreakScope) {
+    assert((Flags & ControlScope) != 0 || "Must be control scope");
+    assert((Flags & BreakScope) != 0 || "Already cleared");
+    BreakParent = 0;
+ }
+  if (FlagsToClear & ContinueScope) {
+    assert ((Flags & ControlScope) != 0 || "Must be control scope");
+    assert((Flags & ContinueScope) != 0 || "Already cleared");
+    ContinueParent = 0;
+  }
+  Flags &= ~FlagsToClear;
+}

These are incorrect: the BreakParent and ContinueParent should be inherited
from the parent here, rather than set to zero. However, with the first
change above, RemoveFlags will be unused and can just be deleted.

You're missing test cases for 'break' and 'continue' in the condition of a
do-while, which would have caught the above bug.


On Mon, Dec 9, 2013 at 4:06 AM, Serge Pavlov <sepavloff at gmail.com> wrote:

> If there is no objections, I'll commit this fix.
>

See http://llvm.org/docs/DeveloperPolicy.html#code-reviews bullet 5: "Do
not assume silent approval".
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131209/e3b4e044/attachment.html>