Fixes problems with line merging in the face of preprocessor directives.

This patch prepares being able to test for and fix more problems (see
FIXME in the test for example).

Previously we would output unwrapped lines for preprocessor directives
at the point where we also parsed the hash token. Since often
projections only terminate (and thus output their own unwrapped line)
after peeking at the next token, this would lead to the formatter seeing
the preprocessor directives out-of-order (slightly earlier). To be able
to correctly identify lines to merge, the formatter needs a well-defined
order of unwrapped lines, which this patch introduces.

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

diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp
index 1f71b5b7dacc472bcec6411a198d5c6a31b4ac55..a0a18ac1cfc4859bcf63e0f6495308558e759439 100644 (file)
--- a/lib/Format/UnwrappedLineParser.cpp
+++ b/lib/Format/UnwrappedLineParser.cpp
@@ -79,7 +79,11 @@ private:
 
 class ScopedLineState {
 public:
-  ScopedLineState(UnwrappedLineParser &Parser) : Parser(Parser) {
+  ScopedLineState(UnwrappedLineParser &Parser,
+                  bool SwitchToPreprocessorLines = false)
+      : Parser(Parser), SwitchToPreprocessorLines(SwitchToPreprocessorLines) {
+    if (SwitchToPreprocessorLines)
+      Parser.CurrentLines = &Parser.PreprocessorDirectives;
     PreBlockLine = Parser.Line.take();
     Parser.Line.reset(new UnwrappedLine());
     Parser.Line->Level = PreBlockLine->Level;
@@ -93,10 +97,13 @@ public:
     assert(Parser.Line->Tokens.empty());
     Parser.Line.reset(PreBlockLine);
     Parser.MustBreakBeforeNextToken = true;
+    if (SwitchToPreprocessorLines)
+      Parser.CurrentLines = &Parser.Lines;
   }
 
 private:
   UnwrappedLineParser &Parser;
+  const bool SwitchToPreprocessorLines;
 
   UnwrappedLine *PreBlockLine;
 };
@@ -104,14 +111,20 @@ private:
 UnwrappedLineParser::UnwrappedLineParser(
     clang::DiagnosticsEngine &Diag, const FormatStyle &Style,
     FormatTokenSource &Tokens, UnwrappedLineConsumer &Callback)
-    : Line(new UnwrappedLine), MustBreakBeforeNextToken(false), Diag(Diag),
-      Style(Style), Tokens(&Tokens), Callback(Callback) {
-}
+    : Line(new UnwrappedLine), MustBreakBeforeNextToken(false),
+      CurrentLines(&Lines), Diag(Diag), Style(Style), Tokens(&Tokens),
+      Callback(Callback) {}
 
 bool UnwrappedLineParser::parse() {
   DEBUG(llvm::dbgs() << "----\n");
   readToken();
-  return parseFile();
+  bool Error = parseFile();
+  for (std::vector<UnwrappedLine>::iterator I = Lines.begin(),
+                                            E = Lines.end();
+       I != E; ++I) {
+    Callback.consumeUnwrappedLine(*I);
+  }
+  return Error;
 }
 
 bool UnwrappedLineParser::parseFile() {
@@ -668,8 +681,17 @@ void UnwrappedLineParser::addUnwrappedLine() {
     }
     llvm::dbgs() << "\n";
   });
-  Callback.consumeUnwrappedLine(*Line);
+  CurrentLines->push_back(*Line);
   Line->Tokens.clear();
+  if (CurrentLines == &Lines && !PreprocessorDirectives.empty()) {
+    for (std::vector<UnwrappedLine>::iterator I = PreprocessorDirectives
+             .begin(), E = PreprocessorDirectives.end();
+         I != E; ++I) {
+      CurrentLines->push_back(*I);
+    }
+    PreprocessorDirectives.clear();
+  }
+
 }
 
 bool UnwrappedLineParser::eof() const {
@@ -692,7 +714,11 @@ void UnwrappedLineParser::readToken() {
   while (!Line->InPPDirective && FormatTok.Tok.is(tok::hash) &&
          ((FormatTok.NewlinesBefore > 0 && FormatTok.HasUnescapedNewline) ||
           FormatTok.IsFirst)) {
-    ScopedLineState BlockState(*this);
+    // If there is an unfinished unwrapped line, we flush the preprocessor
+    // directives only after that unwrapped line was finished later.
+    bool SwitchToPreprocessorLines = !Line->Tokens.empty() &&
+                                     CurrentLines == &Lines;
+    ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
     parsePPDirective();
   }
 }

diff --git a/lib/Format/UnwrappedLineParser.h b/lib/Format/UnwrappedLineParser.h
index c261732032688a20d4671c4cc1d2fb03f21a4b2c..fb1812852aee2b2805526183ce00eacd9e04f84a 100644 (file)
--- a/lib/Format/UnwrappedLineParser.h
+++ b/lib/Format/UnwrappedLineParser.h
@@ -159,6 +159,21 @@ private:
   FormatToken FormatTok;
   bool MustBreakBeforeNextToken;
 
+  // The parsed lines. This is a pointer so we can switch it out to parse an
+  // unrelated set of unwrapped lines and put them into place later.
+  std::vector<UnwrappedLine> Lines;
+
+  // Preprocessor directives are parsed out-of-order from other unwrapped lines.
+  // Thus, we need to keep a list of preprocessor directives to be reported
+  // after an unwarpped line that has been started was finished.
+  std::vector<UnwrappedLine> PreprocessorDirectives;
+
+  // New unwrapped lines are added via CurrentLines.
+  // Usually points to \c &Lines. While parsing a preprocessor directive when
+  // there is an unfinished previous unwrapped line, will point to
+  // \c &PreprocessorDirectives.
+  std::vector<UnwrappedLine> *CurrentLines;
+
   clang::DiagnosticsEngine &Diag;
   const FormatStyle &Style;
   FormatTokenSource *Tokens;

diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 806707cb4cd5c4e94cd321615f9b10ab71851e96..6b77868776a73d092f7c04c3ae7d753b4d3815e6 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -1537,6 +1537,16 @@ TEST_F(FormatTest, DoNotInterfereWithErrorAndWarning) {
   EXPECT_EQ("#warning 1", format("  #  warning 1"));
 }
 
+TEST_F(FormatTest, MergeHandlingInTheFaceOfPreprocessorDirectives) {
+  FormatStyle AllowsMergedIf = getGoogleStyle();
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = true;
+  verifyFormat("void f() { f(); }\n#error E", AllowsMergedIf);
+  verifyFormat("if (true) return 42;\n#error E", AllowsMergedIf);
+
+  // FIXME:
+  // verifyFormat("if (true)\n#error E\n  return 42;", AllowsMergedIf);
+}
+
 // FIXME: This breaks the order of the unwrapped lines:
 // TEST_F(FormatTest, OrderUnwrappedLines) {
 //   verifyFormat("{\n"