Error recovery part 2

Summary: Adds recovery for structural errors in clang-format.

Reviewers: djasper

Reviewed By: djasper

CC: cfe-commits, silvas

Differential Revision: http://llvm-reviews.chandlerc.com/D164

llvm-svn: 169286

clang/lib/Format/Format.cpp

@@ -72,28 +72,29 @@ public:
   UnwrappedLineFormatter(const FormatStyle &Style, SourceManager &SourceMgr,
                          const UnwrappedLine &Line,
                          const std::vector<TokenAnnotation> &Annotations,
-                         tooling::Replacements &Replaces)
+                         tooling::Replacements &Replaces, bool StructuralError)
       : Style(Style),
         SourceMgr(SourceMgr),
         Line(Line),
         Annotations(Annotations),
-        Replaces(Replaces) {
+        Replaces(Replaces),
+        StructuralError(StructuralError) {
     Parameters.PenaltyExtraLine = 100;
     Parameters.PenaltyIndentLevel = 5;
   }
 
   void format() {
-    formatFirstToken();
+    unsigned Indent = formatFirstToken();
     count = 0;
     IndentState State;
-    State.Column = Line.Level * 2 + Line.Tokens[0].Tok.getLength();
+    State.Column = Indent + Line.Tokens[0].Tok.getLength();
     State.CtorInitializerOnNewLine = false;
     State.InCtorInitializer = false;
     State.ConsumedTokens = 1;
 
     //State.UsedIndent.push_back(Line.Level * 2);
-    State.Indent.push_back(Line.Level * 2 + 4);
+    State.Indent.push_back(Indent + 4);
-    State.LastSpace.push_back(Line.Level * 2);
+    State.LastSpace.push_back(Indent);
 
     // Start iterating at 1 as we have correctly formatted of Token #0 above.
     for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) {
@@ -315,20 +316,22 @@ private:
 
   /// \brief Add a new line and the required indent before the first Token
   /// of the \c UnwrappedLine.
-  void formatFirstToken() {
+  unsigned formatFirstToken() {
     const FormatToken &Token = Line.Tokens[0];
-    if (Token.WhiteSpaceStart.isValid()) {
+    if (!Token.WhiteSpaceStart.isValid() || StructuralError)
-      unsigned Newlines =
+      return SourceMgr.getSpellingColumnNumber(Token.Tok.getLocation()) - 1;
-          std::min(Token.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1);
+
-      unsigned Offset = SourceMgr.getFileOffset(Token.WhiteSpaceStart);
+    unsigned Newlines =
-      if (Newlines == 0 && Offset != 0)
+        std::min(Token.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1);
-        Newlines = 1;
+    unsigned Offset = SourceMgr.getFileOffset(Token.WhiteSpaceStart);
-      unsigned Indent = Line.Level * 2;
+    if (Newlines == 0 && Offset != 0)
-      if (Token.Tok.is(tok::kw_public) || Token.Tok.is(tok::kw_protected) ||
+      Newlines = 1;
-          Token.Tok.is(tok::kw_private))
+    unsigned Indent = Line.Level * 2;
-        Indent += Style.AccessModifierOffset;
+    if (Token.Tok.is(tok::kw_public) || Token.Tok.is(tok::kw_protected) ||
-      replaceWhitespace(Token, Newlines, Indent);
+        Token.Tok.is(tok::kw_private))
-    }
+      Indent += Style.AccessModifierOffset;
+    replaceWhitespace(Token, Newlines, Indent);
+    return Indent;
   }
 
   FormatStyle Style;
@@ -337,6 +340,7 @@ private:
   const std::vector<TokenAnnotation> &Annotations;
   tooling::Replacements &Replaces;
   unsigned int count;
+  bool StructuralError;
 
   OptimizationParameters Parameters;
 };
@@ -678,17 +682,26 @@ public:
       : Style(Style),
         Lex(Lex),
         SourceMgr(SourceMgr),
-        Ranges(Ranges) {
+        Ranges(Ranges),
+        StructuralError(false) {
   }
 
   tooling::Replacements format() {
     UnwrappedLineParser Parser(Lex, SourceMgr, *this);
-    Parser.parse();
+    StructuralError = Parser.parse();
+    for (std::vector<UnwrappedLine>::iterator I = UnwrappedLines.begin(),
+                                              E = UnwrappedLines.end();
+         I != E; ++I)
+      doFormatUnwrappedLine(*I);
     return Replaces;
   }
 
 private:
   virtual void formatUnwrappedLine(const UnwrappedLine &TheLine) {
+    UnwrappedLines.push_back(TheLine);
+  }
+
+  void doFormatUnwrappedLine(const UnwrappedLine &TheLine) {
     if (TheLine.Tokens.size() == 0)
       return;
 
@@ -706,7 +719,8 @@ private:
       TokenAnnotator Annotator(TheLine, Style, SourceMgr);
       Annotator.annotate();
       UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine,
-                                       Annotator.getAnnotations(), Replaces);
+                                       Annotator.getAnnotations(), Replaces,
+                                       StructuralError);
       Formatter.format();
       return;
     }
@@ -717,6 +731,8 @@ private:
   SourceManager &SourceMgr;
   tooling::Replacements Replaces;
   std::vector<CharSourceRange> Ranges;
+  std::vector<UnwrappedLine> UnwrappedLines;
+  bool StructuralError;
 };
 
 tooling::Replacements reformat(const FormatStyle &Style, Lexer &Lex,

clang/lib/Format/UnwrappedLineParser.cpp

@@ -32,12 +32,13 @@ UnwrappedLineParser::UnwrappedLineParser(Lexer &Lex, SourceManager &SourceMgr,
   Lex.SetKeepWhitespaceMode(true);
 }
 
-void UnwrappedLineParser::parse() {
+bool UnwrappedLineParser::parse() {
   parseToken();
-  parseLevel();
+  return parseLevel();
 }
 
-void UnwrappedLineParser::parseLevel() {
+bool UnwrappedLineParser::parseLevel() {
+  bool Error = false;
   do {
     switch (FormatTok.Tok.getKind()) {
     case tok::hash:
@@ -47,19 +48,20 @@ void UnwrappedLineParser::parseLevel() {
       parseComment();
       break;
     case tok::l_brace:
-      parseBlock();
+      Error |= parseBlock();
       addUnwrappedLine();
       break;
     case tok::r_brace:
-      return;
+      return false;
     default:
       parseStatement();
       break;
     }
   } while (!eof());
+  return Error;
 }
 
-void UnwrappedLineParser::parseBlock() {
+bool UnwrappedLineParser::parseBlock() {
   nextToken();
 
   // FIXME: Remove this hack to handle namespaces.
@@ -74,11 +76,12 @@ void UnwrappedLineParser::parseBlock() {
     --Line.Level;
   // FIXME: Add error handling.
   if (!FormatTok.Tok.is(tok::r_brace))
-    return;
+    return true;
 
   nextToken();
   if (FormatTok.Tok.is(tok::semi))
     nextToken();
+  return false;
 }
 
 void UnwrappedLineParser::parsePPDirective() {

clang/lib/Format/UnwrappedLineParser.h

@@ -79,11 +79,12 @@ public:
   UnwrappedLineParser(Lexer &Lex, SourceManager &SourceMgr,
                       UnwrappedLineConsumer &Callback);
 
-  void parse();
+  /// Returns true in case of a structural error.
+  bool parse();
 
 private:
-  void parseLevel();
+  bool parseLevel();
-  void parseBlock();
+  bool parseBlock();
   void parsePPDirective();
   void parseComment();
   void parseStatement();

clang/unittests/Format/FormatTest.cpp

@@ -381,5 +381,23 @@ TEST_F(FormatTest, IncorrectCodeDoNoWhile) {
                "};");
 }
 
+TEST_F(FormatTest, IncorrectCodeErrorDetection) {
+  EXPECT_EQ("{\n{\n}\n", format("{\n{\n}\n"));
+  EXPECT_EQ("{\n  {\n}\n", format("{\n  {\n}\n"));
+  EXPECT_EQ("{\n  {\n  }\n", format("{\n  {\n  }\n"));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 10;
+  EXPECT_EQ("{\n"
+            "    {\n"
+            " breakme(\n"
+            "     qwe);\n"
+            "}\n", format("{\n"
+                          "    {\n"
+                          " breakme(qwe);\n"
+                          "}\n", Style));
+
+}
+
 }  // end namespace tooling
 }  // end namespace clang