[clang-format] Align trailing comments if ColumnLimit is 0

[pboettch]

Summary

ColumnLimit = 0 means no limit, so comment should always be aligned if requested. This was broken with

https://llvm.org/svn/llvm-project/cfe/trunk@304687

introduced via

https://reviews.llvm.org/D33830

and is included in 5.0.0-rc2. This commit fixes it and adds a unittest for this property.

Should go into clang-5.0 IMHO.

[djasper]

unittests/Format/FormatTestComments.cpp

2479

s/Colums/ColumnLimit/

[krasimir]

@pboettch: could you point me to information about how backporting patches to 5.0.0 works? This patch doesn't apply cleanly at head.

lib/Format/WhitespaceManager.cpp

477

If Style.ColumnLimit == 0, this subtraction overflows and gives a huge unsigned number. Could you please directly use UINT_MAX or something similar in that case?

[pboettch]

lib/Format/WhitespaceManager.cpp

477

I saw that it will overflow when adding the == 0. My idea was to simply restore the behavior as it was before your modification.

It seems that ColumnLimit=0 is not well handled by clang-format. There are a lot of cases where ColumnLimit=10000 is not behaving the same way as =0.

[pboettch]

Integrated requests made by @djasper and @krasimir .

[krasimir]

Thank you! I'll ask around if this can be merged into 5.0.0.

[djasper]

lib/Format/WhitespaceManager.cpp

477

That's very much intentional. ColumnLimit=10000 means (basically): Put as much as you can on one line. ColumnLimit=0 means: Try to stick with the original code's line breaks.

[hans]

In D36967#847694, @krasimir wrote:

Thank you! I'll ask around if this can be merged into 5.0.0.

Since it's fixing a regression, merging it sounds good to me. Let me know when it lands.

[krasimir]

@pboettch, do you have commit access or should I commit this patch for you?

[pboettch]

@krasimir No I don't have commit right. Feel free to commit this change. Thanks.

lib/Format/WhitespaceManager.cpp

       continue;
 
     unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
-    unsigned ChangeMaxColumn = Style.ColumnLimit >= Changes[i].TokenLength
-                                   ? Style.ColumnLimit - Changes[i].TokenLength
-                                   : ChangeMinColumn;
+    unsigned ChangeMaxColumn;
+
+    if (Style.ColumnLimit == 0)
+      ChangeMaxColumn = UINT_MAX;
+    else if (Style.ColumnLimit >= Changes[i].TokenLength)
+      ChangeMaxColumn = Style.ColumnLimit - Changes[i].TokenLength;
+    else
+      ChangeMaxColumn = ChangeMinColumn;
 
     // If we don't create a replacement for this change, we have to consider
     // it to be immovable.

unittests/Format/FormatTestComments.cpp

                    "int k; // line longg long",
                    getLLVMStyleWithColumns(20)));
 
+  // Always align if ColumnLimit = 0
+  EXPECT_EQ("int i, j; // line 1\n"
+            "int k;    // line longg long",
+            format("int i, j; // line 1\n"
+                   "int k; // line longg long",
+                   getLLVMStyleWithColumns(0)));
+
   // Align comment line sections aligned with the next token with the next
   // token.
   EXPECT_EQ("class A {\n"