[PATCH] D36956: [clang-format] Emit absolute splits before lines for comments

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a subscriber: klimek.

This patch makes the splits emitted for the beginning of comment lines during
reformatting absolute. Previously, they were relative to the start of the
non-whitespace content of the line, which messes up further TailOffset
calculations in breakProtrudingToken. This fixes an assertion failure reported
in bug 34236: https://bugs.llvm.org/show_bug.cgi?id=34236.


https://reviews.llvm.org/D36956

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


Index: unittests/Format/FormatTestComments.cpp
===================================================================
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -2780,6 +2780,23 @@
                    "* long */",
                    getLLVMStyleWithColumns(20)));
 }
+
+TEST_F(FormatTestComments, NoCrush_Bug34236) {
+  // This is a test case from a crusher reported in bug 34236:
+  // https://bugs.llvm.org/show_bug.cgi?id=34236
+  EXPECT_EQ(
+      R"(
+/*                                                                */ /*
+                                                                      *       a
+                                                                      * b c
+                                                                      * d*/)",
+      format(
+          R"(
+/*                                                                */ /*
+ *       a b
+ *       c     d*/)",
+          getLLVMStyleWithColumns(80)));
+}
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/BreakableToken.cpp
===================================================================
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -545,15 +545,18 @@
 }
 
 BreakableToken::Split BreakableBlockComment::getSplitBefore(
-    unsigned LineIndex,
-    unsigned PreviousEndColumn,
-    unsigned ColumnLimit,
+    unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
     llvm::Regex &CommentPragmasRegex) const {
   if (!mayReflow(LineIndex, CommentPragmasRegex))
     return Split(StringRef::npos, 0);
   StringRef TrimmedContent = Content[LineIndex].ltrim(Blanks);
-  return getReflowSplit(TrimmedContent, ReflowPrefix, PreviousEndColumn,
-                        ColumnLimit);
+  Split TrimmedSplit = getReflowSplit(TrimmedContent, ReflowPrefix,
+                                      PreviousEndColumn, ColumnLimit);
+  if (TrimmedSplit.first == StringRef::npos)
+    return TrimmedSplit;
+  return Split(TrimmedSplit.first + Content[LineIndex].size() -
+                   TrimmedContent.size(),
+               TrimmedSplit.second);
 }
 
 unsigned BreakableBlockComment::getReflownColumn(
@@ -633,17 +636,12 @@
         /*CurrentPrefix=*/ReflowPrefix, InPPDirective, /*Newlines=*/0,
         /*Spaces=*/0);
     // Check if we need to also insert a break at the whitespace range.
-    // For this we first adapt the reflow split relative to the beginning of the
-    // content.
     // Note that we don't need a penalty for this break, since it doesn't change
     // the total number of lines.
-    Split BreakSplit = SplitBefore;
-    BreakSplit.first += TrimmedContent.data() - Content[LineIndex].data();
     unsigned ReflownColumn =
         getReflownColumn(TrimmedContent, LineIndex, PreviousEndColumn);
-    if (ReflownColumn > ColumnLimit) {
-      insertBreak(LineIndex, 0, BreakSplit, Whitespaces);
-    }
+    if (ReflownColumn > ColumnLimit)
+      insertBreak(LineIndex, 0, SplitBefore, Whitespaces);
     return;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36956.111965.patch
Type: text/x-patch
Size: 3069 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170821/9555485d/attachment.bin>