Fix crasher bug.

Due to not resetting the fake rparen data on the token when iterating
over annotated lines, we would pop the last element of the paren stack.

This patch fixes the underlying root cause, and makes the code more
robust against similar problems in the future:
- reset the first token when iterating on the same annotated lines due
  to preprocessor branches
- never pop the last element from the paren stack, so we do not crash,
  but rather incorrectly format
- add assert()s so we can figure out if our assumptions are violated

llvm-svn: 204140

3 files changed, 46 insertions, 16 deletions

diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 380532a47cc..cb4e4f53b2f 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -217,8 +217,8 @@ unsigned ContinuationIndenter::addTokenToState(LineState &State, bool Newline,
                                                unsigned ExtraSpaces) {
   const FormatToken &Current = *State.NextToken;
 
-  if (State.Stack.size() == 0 ||
-      (Current.Type == TT_ImplicitStringLiteral &&
+  assert(!State.Stack.empty());
+  if ((Current.Type == TT_ImplicitStringLiteral &&
        (Current.Previous->Tok.getIdentifierInfo() == NULL ||
         Current.Previous->Tok.getIdentifierInfo()->getPPKeywordID() ==
             tok::pp_not_keyword))) {
@@ -628,8 +628,14 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
         //   SomeFunction(a, [] {
         //                     f();  // break
         //                   });
-        for (unsigned i = 0; i != Current.MatchingParen->FakeRParens; ++i)
+        for (unsigned i = 0; i != Current.MatchingParen->FakeRParens; ++i) {
+          assert(State.Stack.size() > 1);
+          if (State.Stack.size() == 1) {
+            // Do not pop the last element.
+            break;
+          }
           State.Stack.pop_back();
+        }
         bool IsObjCBlock =
             Previous &&
             (Previous->is(tok::caret) ||
@@ -709,6 +715,11 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
     // as they will have been removed early (see above).
     for (unsigned i = 0, e = Current.FakeRParens; i != e; ++i) {
       unsigned VariablePos = State.Stack.back().VariablePos;
+      assert(State.Stack.size() > 1);
+      if (State.Stack.size() == 1) {
+        // Do not pop the last element.
+        break;
+      }
       State.Stack.pop_back();
       State.Stack.back().VariablePos = VariablePos;
     }
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 34aef99a7b2..a91c8474521 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -34,6 +34,7 @@ public:
       : Style(Style), Line(Line), CurrentToken(Line.First),
         KeywordVirtualFound(false), AutoFound(false), Ident_in(Ident_in) {
     Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
+    resetTokenMetadata(CurrentToken);
   }
 
 private:
@@ -571,6 +572,22 @@ public:
   }
 
 private:
+  void resetTokenMetadata(FormatToken *Token) {
+    if (Token == nullptr) return;
+
+    // Reset token type in case we have already looked at it and then
+    // recovered from an error (e.g. failure to find the matching >).
+    if (CurrentToken->Type != TT_LambdaLSquare &&
+        CurrentToken->Type != TT_FunctionLBrace &&
+        CurrentToken->Type != TT_ImplicitStringLiteral &&
+        CurrentToken->Type != TT_TrailingReturnArrow)
+      CurrentToken->Type = TT_Unknown;
+    if (CurrentToken->Role)
+      CurrentToken->Role.reset(NULL);
+    CurrentToken->FakeLParens.clear();
+    CurrentToken->FakeRParens = 0;
+  }
+
   void next() {
     if (CurrentToken != NULL) {
       determineTokenType(*CurrentToken);
@@ -581,19 +598,7 @@ private:
     if (CurrentToken != NULL)
       CurrentToken = CurrentToken->Next;
 
-    if (CurrentToken != NULL) {
-      // Reset token type in case we have already looked at it and then
-      // recovered from an error (e.g. failure to find the matching >).
-      if (CurrentToken->Type != TT_LambdaLSquare &&
-          CurrentToken->Type != TT_FunctionLBrace &&
-          CurrentToken->Type != TT_ImplicitStringLiteral &&
-          CurrentToken->Type != TT_TrailingReturnArrow)
-        CurrentToken->Type = TT_Unknown;
-      if (CurrentToken->Role)
-        CurrentToken->Role.reset(NULL);
-      CurrentToken->FakeLParens.clear();
-      CurrentToken->FakeRParens = 0;
-    }
+    resetTokenMetadata(CurrentToken);
   }
 
   /// \brief A struct to hold information valid in a specific context, e.g.
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 5bd0822281e..b0324015cd6 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8173,5 +8173,19 @@ TEST_F(FormatTest, SpacesInAngles) {
   verifyFormat("A<A<int>>();", Spaces);
 }
 
+TEST_F(FormatTest, HandleUnbalancedImplicitBracesAcrossPPBranches) {
+  std::string code = "#if A\n"
+                     "#if B\n"
+                     "a.\n"
+                     "#endif\n"
+                     "    a = 1;\n"
+                     "#else\n"
+                     "#endif\n"
+                     "#if C\n"
+                     "#else\n"
+                     "#endif\n";
+  EXPECT_EQ(code, format(code));
+}
+
 } // end namespace tooling
 } // end namespace clang