[clang-format] Fix break being added to macro define with ColumnLimit: 0

[futuarmo]

Summary

Fix for #49164 issue.

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

4811

Can you fix this first

[curdeius]

clang/unittests/Format/FormatTest.cpp

4810–4811

Please test with something simpler.

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

4810–4811

Did this show the bugged behavior without the patch?

Regarding the // clang-format off there is the question what do we want in the tests? Show the long lines as the long lines, then we need to turn it off some times. Or do we want to keep the limit and break the strings, thus making it harder to read for the human?

[curdeius]

clang/unittests/Format/FormatTest.cpp

4810–4811

Did this show the bugged behavior without the patch?

Yes. I checked with a pretty fresh master.

Regarding the // clang-format off there is the question what do we want in the tests? Show the long lines as the long lines, then we need to turn it off some times. Or do we want to keep the limit and break the strings, thus making it harder to read for the human?

I'm pretty much against adding // clang-format off too, it hurts more than it helps IMO.

[HazardyKnusperkeks]

If the pre merge checks pass.

[futuarmo]

Yes. Looks like I understood wrong how pre-merge clang-format check works, thanks everyone for help

[curdeius]

LGTM. Thanks for working on this!
But before landing, please retest the original misformatted code from the bug report with a freshly built clang-format.

Also, do you have commit rights? If not, do you want someone to land it on your behalf? If so, please send your name and email address to be used for the contribution.

[futuarmo]

I will build fresh main branch tomorrow (it tooks 1+ hour) and will comment here about test results
I don't have commit rights, so it will be great if someone commit it with user "Armen Khachkinaev" and email "[e-mail]"
Thanks

[curdeius]

@MyDeveloperDay, do you have any more objections?

[MyDeveloperDay]

Assuming all other tests pass, I'm ok

[futuarmo]

@curdeius, all format tests pass on freshly built version

[owenpan]

clang/unittests/Format/FormatTest.cpp

4810

Please remove the newline and re-run FormatTests.

4810–4811

Regarding the // clang-format off there is the question what do we want in the tests? Show the long lines as the long lines, then we need to turn it off some times. Or do we want to keep the limit and break the strings, thus making it harder to read for the human?

I prefer the latter.

[futuarmo]

clang/unittests/Format/FormatTest.cpp

4810

FormatTests pass again

[curdeius]

clang/unittests/Format/FormatTest.cpp

4810

Done when landing.

[curdeius]

clang/unittests/Format/FormatTest.cpp

19610

Oops. Fixed post-commit.

clang/lib/Format/ContinuationIndenter.cpp

if (Current.is(tok::lessless) && Current.isNot(TT_OverloadedOperator) &&
     return true;
 
   if (Current.NestingLevel == 0 && !Current.isTrailingComment()) {
     // Always break after "template <...>" and leading annotations. This is only
     // for cases where the entire line does not fit on a single line as a
     // different LineFormatter would be used otherwise.
     if (Previous.ClosesTemplateDeclaration)
       return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-    if (Previous.is(TT_FunctionAnnotationRParen))
+    if (Previous.is(TT_FunctionAnnotationRParen) &&
+        State.Line->Type != LT_PreprocessorDirective)
       return true;
     if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
         Current.isNot(TT_LeadingJavaAnnotation))
       return true;
   }
 
   if (Style.isJavaScript() && Previous.is(tok::r_paren) &&
       Previous.is(TT_JavaAnnotation)) {

clang/unittests/Format/FormatTest.cpp

verifyFormat("int a = 0;\n"
                "int x = FOO(a)\n"
                "int b = 0;",
                Style);
   verifyFormat("void foo(int a) { FOO(a) }\n"
                "uint32_t bar() {}",
                Style);
 }
 
+TEST_F(FormatTest, FormatsMacrosWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  verifyFormat("#define A LOOOOOOOOOOOOOOOOOOONG() LOOOOOOOOOOOOOOOOOOONG()",
+               ZeroColumn);
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
                "  f({     \\\n"
                "    g();  \\\n"
                "  });",
                getLLVMStyleWithColumns(11));
 }
 

 }
 
 // FIXME: Encode Cyrillic and CJK characters below to appease MS compilers.
 #if !defined(_MSC_VER)
 
 TEST_F(FormatTest, CountsUTF8CharactersProperly) {
   verifyFormat("\"Однажды в студёную зимнюю пору...\"",
                getLLVMStyleWithColumns(35));
-  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"",
-               getLLVMStyleWithColumns(31));
+  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"", getLLVMStyleWithColumns(31));
   verifyFormat("// Однажды в студёную зимнюю пору...",
                getLLVMStyleWithColumns(36));
   verifyFormat("// 一 二 三 四 五 六 七 八 九 十", getLLVMStyleWithColumns(32));
   verifyFormat("/* Однажды в студёную зимнюю пору... */",
                getLLVMStyleWithColumns(39));
-  verifyFormat("/* 一 二 三 四 五 六 七 八 九 十 */",
-               getLLVMStyleWithColumns(35));
+  verifyFormat("/* 一 二 三 四 五 六 七 八 九 十 */", getLLVMStyleWithColumns(35));
 }
 
 TEST_F(FormatTest, SplitsUTF8Strings) {
   // Non-printable characters' width is currently considered to be the length in
   // bytes in UTF8. The characters can be displayed in very different manner
   // (zero-width, single width with a substitution glyph, expanded to their code
   // (e.g. "<8d>"), so there's no single correct way to handle them.
   EXPECT_EQ("\"aaaaÄ\"\n"
             "\"\xc2\x8d\";",
             format("\"aaaaÄ\xc2\x8d\";", getLLVMStyleWithColumns(10)));
   EXPECT_EQ("\"aaaaaaaÄ\"\n"
             "\"\xc2\x8d\";",
             format("\"aaaaaaaÄ\xc2\x8d\";", getLLVMStyleWithColumns(10)));
   EXPECT_EQ("\"Однажды, в \"\n"
             "\"студёную \"\n"
             "\"зимнюю \"\n"
             "\"пору,\"",
             format("\"Однажды, в студёную зимнюю пору,\"",
                    getLLVMStyleWithColumns(13)));
-  EXPECT_EQ(
-      "\"一 二 三 \"\n"
+  EXPECT_EQ("\"一 二 三 \"\n"
             "\"四 五六 \"\n"
             "\"七 八 九 \"\n"
             "\"十\"",
             format("\"一 二 三 四 五六 七 八 九 十\"", getLLVMStyleWithColumns(11)));
-  EXPECT_EQ("\"一\t\"\n"
-            "\"二 \t\"\n"
-            "\"三 四 \"\n"
-            "\"五\t\"\n"
-            "\"六 \t\"\n"
-            "\"七 \"\n"
-            "\"八九十\tqq\"",
-            format("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"",
-                   getLLVMStyleWithColumns(11)));
+  EXPECT_EQ(
+      "\"一\t\"\n"
+      format("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"", getLLVMStyleWithColumns(11)));
 
   // UTF8 character in an escape sequence.
   EXPECT_EQ("\"aaaaaa\"\n"
             "\"\\\xC2\x8D\"",
             format("\"aaaaaa\\\xC2\x8D\"", getLLVMStyleWithColumns(10)));
 }
 
 TEST_F(FormatTest, HandlesDoubleWidthCharsInMultiLineStrings) {
EXPECT_EQ("/* Гляжу,\n"
             " * гору\n"
             " * Лошадка,\n"
             " * везущая\n"
             " * хворосту\n"
             " * воз. */",
             format("/* Гляжу, поднимается медленно в гору\n"
                    " * Лошадка, везущая хворосту воз. */",
                    getLLVMStyleWithColumns(13)));
-  EXPECT_EQ(
-      "/* 一二三\n"
+  EXPECT_EQ("/* 一二三\n"
             " * 四五六七\n"
             " * 八  九\n"
             " * 十  */",
             format("/* 一二三 四五六七 八  九 十  */", getLLVMStyleWithColumns(9)));
   EXPECT_EQ("/* 𝓣𝓮𝓼𝓽 𝔣𝔬𝔲𝔯\n"
             " * 𝕓𝕪𝕥𝕖\n"
             " * 𝖀𝕿𝕱-𝟠 */",
-            format("/* 𝓣𝓮𝓼𝓽 𝔣𝔬𝔲𝔯 𝕓𝕪𝕥𝕖 𝖀𝕿𝕱-𝟠 */", getLLVMStyleWithColumns(12)));
+            format("/* 𝓣𝓮𝓼𝓽 𝔣𝔬𝔲𝔯 𝕓𝕪𝕥𝕖 𝖀𝕿𝕱-𝟠 */",
+                   getLLVMStyleWithColumns(12)));
 }
 
 #endif // _MSC_VER
 
 TEST_F(FormatTest, ConstructorInitializerIndentWidth) {
   FormatStyle Style = getLLVMStyle();
 
   Style.ConstructorInitializerIndentWidth = 4;