klimek added inline comments.

================
Comment at: lib/Format/ContinuationIndenter.cpp:1707
+          RemainingTokenColumns = Token->getRemainingLength(
+              NextLineIndex, TailOffset, ContentStartColumn);
+          Reflow = true;
----------------
krasimir wrote:
> When we're reflowing we replace the reflow split with a space, so IMO this 
> should be:
> ```
> RemainingTokenColumns = Token->getRemainingLength(
>               NextLineIndex, TailOffset, ContentStartColumn + 1);
> ```
Actually, ContentStartColumn is just calculated incorrectly above. Added tests, 
and added the +1 above, which makes it +1 for all code below.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1721
+                Token->getSplit(NextLineIndex, TailOffset, ColumnLimit,
+                                ContentStartColumn, CommentPragmasRegex);
+            if (Split.first == StringRef::npos) {
----------------
krasimir wrote:
> Looks like `ContentStartColumn` is consistently used as the start column of 
> the reflown content on the next line.
> I suggest to `++ContentStartColumn` at the beginning of the body of this if 
> statement (and adjust below appropriately).
Yep, that's what I also figured out - that also led to removing 
++ContentStartColumn in the reflow case below, which was what made this work at 
all.
Added tests to ReflowsCommentsPrecise, which flow through all of the corner 
cases of the if's here.


https://reviews.llvm.org/D40310



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to