release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494)

[llvmbot]

Backport d89f200

Requested by: @owenca

[llvmbot]

@mydeveloperday What do you think about merging this PR to the release branch?

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: None (llvmbot)

Changes

Backport d89f200

Requested by: @owenca

---

Full diff: https://github.com/llvm/llvm-project/pull/93494.diff

3 Files Affected:

• (modified) clang/lib/Format/UnwrappedLineParser.cpp (+2-7)
• (modified) clang/unittests/Format/FormatTest.cpp (+8)
• (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+13)

diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index f70affb732a0d..179d77bf00491 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1185,12 +1185,6 @@ void UnwrappedLineParser::parsePPDefine() {
     return;
   }
 
-  if (FormatTok->is(tok::identifier) &&
-      Tokens->peekNextToken()->is(tok::colon)) {
-    nextToken();
-    nextToken();
-  }
-
   // Errors during a preprocessor directive can only affect the layout of the
   // preprocessor directive, and thus we ignore them. An alternative approach
   // would be to use the same approach we use on the file level (no
@@ -1671,7 +1665,8 @@ void UnwrappedLineParser::parseStructuralElement(
     if (!Style.isJavaScript() && !Style.isVerilog() && !Style.isTableGen() &&
         Tokens->peekNextToken()->is(tok::colon) && !Line->MustBeDeclaration) {
       nextToken();
-      Line->Tokens.begin()->Tok->MustBreakBefore = true;
+      if (!Line->InMacroBody || CurrentLines->size() > 1)
+        Line->Tokens.begin()->Tok->MustBreakBefore = true;
       FormatTok->setFinalizedType(TT_GotoLabelColon);
       parseLabel(!Style.IndentGotoLabels);
       if (HasLabel)
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 0161a1685eb12..df730838738d1 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -3118,6 +3118,7 @@ TEST_F(FormatTest, FormatsLabels) {
                "    g();\n"
                "  }\n"
                "}");
+
   FormatStyle Style = getLLVMStyle();
   Style.IndentGotoLabels = false;
   verifyFormat("void f() {\n"
@@ -3157,6 +3158,13 @@ TEST_F(FormatTest, FormatsLabels) {
                "  }\n"
                "}",
                Style);
+
+  Style.ColumnLimit = 15;
+  verifyFormat("#define FOO   \\\n"
+               "label:        \\\n"
+               "  break;",
+               Style);
+
   // The opening brace may either be on the same unwrapped line as the colon or
   // on a separate one. The formatter should recognize both.
   Style = getLLVMStyle();
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 44ebad9d5a872..dfa9c22430f36 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2353,15 +2353,28 @@ TEST_F(TokenAnnotatorTest, UnderstandsLabels) {
   auto Tokens = annotate("{ x: break; }");
   ASSERT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+
   Tokens = annotate("{ case x: break; }");
   ASSERT_EQ(Tokens.size(), 8u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+
   Tokens = annotate("{ x: { break; } }");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon);
+
   Tokens = annotate("{ case x: { break; } }");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
+
+  Tokens = annotate("#define FOO label:");
+  ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon);
+
+  Tokens = annotate("#define FOO \\\n"
+                    "label: \\\n"
+                    "  break;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) {

[owenca]

@tstellar can we get this one in if we are doing another point release?

[tstellar]

@owenca I'm only going to take regression fixes for 18.1.7.

[owenca]

@tstellar this is a regression fix as labeled by the issue the original pr/commit fixed. :)

[tstellar]

@owenca OK, can you give me more details:

1. How risky is this fix?
2. When was the last known working version of clang-format?

[owenca]

@tstellar

1. How risky is this fix?

Very low IMO. (@mydeveloperday @HazardyKnusperkeks @rymiel can chime in if anyone thinks otherwise.)

2. When was the last known working version of clang-format?

17.0.6 (regressed since 18.1.1).

[tstellar]

@owenca Do you have a release note we can put in the release announcement?

[owenca]

@owenca Do you have a release note we can put in the release announcement?

Release note:
Fixes a clang-format regression (since 17.0.6) on formatting goto labels in macro definitions.