r184425 - Use the same set of whitespace characters for all operations in BreakableToken.

[Alexander Kornienko]

Author: alexfh
Date: Thu Jun 20 08:58:37 2013
New Revision: 184425

URL: http://llvm.org/viewvc/llvm-project?rev=184425&view=rev
Log:
Use the same set of whitespace characters for all operations in BreakableToken.

Summary:
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.

Reviewers: klimek

Reviewed By: klimek

CC: cfe-commits

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

Modified:
    cfe/trunk/lib/Format/BreakableToken.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=184425&r1=184424&r2=184425&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Thu Jun 20 08:58:37 2013
@@ -26,6 +26,19 @@ namespace clang {
 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 @@ BreakableToken::Split getCommentSplit(St
     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 @@ BreakableToken::Split getStringSplit(Str
     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 @@ void BreakableBlockComment::adjustWhites
 
   // 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();
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=184425&r1=184424&r2=184425&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Jun 20 08:58:37 2013
@@ -893,6 +893,9 @@ TEST_F(FormatTest, SplitsLongCxxComments
                    "    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) {