[PATCH] Split long lines in multi-line comments.

[Alexander Kornienko]

================
Comment at: lib/Format/Format.cpp:210
@@ +209,3 @@
+
+  bool safeToSplitLine(StringRef Line) {
+    return Line.find_first_of("/\\:<>()[]{}*&%") == StringRef::npos;
----------------
Daniel Jasper wrote:
> nit: I'd prefer isLineSafeToSplit
Done.

================
Comment at: lib/Format/Format.cpp:254
@@ +253,3 @@
+    if (Prefix.endswith("*")) {
+      NewLine += " ";
+      --ColumnLimit;
----------------
Daniel Jasper wrote:
> I presume, that the " " is not part of the prefix because findCommonPrefix also looks at the last line of a comment which can be "*/". Why can't findCommonPrefix not just ignore that line instead?
Because then we'd have to ignore other lines missing space after *, e.g. "empty" lines:
/*
 * This is a cool comment.
 *
 * This is another paragraph.
 * ...

================
Comment at: unittests/Format/FormatTest.cpp:661
@@ +660,3 @@
+            format("/*\n"
+                   "This is a long      "
+                   "comment that doesn't "
----------------
Daniel Jasper wrote:
> nit: Is there a specific reason to split here and in the next line? I think not, but it makes you wonder a little.
With this layout it was easier for me to see how this should be split with regards to whitespace etc.

================
Comment at: lib/Format/Format.cpp:274
@@ +273,3 @@
+            SourceMgr,
+            TokenLoc.getLocWithOffset(Line.data() + BreakPos - Text.data()),
+            SpacePos - BreakPos + 1, NewLine));
----------------
Daniel Jasper wrote:
> I am a bit worried that this seems to rely on implementation details of StringRef (with data() always returning the exact pointer offset after calls to split, substr and ltrim).
There are no guarantees in the StringRef interface documentation, but a quick look at StringRef::data() usages revealed a lot of places that rely not only on the pointed data, but on the pointer value itself. Among these are object file format implementations, serialization etc. I would assume, it's rather unlikely that StringRef implementation will change and break all of these.

================
Comment at: lib/Format/Format.cpp:196
@@ -195,2 +195,3 @@
 private:
-  void indentBlockComment(const FormatToken &Tok, int Indent) {
+  StringRef findCommonPrefix(ArrayRef<StringRef> Lines) {
+    if (Lines.size() < 3)
----------------
Daniel Jasper wrote:
> This needs a comment describing the logic (or input->output examples).
Done.

================
Comment at: lib/Format/Format.cpp:201
@@ +200,3 @@
+    for (size_t i = 2; i < Lines.size(); ++i) {
+      for (size_t j = 0; j < Prefix.size() && j < Lines[i].size(); ++j)
+        if (Prefix[j] != Lines[i][j]) {
----------------
Daniel Jasper wrote:
> Nit: Add {}
Done.

================
Comment at: lib/Format/Format.cpp:246
@@ +245,3 @@
+    StringRef Prefix = findCommonPrefix(Lines);
+    size_t ColumnLimit = Style.ColumnLimit - Prefix.size() - IndentDelta;
+    std::string NewLine("\n");
----------------
Daniel Jasper wrote:
> Try to reuse more stuff from breakProtrudingToken(), which does something similar for String literals. Especially, reuse WhitespaceManager::breakToken(), which already does some of this logic and also works in multi-line defines.
It turned out to be not useful for this specific case.


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