[PATCH] Use the same set of whitespace characters for all operations in BreakableToken.

[Alexander Kornienko]

Hi klimek,

Fixes a problem where \t,\v or \f could lead to a crash when placed as
a first character in a line comment. The cause is that rtrim and ltrim handle
these characters, but our code didn't, so some invariants could be broken.

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

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

Index: lib/Format/BreakableToken.cpp
===================================================================
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -26,6 +26,19 @@
 namespace format {
 namespace {
 
+static const char *Blanks = " \t\v\f";
+static bool IsBlank(char C) {
+  switch (C) {
+    case ' ':
+    case '\t':
+    case '\v':
+    case '\f':
+      return true;
+    default:
+      return false;
+  }
+}
+
 BreakableToken::Split getCommentSplit(StringRef Text,
                                       unsigned ContentStartColumn,
                                       unsigned ColumnLimit,
@@ -41,22 +54,22 @@
     MaxSplitBytes +=
         encoding::getCodePointNumBytes(Text[MaxSplitBytes], Encoding);
 
-  StringRef::size_type SpaceOffset = Text.rfind(' ', MaxSplitBytes);
+  StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
   if (SpaceOffset == StringRef::npos ||
       // Don't break at leading whitespace.
-      Text.find_last_not_of(' ', SpaceOffset) == StringRef::npos) {
+      Text.find_last_not_of(Blanks, SpaceOffset) == StringRef::npos) {
     // Make sure that we don't break at leading whitespace that
     // reaches past MaxSplit.
-    StringRef::size_type FirstNonWhitespace = Text.find_first_not_of(" ");
+    StringRef::size_type FirstNonWhitespace = Text.find_first_not_of(Blanks);
     if (FirstNonWhitespace == StringRef::npos)
       // If the comment is only whitespace, we cannot split.
       return BreakableToken::Split(StringRef::npos, 0);
-    SpaceOffset =
-        Text.find(' ', std::max<unsigned>(MaxSplitBytes, FirstNonWhitespace));
+    SpaceOffset = Text.find_first_of(
+        Blanks, std::max<unsigned>(MaxSplitBytes, FirstNonWhitespace));
   }
   if (SpaceOffset != StringRef::npos && SpaceOffset != 0) {
-    StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim();
-    StringRef AfterCut = Text.substr(SpaceOffset).ltrim();
+    StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks);
+    StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks);
     return BreakableToken::Split(BeforeCut.size(),
                                  AfterCut.begin() - BeforeCut.end());
   }
@@ -92,11 +105,11 @@
     if (Chars > MaxSplit)
       break;
 
-    if (Text[0] == ' ')
+    if (IsBlank(Text[0]))
       SpaceOffset = SplitPoint;
     if (Text[0] == '/')
       SlashOffset = SplitPoint;
-    if (Text[0] != '\\' && !isAlphanumeric(Text[0]))
+    if (Advance == 1 && !isAlphanumeric(Text[0]))
       WordStartOffset = SplitPoint;
 
     SplitPoint += Advance;
@@ -280,13 +293,13 @@
 
   // Calculate the end of the non-whitespace text in the previous line.
   EndOfPreviousLine =
-      Lines[LineIndex - 1].find_last_not_of(" \t", EndOfPreviousLine);
+      Lines[LineIndex - 1].find_last_not_of(Blanks, EndOfPreviousLine);
   if (EndOfPreviousLine == StringRef::npos)
     EndOfPreviousLine = 0;
   else
     ++EndOfPreviousLine;
   // Calculate the start of the non-whitespace text in the current line.
-  size_t StartOfLine = Lines[LineIndex].find_first_not_of(" \t");
+  size_t StartOfLine = Lines[LineIndex].find_first_not_of(Blanks);
   if (StartOfLine == StringRef::npos)
     StartOfLine = Lines[LineIndex].size();
 
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -893,6 +893,9 @@
                    "    int bbb, // xxxxxxx yyyyyyyyy\n"
                    "    int c, int d, int e) {}",
                    getLLVMStyleWithColumns(40)));
+  EXPECT_EQ("//\t aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+            format("//\t aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+                   getLLVMStyleWithColumns(20)));
 }
 
 TEST_F(FormatTest, PriorityOfCommentBreaking) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1013.1.patch
Type: text/x-patch
Size: 3828 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130620/a9d1ba1a/attachment.bin>