[PATCH] Remove extra whitespace instead of breaking the line in comments when possible.

[Alexander Kornienko]

Hi klimek,

Solves the problem described in http://llvm.org/PR17756

http://llvm-reviews.chandlerc.com/D2131

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTest.cpp

Index: lib/Format/BreakableToken.cpp
===================================================================
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -173,7 +173,10 @@
 
 void BreakableStringLiteral::insertBreak(unsigned LineIndex,
                                          unsigned TailOffset, Split Split,
+                                         bool InsertNewline,
                                          WhitespaceManager &Whitespaces) {
+  assert(InsertNewline &&
+         "One does not simply remove whitespace from a string literal");
   Whitespaces.replaceWhitespaceInToken(
       Tok, Prefix.size() + TailOffset + Split.first, Split.second, Postfix,
       Prefix, InPPDirective, 1, IndentLevel, StartColumn);
@@ -211,11 +214,16 @@
 }
 
 void BreakableLineComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
-                                       Split Split,
+                                       Split Split, bool InsertNewline,
                                        WhitespaceManager &Whitespaces) {
-  Whitespaces.replaceWhitespaceInToken(
-      Tok, OriginalPrefix.size() + TailOffset + Split.first, Split.second,
-      Postfix, Prefix, InPPDirective, 1, IndentLevel, StartColumn);
+  if (InsertNewline)
+    Whitespaces.replaceWhitespaceInToken(
+        Tok, OriginalPrefix.size() + TailOffset + Split.first, Split.second,
+        Postfix, Prefix, InPPDirective, 1, IndentLevel, StartColumn);
+  else
+    Whitespaces.replaceWhitespaceInToken(
+        Tok, OriginalPrefix.size() + TailOffset + Split.first, Split.second, "",
+        " ", false, 0, 0, 0);
 }
 
 void
@@ -355,7 +363,7 @@
 }
 
 void BreakableBlockComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
-                                        Split Split,
+                                        Split Split, bool InsertNewline,
                                         WhitespaceManager &Whitespaces) {
   StringRef Text = Lines[LineIndex].substr(TailOffset);
   StringRef Prefix = Decoration;
@@ -369,9 +377,13 @@
       Text.data() - Tok.TokenText.data() + Split.first;
   unsigned CharsToRemove = Split.second;
   assert(IndentAtLineBreak >= Decoration.size());
-  Whitespaces.replaceWhitespaceInToken(
-      Tok, BreakOffsetInToken, CharsToRemove, "", Prefix, InPPDirective, 1,
-      IndentLevel, IndentAtLineBreak - Decoration.size());
+  if (InsertNewline)
+    Whitespaces.replaceWhitespaceInToken(
+        Tok, BreakOffsetInToken, CharsToRemove, "", Prefix, InPPDirective, 1,
+        IndentLevel, IndentAtLineBreak - Decoration.size());
+  else
+    Whitespaces.replaceWhitespaceInToken(Tok, BreakOffsetInToken, CharsToRemove,
+                                         "", " ", false, 0, 0, 0);
 }
 
 void
Index: lib/Format/BreakableToken.h
===================================================================
--- lib/Format/BreakableToken.h
+++ lib/Format/BreakableToken.h
@@ -59,6 +59,7 @@
 
   /// \brief Emits the previously retrieved \p Split via \p Whitespaces.
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
+                           bool InsertNewline,
                            WhitespaceManager &Whitespaces) = 0;
 
   /// \brief Replaces the whitespace between \p LineIndex-1 and \p LineIndex.
@@ -120,6 +121,7 @@
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
                          unsigned ColumnLimit) const;
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
+                           bool InsertNewline,
                            WhitespaceManager &Whitespaces);
 };
 
@@ -136,6 +138,7 @@
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
                          unsigned ColumnLimit) const;
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
+                           bool InsertNewline,
                            WhitespaceManager &Whitespaces);
   virtual void replaceWhitespaceBefore(unsigned LineIndex,
                                        WhitespaceManager &Whitespaces);
@@ -165,6 +168,7 @@
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
                          unsigned ColumnLimit) const;
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
+                           bool InsertNewline,
                            WhitespaceManager &Whitespaces);
   virtual void replaceWhitespaceBefore(unsigned LineIndex,
                                        WhitespaceManager &Whitespaces);
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -811,9 +811,18 @@
       assert(Split.first != 0);
       unsigned NewRemainingTokenColumns = Token->getLineLengthAfterSplit(
           LineIndex, TailOffset + Split.first + Split.second, StringRef::npos);
+
+      // We can remove extra whitespace instead of breaking the line.
+      if (RemainingTokenColumns + 1 - Split.second <= RemainingSpace) {
+        RemainingTokenColumns = 0;
+        if (!DryRun)
+          Token->insertBreak(LineIndex, TailOffset, Split, false, Whitespaces);
+        break;
+      }
+
       assert(NewRemainingTokenColumns < RemainingTokenColumns);
       if (!DryRun)
-        Token->insertBreak(LineIndex, TailOffset, Split, Whitespaces);
+        Token->insertBreak(LineIndex, TailOffset, Split, true, Whitespaces);
       Penalty += Current.SplitPenalty;
       unsigned ColumnsUsed =
           Token->getLineLengthAfterSplit(LineIndex, TailOffset, Split.first);
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1224,6 +1224,15 @@
                    "      \n"
                    "               \n"
                    "      */\n"));
+
+  EXPECT_EQ("/* a a */",
+            format("/* a a            */", getLLVMStyleWithColumns(15)));
+  EXPECT_EQ("/* aaa aaa\n"
+            " * aaaaa */",
+            format("/* aaa aaa aaaaa       */", getLLVMStyleWithColumns(15)));
+  EXPECT_EQ("/* aaa aaa\n"
+            " * aaaaa     */",
+            format("/* aaa aaa aaaaa     */", getLLVMStyleWithColumns(15)));
 }
 
 TEST_F(FormatTest, SplitsLongLinesInCommentsInPreprocessor) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2131.1.patch
Type: text/x-patch
Size: 6418 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131108/d04257cb/attachment.bin>