[clang-format] Introduce NoFallThrough option into AllowShortCaseLabelsOnASingleLine

[MyDeveloperDay]

Summary

As highlighted by https://github.com/llvm/llvm-project/issues/57609

Give more control when using the AllowShortCaseLabelsOnASingleLine to not put fallthrough code onto the same line

switch (i) {
case 0: g(); [[fallthrough]];
case 1: f(); break;
}

whether that fallthrough is marked with the attribute or implicit

switch (i) {
case 0:
case 1: return i;
case 2:
case 3: i *= 2; break;
}

in these cases don't make them short

switch (i) {
case 0:
  g();
  [[fallthrough]];
case 1:
  f();
  break;
}

switch (i) {
case 0:
case 1:
  return i;
case 2:
case 3:
  i *= 2;
  break;
}

Fixes https://github.com/llvm/llvm-project/issues/57609

[MyDeveloperDay]

self review

[curdeius]

clang/docs/ClangFormatStyleOptions.rst

935

3595

Unrelated change.

clang/lib/Format/UnwrappedLineFormatter.cpp

643–657

Can we merge the conditions like this?

[MyDeveloperDay]

• rebase with fixed doc NFC update
• rework the nofallback condition to prevent repeated checks
• fix grammar checks

[curdeius]

LGTM after fixing the last comment.

clang/lib/Format/UnwrappedLineFormatter.cpp

650

Haven't seen this before.

[owenpan]

clang/lib/Format/UnwrappedLineFormatter.cpp

567

See below.

635–637

We don't need to add Style.

643–644

Nit.

646

Last should never be null.

650–653

Use endsSequence instead.

[HazardyKnusperkeks]

clang/include/clang/Format/Format.h

469

While we're at it, shouldn't there be a Leave? ;)

[MyDeveloperDay]

clang/include/clang/Format/Format.h

469

I think Leave is Never

[HazardyKnusperkeks]

clang/include/clang/Format/Format.h

469

No. Leave would leave me with this:

switch (a) {
  case 1: x = 1; break;
  case 2:
    return;
}

If I'd start with it. Leave allows to mix the Yes and No options.

clang/docs/ClangFormatStyleOptions.rst

* ``SBS_Always`` (in configuration: ``Always``)
 
     .. code-block:: c++
 
       while (true) {}
       while (true) { continue; }
 
 
 
-**AllowShortCaseLabelsOnASingleLine** (``Boolean``) :versionbadge:`clang-format 3.6`
-  If ``true``, short case labels will be contracted to a single line.
+**AllowShortCaseLabelsOnASingleLine** (``ShortCaseLabelStyle``) :versionbadge:`clang-format 3.6`
+  Determine if Short case labels will be contracted to a single line.
+
+  Possible values:
+
+  * ``SCLS_Never`` (in configuration: ``Never``)
+    Never merge case code
+
+  * ``SCLS_NoFallThrough`` (in configuration: ``NoFallThrough``)
+    Always contract 'simple' small case label code but
+    ignore cases where fallthrough occurs
+    (implicit or marked fallthrough)
 
     .. code-block:: c++
 
-    true:                                   false:
-    switch (a) {                    vs.     switch (a) {
-    case 1: x = 1; break;                   case 1:
-    case 2: return;                           x = 1;
-    }                                         break;
-                                            case 2:
-                                              return;
-                                            }
+      case 0:
+      case 1:
+        x = 1;
+        break;
+      case 2: x = 2; break;
+      case 3: x = 3;
+              [[fallthrough]];
+      case 4:
+        x = 4;
+        break;
+
+  * ``SCLS_Always`` (in configuration: ``Always``)
+    Always contract short case labels to a single line.
+
+    .. code-block:: c++
+
+      case 0:
+      case 1: x = 1; break;
+      case 2: x = 2; break;
+      case 3: x = 3; [[fallthrough]];
+      case 4: x = 4; break;
+
+
 
 **AllowShortEnumsOnASingleLine** (``Boolean``) :versionbadge:`clang-format 11`
   Allow short enums on a single line.
 
   .. code-block:: c++
 
     true:
     enum { A, B } myEnum;
.. code-block:: c++
        int const *a;
 
   * ``QAS_Custom`` (in configuration: ``Custom``)
     Change specifiers/qualifiers to be aligned based on ``QualifierOrder``.
     With:
 
     .. code-block:: yaml
 
-      QualifierOrder: ['inline', 'static', 'type', 'const']
+      QualifierOrder: ['inline', 'static' , 'type', 'const']
 
 
     .. code-block:: c++
 
 
        int const a;
        int const *a;
 

clang/include/clang/Format/Format.h

enum ShortBlockStyle : int8_t {
     SBS_Always,
   };
 
   /// Dependent on the value, ``while (true) { continue; }`` can be put on a
   /// single line.
   /// \version 3.5
   ShortBlockStyle AllowShortBlocksOnASingleLine;
 
-  /// If ``true``, short case labels will be contracted to a single line.
+  /// Different styles for merging short case labels.
+  enum ShortCaseLabelStyle : int8_t {
+    /// Never merge case code
+    SCLS_Never,
+    /// Always contract 'simple' small case label code but
+    /// ignore cases where fallthrough occurs
+    /// (implicit or marked fallthrough)
     /// \code
-  ///   true:                                   false:
-  ///   switch (a) {                    vs.     switch (a) {
-  ///   case 1: x = 1; break;                   case 1:
-  ///   case 2: return;                           x = 1;
-  ///   }                                         break;
-  ///                                           case 2:
-  ///                                             return;
-  ///                                           }
+    ///   case 0:
+    ///   case 1:
+    ///     x = 1;
+    ///     break;
+    ///   case 2: x = 2; break;
+    ///   case 3: x = 3;
+    ///           [[fallthrough]];
+    ///   case 4:
+    ///     x = 4;
+    ///     break;
+    /// \endcode
+    SCLS_NoFallThrough,
+    /// Always contract short case labels to a single line.
+    /// \code
+    ///   case 0:
+    ///   case 1: x = 1; break;
+    ///   case 2: x = 2; break;
+    ///   case 3: x = 3; [[fallthrough]];
+    ///   case 4: x = 4; break;
     /// \endcode
+    SCLS_Always
+  };
+
+  /// Determine if Short case labels will be contracted to a single line.
   /// \version 3.6
-  bool AllowShortCaseLabelsOnASingleLine;
+  ShortCaseLabelStyle AllowShortCaseLabelsOnASingleLine;
 
   /// Different styles for merging short functions containing at most one
   /// statement.
   enum ShortFunctionStyle : int8_t {
     /// Never merge functions into a single line.
     SFS_None,
     /// Only merge functions defined inside a class. Same as "inline",
     /// except it does not implies "empty": i.e. top level empty functions

clang/lib/Format/Format.cpp

static void enumeration(IO &IO, FormatStyle::SpacesInAnglesStyle &Value) {
     IO.enumCase(Value, "Leave", FormatStyle::SIAS_Leave);
 
     // For backward compatibility.
     IO.enumCase(Value, "false", FormatStyle::SIAS_Never);
     IO.enumCase(Value, "true", FormatStyle::SIAS_Always);
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::ShortCaseLabelStyle> {
+  static void enumeration(IO &IO, FormatStyle::ShortCaseLabelStyle &Value) {
+    IO.enumCase(Value, "Never", FormatStyle::SCLS_Never);
+    IO.enumCase(Value, "Always", FormatStyle::SCLS_Always);
+    IO.enumCase(Value, "NoFallThrough", FormatStyle::SCLS_NoFallThrough);
+
+    // For backward compatibility.
+    IO.enumCase(Value, "false", FormatStyle::SCLS_Never);
+    IO.enumCase(Value, "true", FormatStyle::SCLS_Always);
+  }
+};
+
 template <> struct MappingTraits<FormatStyle> {
   static void mapping(IO &IO, FormatStyle &Style) {
     // When reading, read the language first, we need it for getPredefinedStyle.
     IO.mapOptional("Language", Style.Language);
 
     if (IO.outputting()) {
       StringRef Styles[] = {"LLVM",   "Google", "Chromium", "Mozilla",
                             "WebKit", "GNU",    "Microsoft"};
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AlignConsecutiveBitFields = {};
   LLVMStyle.AlignConsecutiveDeclarations = {};
   LLVMStyle.AlignConsecutiveMacros = {};
   LLVMStyle.AllowAllArgumentsOnNextLine = true;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
   LLVMStyle.AllowShortEnumsOnASingleLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
-  LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
+  LLVMStyle.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Never;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_MultiLine;
   LLVMStyle.AttributeMacros.push_back("__capability");
FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
   Style.BraceWrapping.AfterExternBlock = true;
   Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
   Style.BraceWrapping.BeforeCatch = true;
   Style.BraceWrapping.BeforeElse = true;
   Style.BraceWrapping.BeforeWhile = false;
   Style.PenaltyReturnTypeOnItsOwnLine = 1000;
   Style.AllowShortEnumsOnASingleLine = false;
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
-  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Never;
   Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   Style.AllowShortLoopsOnASingleLine = false;
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   return Style;
 }
 
 FormatStyle getNoStyle() {

clang/lib/Format/UnwrappedLineFormatter.cpp

tryFitMultipleLinesInOne(LevelIndentTracker &IndentTracker,
     if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do,
                                 TT_ForEachMacro)) {
       return Style.AllowShortLoopsOnASingleLine
                  ? tryMergeSimpleControlStatement(I, E, Limit)
                  : 0;
     }
     if (TheLine->First->isOneOf(tok::kw_case, tok::kw_default)) {
       return Style.AllowShortCaseLabelsOnASingleLine
-                 ? tryMergeShortCaseLabels(I, E, Limit)
+                 ? tryMergeShortCaseLabels(I, E, Limit, PreviousLine, Style)
                  : 0;
     }
     if (TheLine->InPPDirective &&
         (TheLine->First->HasUnescapedNewline || TheLine->First->IsFirst)) {
       return tryMergeSimplePPDirective(I, E, Limit);
     }
     return 0;
   }
if (Style.AllowShortIfStatementsOnASingleLine ==
       }
     }
     return 1;
   }
 
   unsigned
   tryMergeShortCaseLabels(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
                           SmallVectorImpl<AnnotatedLine *>::const_iterator E,
-                          unsigned Limit) {
+                          unsigned Limit, const AnnotatedLine *PreviousLine,
+                          const FormatStyle &Style) {
     if (Limit == 0 || I + 1 == E ||
         I[1]->First->isOneOf(tok::kw_case, tok::kw_default)) {
       return 0;
     }
     if (I[0]->Last->is(tok::l_brace) || I[1]->First->is(tok::l_brace))
       return 0;
+    bool NoFallThrough = Style.AllowShortCaseLabelsOnASingleLine ==
+                         FormatStyle::SCLS_NoFallThrough;
+    // If the last thing on the line before was just case X:  then don't merge.
+    if (NoFallThrough && PreviousLine && PreviousLine->Last &&
+        PreviousLine->Last->is(tok::colon))
+      return 0;
+    // Check if the last thing on the line before was an attribute and if so
+    // don't merge.
+    if (NoFallThrough && PreviousLine && PreviousLine->Last &&
+        PreviousLine->Last->Previous && PreviousLine->Last->Previous) {
+      auto *PrevPrev = PreviousLine->Last->Previous->Previous;
+      if (PrevPrev && PrevPrev->startsSequence(TT_AttributeSquare,
+                                               TT_AttributeSquare, tok::semi))
+        return 0;
+    }
     unsigned NumStmts = 0;
     unsigned Length = 0;
     bool EndsWithComment = false;
     bool InPPDirective = I[0]->InPPDirective;
     const unsigned Level = I[0]->Level;
     for (; NumStmts < 3; ++NumStmts) {
       if (I + 1 + NumStmts == E)
         break;
       const AnnotatedLine *Line = I[1 + NumStmts];
       if (Line->InPPDirective != InPPDirective)
         break;
       if (Line->First->isOneOf(tok::kw_case, tok::kw_default, tok::r_brace))
         break;
+      // When the contents of the case contains an [[attribute]] don't merge it
+      // could be a fallthough.
+      if (NoFallThrough && Line->First->is(TT_AttributeSquare))
+        return 0;
       if (Line->First->isOneOf(tok::kw_if, tok::kw_for, tok::kw_switch,
                                tok::kw_while) ||
           EndsWithComment) {
         return 0;
       }
       if (Line->First->is(tok::comment)) {
         if (Level != Line->Level)
           return 0;

clang/unittests/Format/FormatTest.cpp

verifyFormat("typedef enum\n"
                "  B,\n"
                "  C\n"
                "} ShortEnum1, ShortEnum2;",
                Style);
 }
 
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
-  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
   verifyFormat("switch (a) {\n"
                "case 1: x = 1; break;\n"
                "case 2: return;\n"
                "case 3:\n"
                "case 4:\n"
                "case 5: return;\n"
                "case 6: // comment\n"
                "  return;\n"
verifyFormat("switch (a) {\n"
                "case 4:\n"
                "case 5: return;\n"
                "default:\n"
                "  y = 1;\n"
                "  break;\n"
                "}",
                Style);
   Style.ColumnLimit = 80;
-  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Never;
   Style.IndentCaseLabels = true;
   EXPECT_EQ("switch (n) {\n"
             "  default /*comments*/:\n"
             "    return true;\n"
             "  case 0:\n"
             "    return false;\n"
             "}",
             format("switch (n) {\n"
                    "default/*comments*/:\n"
                    "  return true;\n"
                    "case 0:\n"
                    "  return false;\n"
                    "}",
                    Style));
-  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
             "{\n"
             "  case 0:\n"
             "  {\n"
             "    return false;\n"
             "  }\n"
             "  default:\n"
             "  {\n"
             "    return true;\n"
             "  }\n"
             "}",
             format("switch (n) {\n"
                    "  case 0: {\n"
                    "    return false;\n"
                    "  }\n"
                    "  default:\n"
                    "  {\n"
                    "    return true;\n"
                    "  }\n"
                    "}",
                    Style));
+
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_NoFallThrough;
+  /*
+  verifyFormat("switch (a)\n"
+               "{\n"
+               "case 1: x = 1; break;\n"
+               "case 2: return;\n"
+               "case 3:\n"
+               "case 4:\n"
+               "case 5: return;\n"
+               "case 6: // comment\n"
+               "  return;\n"
+               "case 7:\n"
+               "  // comment\n"
+               "  return;\n"
+               "case 8:\n"
+               "  x = 8; // comment\n"
+               "  break;\n"
+               "default: y = 1; break;\n"
+               "}",
+               Style);
+  */
+  verifyFormat("switch (a)\n"
+               "{\n"
+               "  case 0:\n"
+               "  case 1:\n"
+               "    x = 1;\n"
+               "    break;\n"
+               "  case 2: x = 2; break;\n"
+               "  case 3:\n"
+               "    x = 3;\n"
+               "    [[fallthrough]];\n"
+               "  case 4:\n"
+               "    x = 4;\n"
+               "    break;\n"
+               "  default: y = 1; break;\n"
+               "}",
+               Style);
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
+  verifyFormat("switch (a)\n"
+               "{\n"
+               "  case 0:\n"
+               "  case 1: x = 1; break;\n"
+               "  case 2: x = 2; break;\n"
+               "  case 3: x = 3; [[fallthrough]];\n"
+               "  case 4: x = 4; break;\n"
+               "  default: y = 1; break;\n"
+               "}",
+               Style);
 }
 
 TEST_F(FormatTest, FormatsLabels) {
   verifyFormat("void f() {\n"
                "  some_code();\n"
                "test_label:\n"
                "  some_other_code();\n"
                "  {\n"
#define CHECK_PARSE(TEXT, FIELD, VALUE)                                        \
   EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
 
 TEST_F(FormatTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
-  CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
#undef CHECK_ALIGN_CONSECUTIVE
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: All",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
   // For backward compatibility:
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: false",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: true",
               AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
 
+  Style.AllowShortCaseLabelsOnASingleLine = FormatStyle::SCLS_Always;
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: Never",
+              AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Never);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: Always",
+              AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Always);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: NoFallThrough",
+              AllowShortCaseLabelsOnASingleLine,
+              FormatStyle::SCLS_NoFallThrough);
+  // For backward compatibility:
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: false",
+              AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Never);
+  CHECK_PARSE("AllowShortCaseLabelsOnASingleLine: true",
+              AllowShortCaseLabelsOnASingleLine, FormatStyle::SCLS_Always);
+
   Style.SpaceAroundPointerQualifiers = FormatStyle::SAPQ_Both;
   CHECK_PARSE("SpaceAroundPointerQualifiers: Default",
               SpaceAroundPointerQualifiers, FormatStyle::SAPQ_Default);
   CHECK_PARSE("SpaceAroundPointerQualifiers: Before",
               SpaceAroundPointerQualifiers, FormatStyle::SAPQ_Before);
   CHECK_PARSE("SpaceAroundPointerQualifiers: After",
               SpaceAroundPointerQualifiers, FormatStyle::SAPQ_After);
   CHECK_PARSE("SpaceAroundPointerQualifiers: Both",