[clang] [clang-format] Fix parsing of goto labels (PR #196815)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: Björn Schäpers (HazardyKnusperkeks)

<details>
<summary>Changes</summary>

Goto labels are not the same as case labels, refering to the indentation of goto labels but also checking case label indentation on another line is quite confusing. Also just put the goto label on its own line.

Fixes #<!-- -->196662.

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


3 Files Affected:

- (modified) clang/lib/Format/UnwrappedLineParser.cpp (+25-21) 
- (modified) clang/lib/Format/UnwrappedLineParser.h (+1-2) 
- (modified) clang/unittests/Format/FormatTest.cpp (+19) 


``````````diff
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 022fd62ed2bfc..85542fcafca93 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1554,13 +1554,13 @@ void UnwrappedLineParser::parseStructuralElement(
     nextToken();
     if (FormatTok->is(tok::colon)) {
       FormatTok->setFinalizedType(TT_CaseLabelColon);
-      parseLabel();
+      parseCaseLabel();
       return;
     }
     if (FormatTok->is(tok::arrow)) {
       FormatTok->setFinalizedType(TT_CaseLabelArrow);
       Default->setFinalizedType(TT_SwitchExpressionLabel);
-      parseLabel();
+      parseCaseLabel();
       return;
     }
     // e.g. "default void f() {}" in a Java interface.
@@ -1582,7 +1582,7 @@ void UnwrappedLineParser::parseStructuralElement(
       nextToken();
       break;
     }
-    parseCaseLabel();
+    parseCase();
     return;
   case tok::kw_goto:
     nextToken();
@@ -1716,7 +1716,22 @@ void UnwrappedLineParser::parseStructuralElement(
       if (!Line->InMacroBody || CurrentLines->size() > 1)
         Line->Tokens.begin()->Tok->MustBreakBefore = true;
       FormatTok->setFinalizedType(TT_GotoLabelColon);
-      parseLabel(Style.IndentGotoLabels);
+      const auto OldLineLevel = Line->Level;
+      switch (Style.IndentGotoLabels) {
+      case FormatStyle::IGLS_NoIndent:
+        Line->Level = 0;
+        break;
+      case FormatStyle::IGLS_OuterIndent:
+        if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
+          --Line->Level;
+        break;
+      case FormatStyle::IGLS_HalfIndent:
+      case FormatStyle::IGLS_InnerIndent:
+        break;
+      }
+      nextToken();
+      addUnwrappedLine();
+      Line->Level = OldLineLevel;
       if (HasLabel)
         *HasLabel = true;
       return;
@@ -2151,7 +2166,7 @@ void UnwrappedLineParser::parseStructuralElement(
         nextToken();
         break;
       }
-      parseCaseLabel();
+      parseCase();
       break;
     case tok::kw_default:
       nextToken();
@@ -3374,23 +3389,12 @@ void UnwrappedLineParser::parseDoWhile() {
   parseStructuralElement();
 }
 
-void UnwrappedLineParser::parseLabel(
-    FormatStyle::IndentGotoLabelStyle IndentGotoLabels) {
+void UnwrappedLineParser::parseCaseLabel() {
   nextToken();
   unsigned OldLineLevel = Line->Level;
 
-  switch (IndentGotoLabels) {
-  case FormatStyle::IGLS_NoIndent:
-    Line->Level = 0;
-    break;
-  case FormatStyle::IGLS_OuterIndent:
-    if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
-      --Line->Level;
-    break;
-  case FormatStyle::IGLS_HalfIndent:
-  case FormatStyle::IGLS_InnerIndent:
-    break;
-  }
+  if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
+    --Line->Level;
 
   if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
       FormatTok->is(tok::l_brace)) {
@@ -3423,7 +3427,7 @@ void UnwrappedLineParser::parseLabel(
   }
 }
 
-void UnwrappedLineParser::parseCaseLabel() {
+void UnwrappedLineParser::parseCase() {
   assert(FormatTok->is(tok::kw_case) && "'case' expected");
   auto *Case = FormatTok;
 
@@ -3440,7 +3444,7 @@ void UnwrappedLineParser::parseCaseLabel() {
       break;
     }
   } while (!eof());
-  parseLabel();
+  parseCaseLabel();
 }
 
 void UnwrappedLineParser::parseSwitch(bool IsExpr) {
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 8181eec1495f7..cc7be26c2fc5b 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -159,9 +159,8 @@ class UnwrappedLineParser {
   void parseLoopBody(bool KeepBraces, bool WrapRightBrace);
   void parseForOrWhileLoop(bool HasParens = true);
   void parseDoWhile();
-  void parseLabel(FormatStyle::IndentGotoLabelStyle IndentGotoLabels =
-                      FormatStyle::IGLS_OuterIndent);
   void parseCaseLabel();
+  void parseCase();
   void parseSwitch(bool IsExpr);
   void parseNamespace();
   bool parseModuleImport();
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 4245bd1c58153..5a963cf7f36ea 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -3168,6 +3168,25 @@ TEST_F(FormatTest, FormatsLabels) {
                "    g();\n"
                "  }\n"
                "}");
+  verifyFormat("void func() {\n"
+               "label:\n"
+               "  {\n"
+               "    // Block\n"
+               "  }\n"
+               "}");
+  verifyFormat("void func() {\n"
+               "label: // Comment\n"
+               "  {\n"
+               "    // Block\n"
+               "  }\n"
+               "}");
+  verifyFormat("void func() {\n"
+               "label:\n"
+               "  // Comment\n"
+               "  {\n"
+               "    // Block\n"
+               "  }\n"
+               "}");
 
   FormatStyle Style = getLLVMStyle();
   Style.IndentGotoLabels = FormatStyle::IGLS_NoIndent;

``````````

</details>


https://github.com/llvm/llvm-project/pull/196815