]> granicus.if.org Git - clang/commitdiff
Correctly calculate OriginalColumn after multi-line tokens.
authorAlexander Kornienko <alexfh@google.com>
Tue, 10 Sep 2013 12:29:48 +0000 (12:29 +0000)
committerAlexander Kornienko <alexfh@google.com>
Tue, 10 Sep 2013 12:29:48 +0000 (12:29 +0000)
Summary: This also unifies the handling of escaped newlines for all tokens.

Reviewers: djasper

Reviewed By: djasper

CC: cfe-commits, klimek
Differential Revision: http://llvm-reviews.chandlerc.com/D1638

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

lib/Format/ContinuationIndenter.cpp
lib/Format/ContinuationIndenter.h
lib/Format/Format.cpp
unittests/Format/FormatTest.cpp

index 0b380041dfeccdd9feb5f013df19af3b4b3b5d65..f1924c3776d01a0f39e75c593ba391374aa2d3f2 100644 (file)
@@ -611,9 +611,8 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
   return Penalty;
 }
 
-unsigned
-ContinuationIndenter::addMultilineStringLiteral(const FormatToken &Current,
-                                                LineState &State) {
+unsigned ContinuationIndenter::addMultilineToken(const FormatToken &Current,
+                                                 LineState &State) {
   // Break before further function parameters on all levels.
   for (unsigned i = 0, e = State.Stack.size(); i != e; ++i)
     State.Stack[i].BreakBeforeParameter = true;
@@ -631,6 +630,11 @@ ContinuationIndenter::addMultilineStringLiteral(const FormatToken &Current,
 unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
                                                     LineState &State,
                                                     bool DryRun) {
+  // Don't break multi-line tokens other than block comments. Instead, just
+  // update the state.
+  if (Current.Type != TT_BlockComment && Current.IsMultiline)
+    return addMultilineToken(Current, State);
+
   if (!Current.isOneOf(tok::string_literal, tok::comment))
     return 0;
 
@@ -639,12 +643,6 @@ unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
 
   if (Current.is(tok::string_literal) &&
       Current.Type != TT_ImplicitStringLiteral) {
-    // Don't break string literals with (in case of non-raw strings, escaped)
-    // newlines. As clang-format must not change the string's content, it is
-    // unlikely that we'll end up with a better format.
-    if (Current.IsMultiline)
-      return addMultilineStringLiteral(Current, State);
-
     // Only break up default narrow strings.
     if (!Current.TokenText.startswith("\""))
       return 0;
@@ -662,16 +660,6 @@ unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
   } else if (Current.Type == TT_LineComment &&
              (Current.Previous == NULL ||
               Current.Previous->Type != TT_ImplicitStringLiteral)) {
-    // Don't break line comments with escaped newlines. These look like
-    // separate line comments, but in fact contain a single line comment with
-    // multiple lines including leading whitespace and the '//' markers.
-    //
-    // FIXME: If we want to handle them correctly, we'll need to adjust
-    // leading whitespace in consecutive lines when changing indentation of
-    // the first line similar to what we do with block comments.
-    if (Current.IsMultiline)
-      return 0;
-
     Token.reset(new BreakableLineComment(
         Current, StartColumn, State.Line->InPPDirective, Encoding, Style));
   } else {
index ef698a7a5205ea29de4b5d9cf638bc14673b4c4f..608a827194fc4440b8630b317a27a655a45157ee 100644 (file)
@@ -84,13 +84,12 @@ private:
   unsigned breakProtrudingToken(const FormatToken &Current, LineState &State,
                                 bool DryRun);
 
-  /// \brief Adds a multiline string literal to the \p State.
+  /// \brief Adds a multiline token to the \p State.
   ///
   /// \returns Extra penalty for the first line of the literal: last line is
   /// handled in \c addNextStateToQueue, and the penalty for other lines doesn't
   /// matter, as we don't change them.
-  unsigned addMultilineStringLiteral(const FormatToken &Current,
-                                     LineState &State);
+  unsigned addMultilineToken(const FormatToken &Current, LineState &State);
 
   /// \brief Returns \c true if the next token starts a multiline string
   /// literal.
index 3a0802a4609f8ed4d80591cf4ea5629179b7895a..07c6cf974da1a20cf5775f053a89e11a317a089b 100644 (file)
@@ -656,8 +656,6 @@ private:
     // In case the token starts with escaped newlines, we want to
     // take them into account as whitespace - this pattern is quite frequent
     // in macro definitions.
-    // FIXME: What do we want to do with other escaped spaces, and escaped
-    // spaces or newlines in the middle of tokens?
     // FIXME: Add a more explicit test.
     while (FormatTok->TokenText.size() > 1 && FormatTok->TokenText[0] == '\\' &&
            FormatTok->TokenText[1] == '\n') {
@@ -692,26 +690,27 @@ private:
 
     StringRef Text = FormatTok->TokenText;
     size_t FirstNewlinePos = Text.find('\n');
-    if (FirstNewlinePos != StringRef::npos) {
+    if (FirstNewlinePos == StringRef::npos) {
+      // FIXME: ColumnWidth actually depends on the start column, we need to
+      // take this into account when the token is moved.
+      FormatTok->ColumnWidth =
+          encoding::columnWidthWithTabs(Text, Column, Style.TabWidth, Encoding);
+      Column += FormatTok->ColumnWidth;
+    } else {
       FormatTok->IsMultiline = true;
+      // FIXME: ColumnWidth actually depends on the start column, we need to
+      // take this into account when the token is moved.
+      FormatTok->ColumnWidth = encoding::columnWidthWithTabs(
+          Text.substr(0, FirstNewlinePos), Column, Style.TabWidth, Encoding);
+
       // The last line of the token always starts in column 0.
       // Thus, the length can be precomputed even in the presence of tabs.
       FormatTok->LastLineColumnWidth = encoding::columnWidthWithTabs(
           Text.substr(Text.find_last_of('\n') + 1), 0, Style.TabWidth,
           Encoding);
-      Text = Text.substr(0, FirstNewlinePos);
+      Column = FormatTok->LastLineColumnWidth;
     }
 
-    // FIXME: ColumnWidth actually depends on the start column, we need to
-    // take this into account when the token is moved.
-    FormatTok->ColumnWidth =
-        encoding::columnWidthWithTabs(Text, Column, Style.TabWidth, Encoding);
-
-    // FIXME: For multi-line tokens this should be LastLineColumnWidth.
-    // For line comments this should probably be zero. But before changing,
-    // we need good tests for this.
-    Column += FormatTok->ColumnWidth;
-
     return FormatTok;
   }
 
index a27d193cb19ab5732eecba931ce0bd41292aa9b1..c7991b4711b6121fc9f486e87f3a57324243b230 100644 (file)
@@ -5736,9 +5736,6 @@ TEST_F(FormatTest, ConfigurableUseOfTab) {
                "};",
                Tab);
 
-  // FIXME: To correctly count mixed whitespace we need to
-  // also correctly count mixed whitespace in front of the comment.
-
   Tab.TabWidth = 8;
   Tab.IndentWidth = 8;
   EXPECT_EQ("/*\n"
@@ -5795,6 +5792,39 @@ TEST_F(FormatTest, ConfigurableUseOfTab) {
                    "}"));
 }
 
+TEST_F(FormatTest, CalculatesOriginalColumn) {
+  EXPECT_EQ("\"qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
+            "q\"; /* some\n"
+            "       comment */",
+            format("  \"qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
+                   "q\"; /* some\n"
+                   "       comment */",
+                   getLLVMStyle()));
+  EXPECT_EQ("// qqqqqqqqqqqqqqqqqqqqqqqqqq\n"
+            "/* some\n"
+            "   comment */",
+            format("// qqqqqqqqqqqqqqqqqqqqqqqqqq\n"
+                   " /* some\n"
+                   "    comment */",
+                   getLLVMStyle()));
+  EXPECT_EQ("// qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
+            "qqq\n"
+            "/* some\n"
+            "   comment */",
+            format("// qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
+                   "qqq\n"
+                   " /* some\n"
+                   "    comment */",
+                   getLLVMStyle()));
+  EXPECT_EQ("inttt qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
+            "wwww; /* some\n"
+            "         comment */",
+            format("  inttt qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
+                   "wwww; /* some\n"
+                   "         comment */",
+                   getLLVMStyle()));
+}
+
 TEST_F(FormatTest, ConfigurableSpaceAfterControlStatementKeyword) {
   FormatStyle NoSpace = getLLVMStyle();
   NoSpace.SpaceAfterControlStatementKeyword = false;