[llvm-branch-commits] [clang] c7dcc4c - [clang-format] PR48569 clang-format fails to align case label with `switch` with Whitesmith Indentation

[via llvm-branch-commits]

Author: mydeveloperday
Date: 2020-12-26T15:19:03Z
New Revision: c7dcc4c72588db9ffb7ae379983450193b943f5b

URL: https://github.com/llvm/llvm-project/commit/c7dcc4c72588db9ffb7ae379983450193b943f5b
DIFF: https://github.com/llvm/llvm-project/commit/c7dcc4c72588db9ffb7ae379983450193b943f5b.diff

LOG: [clang-format] PR48569 clang-format fails to align case label with `switch` with Whitesmith Indentation

https://bugs.llvm.org/show_bug.cgi?id=48569

This is a tentative fix which addresses a PR raise regarding Case indentation when working with Whitesmiths Indentation

I could not find online any reference sources as to what the case indentation for Whitesmith's should be (or be allowed to be)

But according to the documentation, we don't obey the rules for Whitesmith's

```
In particular, the documentation states that this option is to "indent case labels one level from the switch statement. When false, use the same indentation level as for the switch statement."
```

The behaviour we add here is actually as the TODO in the tests used to state in {D67627}, but when {D82016} was added and I brought these tests out from being TODO I realized I changed the indentation.

Reviewed By: curdeius, HazardyKnusperkeks

Differential Revision: https://reviews.llvm.org/D93806

Added: 
    

Modified: 
    clang/lib/Format/UnwrappedLineParser.cpp
    clang/unittests/Format/FormatTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 4c2ee421d092..99b8c28dca2b 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2244,18 +2244,26 @@ void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
     --Line->Level;
   if (LeftAlignLabel)
     Line->Level = 0;
+
+  bool RemoveWhitesmithsCaseIndent =
+      (!Style.IndentCaseBlocks &&
+       Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths);
+
+  if (RemoveWhitesmithsCaseIndent)
+    --Line->Level;
+
   if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
       FormatTok->Tok.is(tok::l_brace)) {
-    CompoundStatementIndenter Indenter(this, Line->Level,
-                                       Style.BraceWrapping.AfterCaseLabel,
-                                       Style.BraceWrapping.IndentBraces);
+
+    CompoundStatementIndenter Indenter(
+        this, Line->Level, Style.BraceWrapping.AfterCaseLabel,
+        Style.BraceWrapping.IndentBraces || RemoveWhitesmithsCaseIndent);
     parseBlock(/*MustBeDeclaration=*/false);
     if (FormatTok->Tok.is(tok::kw_break)) {
       if (Style.BraceWrapping.AfterControlStatement ==
           FormatStyle::BWACS_Always) {
         addUnwrappedLine();
-        if (!Style.IndentCaseBlocks &&
-            Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) {
+        if (RemoveWhitesmithsCaseIndent) {
           Line->Level++;
         }
       }
@@ -2276,6 +2284,7 @@ void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
 
 void UnwrappedLineParser::parseCaseLabel() {
   assert(FormatTok->Tok.is(tok::kw_case) && "'case' expected");
+
   // FIXME: fix handling of complex expressions here.
   do {
     nextToken();

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index b6d9e67273ac..bdf4abb26c7e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -13658,7 +13658,7 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
                "  {\n"
                "  switch (a)\n"
                "    {\n"
-               "    case 2:\n"
+               "  case 2:\n"
                "    {\n"
                "    }\n"
                "    break;\n"
@@ -13670,18 +13670,18 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
                "  {\n"
                "  switch (a)\n"
                "    {\n"
-               "    case 0:\n"
+               "  case 0:\n"
                "    break;\n"
-               "    case 1:\n"
+               "  case 1:\n"
                "    {\n"
                "    foo();\n"
                "    break;\n"
                "    }\n"
-               "    case 2:\n"
+               "  case 2:\n"
                "    {\n"
                "    }\n"
                "    break;\n"
-               "    default:\n"
+               "  default:\n"
                "    break;\n"
                "    }\n"
                "  }\n",
@@ -13691,12 +13691,12 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
                "  {\n"
                "  switch (a)\n"
                "    {\n"
-               "    case 0:\n"
+               "  case 0:\n"
                "    {\n"
                "    foo(x);\n"
                "    }\n"
                "    break;\n"
-               "    default:\n"
+               "  default:\n"
                "    {\n"
                "    foo(1);\n"
                "    }\n"