Revamps structural error detection / handling.

Previously we'd only detect structural errors on the very first level.
This leads to incorrectly balanced braces not being discovered, and thus
incorrect indentation.

This change fixes the problem by:
- changing the parser to use an error state that can be detected
  anywhere inside the productions, for example if we get an eof on
  SOME_MACRO({ some block <eof>
- previously we'd never break lines when we discovered a structural
  error; now we break even in the case of a structural error if there
  are two unwrapped lines within the same line; thus,
  void f() { while (true) { g(); y(); } }
  will still be re-formatted, even if there's missing braces somewhere
  in the file
- still exclude macro definitions from generating structural error;
  macro definitions are inbalanced snippets

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179379 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 44 insertions, 40 deletions

diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index e956f69875..d0dfdceddc 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -459,7 +459,7 @@ public:
   UnwrappedLineFormatter(const FormatStyle &Style, SourceManager &SourceMgr,
                          const AnnotatedLine &Line, unsigned FirstIndent,
                          const AnnotatedToken &RootToken,
-                         WhitespaceManager &Whitespaces, bool StructuralError)
+                         WhitespaceManager &Whitespaces)
       : Style(Style), SourceMgr(SourceMgr), Line(Line),
         FirstIndent(FirstIndent), RootToken(RootToken),
         Whitespaces(Whitespaces), Count(0) {}
@@ -1381,7 +1381,7 @@ public:
   tooling::Replacements format() {
     LexerBasedFormatTokenSource Tokens(Lex, SourceMgr);
     UnwrappedLineParser Parser(Diag, Style, Tokens, *this);
-    StructuralError = Parser.parse();
+    bool StructuralError = Parser.parse();
     unsigned PreviousEndOfLineColumn = 0;
     TokenAnnotator Annotator(Style, SourceMgr, Lex,
                              Tokens.getIdentTable().get("in"));
@@ -1431,17 +1431,19 @@ public:
         unsigned Indent = LevelIndent;
         if (static_cast<int>(Indent) + Offset >= 0)
           Indent += Offset;
-        if (!FirstTok.WhiteSpaceStart.isValid() || StructuralError) {
-          Indent = LevelIndent =
-              SourceMgr.getSpellingColumnNumber(FirstTok.Tok.getLocation()) - 1;
-        } else {
+        if (FirstTok.WhiteSpaceStart.isValid() &&
+            // Insert a break even if there is a structural error in case where
+            // we break apart a line consisting of multiple unwrapped lines.
+            (FirstTok.NewlinesBefore == 0 || !StructuralError)) {
           formatFirstToken(TheLine.First, PreviousLineLastToken, Indent,
                            TheLine.InPPDirective, PreviousEndOfLineColumn);
+        } else {
+          Indent = LevelIndent =
+              SourceMgr.getSpellingColumnNumber(FirstTok.Tok.getLocation()) - 1;
         }
         tryFitMultipleLinesInOne(Indent, I, E);
         UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine, Indent,
-                                         TheLine.First, Whitespaces,
-                                         StructuralError);
+                                         TheLine.First, Whitespaces);
         PreviousEndOfLineColumn =
             Formatter.format(I + 1 != E ? &*(I + 1) : NULL);
         IndentForLevel[TheLine.Level] = LevelIndent;
@@ -1742,7 +1744,6 @@ private:
   WhitespaceManager Whitespaces;
   std::vector<CharSourceRange> Ranges;
   std::vector<AnnotatedLine> AnnotatedLines;
-  bool StructuralError;
 };
 
 tooling::Replacements
diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp
index cf2ef718e9..722af5d2b7 100644
--- a/lib/Format/UnwrappedLineParser.cpp
+++ b/lib/Format/UnwrappedLineParser.cpp
@@ -45,9 +45,11 @@ private:
 class ScopedMacroState : public FormatTokenSource {
 public:
   ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
-                   FormatToken &ResetToken)
+                   FormatToken &ResetToken, bool &StructuralError)
       : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
-        PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource) {
+        PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
+        StructuralError(StructuralError),
+        PreviousStructuralError(StructuralError) {
     TokenSource = this;
     Line.Level = 0;
     Line.InPPDirective = true;
@@ -58,6 +60,7 @@ public:
     ResetToken = Token;
     Line.InPPDirective = false;
     Line.Level = PreviousLineLevel;
+    StructuralError = PreviousStructuralError;
   }
 
   virtual FormatToken getNextToken() {
@@ -85,6 +88,8 @@ private:
   FormatToken &ResetToken;
   unsigned PreviousLineLevel;
   FormatTokenSource *PreviousTokenSource;
+  bool &StructuralError;
+  bool PreviousStructuralError;
 
   FormatToken Token;
 };
@@ -124,13 +129,13 @@ UnwrappedLineParser::UnwrappedLineParser(
     clang::DiagnosticsEngine &Diag, const FormatStyle &Style,
     FormatTokenSource &Tokens, UnwrappedLineConsumer &Callback)
     : Line(new UnwrappedLine), MustBreakBeforeNextToken(false),
-      CurrentLines(&Lines), Diag(Diag), Style(Style), Tokens(&Tokens),
-      Callback(Callback) {}
+      CurrentLines(&Lines), StructuralError(false), Diag(Diag), Style(Style),
+      Tokens(&Tokens), Callback(Callback) {}
 
 bool UnwrappedLineParser::parse() {
   DEBUG(llvm::dbgs() << "----\n");
   readToken();
-  bool Error = parseFile();
+  parseFile();
   for (std::vector<UnwrappedLine>::iterator I = Lines.begin(), E = Lines.end();
        I != E; ++I) {
     Callback.consumeUnwrappedLine(*I);
@@ -139,23 +144,20 @@ bool UnwrappedLineParser::parse() {
   // Create line with eof token.
   pushToken(FormatTok);
   Callback.consumeUnwrappedLine(*Line);
-
-  return Error;
+  return StructuralError;
 }
 
-bool UnwrappedLineParser::parseFile() {
+void UnwrappedLineParser::parseFile() {
   ScopedDeclarationState DeclarationState(
       *Line, DeclarationScopeStack,
       /*MustBeDeclaration=*/ !Line->InPPDirective);
-  bool Error = parseLevel(/*HasOpeningBrace=*/ false);
+  parseLevel(/*HasOpeningBrace=*/ false);
   // Make sure to format the remaining tokens.
   flushComments(true);
   addUnwrappedLine();
-  return Error;
 }
 
-bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
-  bool Error = false;
+void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
   do {
     switch (FormatTok.Tok.getKind()) {
     case tok::comment:
@@ -165,30 +167,27 @@ bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
     case tok::l_brace:
       // FIXME: Add parameter whether this can happen - if this happens, we must
       // be in a non-declaration context.
-      Error |= parseBlock(/*MustBeDeclaration=*/ false);
+      parseBlock(/*MustBeDeclaration=*/ false);
       addUnwrappedLine();
       break;
     case tok::r_brace:
-      if (HasOpeningBrace) {
-        return false;
-      } else {
-        Diag.Report(FormatTok.Tok.getLocation(),
-                    Diag.getCustomDiagID(clang::DiagnosticsEngine::Error,
-                                         "unexpected '}'"));
-        Error = true;
-        nextToken();
-        addUnwrappedLine();
-      }
+      if (HasOpeningBrace)
+        return;
+      Diag.Report(FormatTok.Tok.getLocation(),
+                  Diag.getCustomDiagID(clang::DiagnosticsEngine::Error,
+                                       "unexpected '}'"));
+      StructuralError = true;
+      nextToken();
+      addUnwrappedLine();
       break;
     default:
       parseStructuralElement();
       break;
     }
   } while (!eof());
-  return Error;
 }
 
-bool UnwrappedLineParser::parseBlock(bool MustBeDeclaration,
+void UnwrappedLineParser::parseBlock(bool MustBeDeclaration,
                                      unsigned AddLevels) {
   assert(FormatTok.Tok.is(tok::l_brace) && "'{' expected");
   nextToken();
@@ -202,17 +201,17 @@ bool UnwrappedLineParser::parseBlock(bool MustBeDeclaration,
 
   if (!FormatTok.Tok.is(tok::r_brace)) {
     Line->Level -= AddLevels;
-    return true;
+    StructuralError = true;
+    return;
   }
 
   nextToken(); // Munch the closing brace.
   Line->Level -= AddLevels;
-  return false;
 }
 
 void UnwrappedLineParser::parsePPDirective() {
   assert(FormatTok.Tok.is(tok::hash) && "'#' expected");
-  ScopedMacroState MacroState(*Line, Tokens, FormatTok);
+  ScopedMacroState MacroState(*Line, Tokens, FormatTok, StructuralError);
   nextToken();
 
   if (FormatTok.Tok.getIdentifierInfo() == NULL) {
diff --git a/lib/Format/UnwrappedLineParser.h b/lib/Format/UnwrappedLineParser.h
index f4fecc5ef0..1326d500e2 100644
--- a/lib/Format/UnwrappedLineParser.h
+++ b/lib/Format/UnwrappedLineParser.h
@@ -125,9 +125,9 @@ public:
   bool parse();
 
 private:
-  bool parseFile();
-  bool parseLevel(bool HasOpeningBrace);
-  bool parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1);
+  void parseFile();
+  void parseLevel(bool HasOpeningBrace);
+  void parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1);
   void parsePPDirective();
   void parsePPDefine();
   void parsePPUnknown();
@@ -187,6 +187,10 @@ private:
   // whether we are in a compound statement or not.
   std::vector<bool> DeclarationScopeStack;
 
+  // Will be true if we encounter an error that leads to possibily incorrect
+  // indentation levels.
+  bool StructuralError;
+
   clang::DiagnosticsEngine &Diag;
   const FormatStyle &Style;
   FormatTokenSource *Tokens;