[clang] 14c0447 - [clang-format] Add IndentCaseBlocks option

via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 19 07:53:26 PST 2020


Author: mydeveloperday
Date: 2020-01-19T15:52:26Z
New Revision: 14c044756e771eb9160d5809b4381bdeb0fc210c

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

LOG: [clang-format] Add IndentCaseBlocks option

Summary:
The documentation for IndentCaseLabels claimed that the "Switch
statement body is always indented one level more than case labels". This
is technically false for the code block immediately following the label.
Its closing bracket aligns with the start of the label.

If the case label are not indented, it leads to a style where the
closing bracket of the block aligns with the closing bracket of the
switch statement, which can be hard to parse.

This change introduces a new option, IndentCaseBlocks, which when true
treats the block as a scope block (which it technically is).

(Note: regenerated ClangFormatStyleOptions.rst using tools/dump_style.py)

Reviewed By: MyDeveloperDay

Patch By: capn

Tags: #clang-format, #clang

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

Added: 
    

Modified: 
    clang/docs/ClangFormatStyleOptions.rst
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Format/Format.h
    clang/lib/Format/Format.cpp
    clang/lib/Format/UnwrappedLineParser.cpp
    clang/unittests/Format/FormatTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 18cdc54a370f..fd77e1e727a9 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1605,12 +1605,36 @@ the configuration (without a prefix: ``Auto``).
   ``ClassImpl.hpp`` would not have the main include file put on top
   before any other include.
 
+**IndentCaseBlocks** (``bool``)
+  Indent case label blocks one level from the case label.
+
+  When ``false``, the block following the case label uses the same
+  indentation level as for the case label, treating the case label the same
+  as an if-statement.
+  When ``true``, the block gets indented as a scope block.
+
+  .. code-block:: c++
+
+     false:                                 true:
+     switch (fool) {                vs.     switch (fool) {
+     case 1: {                              case 1:
+       bar();                                 {
+     } break;                                   bar();
+     default: {                               }
+       plop();                                break;
+     }                                      default:
+     }                                        {
+                                                plop();
+                                              }
+                                            }
+
 **IndentCaseLabels** (``bool``)
   Indent case labels one level from the switch statement.
 
   When ``false``, use the same indentation level as for the switch
   statement. Switch statement body is always indented one level more than
-  case labels.
+  case labels (except the first block following the case label, which
+  itself indents the code - unless IndentCaseBlocks is enabled).
 
   .. code-block:: c++
 
@@ -2329,7 +2353,14 @@ the configuration (without a prefix: ``Auto``).
      x = ( int32 )y                 vs.     x = (int32)y
 
 **SpacesInConditionalStatement** (``bool``)
-  If ``true``, spaces will be inserted around if/for/while (and similar) conditions.
+  If ``true``, spaces will be inserted around if/for/switch/while
+  conditions.
+
+  .. code-block:: c++
+
+     true:                                  false:
+     if ( a )  { ... }              vs.     if (a) { ... }
+     while ( i < 5 )  { ... }               while (i < 5) { ... }
 
 **SpacesInContainerLiterals** (``bool``)
   If ``true``, spaces are inserted inside container literals (e.g.

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 35132cecee57..76a1d748b604 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -177,6 +177,25 @@ clang-format
 ------------
 
 
+- Option ``IndentCaseBlocks`` has been added to support treating the block
+  following a switch case label as a scope block which gets indented itself.
+  It helps avoid having the closing bracket align with the switch statement's
+  closing bracket (when ``IndentCaseLabels`` is ``false``).
+
+  .. code-block:: c++
+  
+    switch (fool) {                vs.     switch (fool) {
+    case 1:                                case 1: {
+      {                                      bar();
+         bar();                            } break;
+      }                                    default: {
+      break;                                 plop();
+    default:                               }
+      {                                    }
+        plop();
+      }
+    }
+
 libclang
 --------
 

diff  --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index add2937f3b43..09a0556ab4c6 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1314,7 +1314,8 @@ struct FormatStyle {
   ///
   /// When ``false``, use the same indentation level as for the switch
   /// statement. Switch statement body is always indented one level more than
-  /// case labels.
+  /// case labels (except the first block following the case label, which
+  /// itself indents the code - unless IndentCaseBlocks is enabled).
   /// \code
   ///    false:                                 true:
   ///    switch (fool) {                vs.     switch (fool) {
@@ -1327,6 +1328,28 @@ struct FormatStyle {
   /// \endcode
   bool IndentCaseLabels;
 
+  /// Indent case label blocks one level from the case label.
+  ///
+  /// When ``false``, the block following the case label uses the same
+  /// indentation level as for the case label, treating the case label the same
+  /// as an if-statement.
+  /// When ``true``, the block gets indented as a scope block.
+  /// \code
+  ///    false:                                 true:
+  ///    switch (fool) {                vs.     switch (fool) {
+  ///    case 1: {                              case 1:
+  ///      bar();                                 {
+  ///    } break;                                   bar();
+  ///    default: {                               }
+  ///      plop();                                break;
+  ///    }                                      default:
+  ///    }                                        {
+  ///                                               plop();
+  ///                                             }
+  ///                                           }
+  /// \endcode
+  bool IndentCaseBlocks;
+
   /// Indent goto labels.
   ///
   /// When ``false``, goto labels are flushed left.
@@ -2119,6 +2142,7 @@ struct FormatStyle {
            IncludeStyle.IncludeIsMainSourceRegex ==
                R.IncludeStyle.IncludeIsMainSourceRegex &&
            IndentCaseLabels == R.IndentCaseLabels &&
+           IndentCaseBlocks == R.IndentCaseBlocks &&
            IndentGotoLabels == R.IndentGotoLabels &&
            IndentPPDirectives == R.IndentPPDirectives &&
            IndentWidth == R.IndentWidth && Language == R.Language &&

diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index cce9458cf22f..a962e32f66b0 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -480,6 +480,7 @@ template <> struct MappingTraits<FormatStyle> {
     IO.mapOptional("IncludeIsMainSourceRegex",
                    Style.IncludeStyle.IncludeIsMainSourceRegex);
     IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
+    IO.mapOptional("IndentCaseBlocks", Style.IndentCaseBlocks);
     IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
     IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
     IO.mapOptional("IndentWidth", Style.IndentWidth);
@@ -782,6 +783,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
   LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
   LLVMStyle.IndentCaseLabels = false;
+  LLVMStyle.IndentCaseBlocks = false;
   LLVMStyle.IndentGotoLabels = true;
   LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
   LLVMStyle.IndentWrappedFunctionNames = false;

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 18b4cc5306f5..321a7405e24a 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2003,7 +2003,8 @@ void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
     --Line->Level;
   if (LeftAlignLabel)
     Line->Level = 0;
-  if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
+  if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
+      FormatTok->Tok.is(tok::l_brace)) {
     CompoundStatementIndenter Indenter(this, Line->Level,
                                        Style.BraceWrapping.AfterCaseLabel,
                                        Style.BraceWrapping.IndentBraces);

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index d689658f99b6..43e75dde6806 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -1211,6 +1211,58 @@ TEST_F(FormatTest, FormatsSwitchStatement) {
                    "  }\n"
                    "}",
                    Style));
+  Style.IndentCaseLabels = false;
+  Style.IndentCaseBlocks = true;
+  EXPECT_EQ("switch (n)\n"
+            "{\n"
+            "case 0:\n"
+            "  {\n"
+            "    return false;\n"
+            "  }\n"
+            "case 1:\n"
+            "  break;\n"
+            "default:\n"
+            "  {\n"
+            "    return true;\n"
+            "  }\n"
+            "}",
+            format("switch (n) {\n"
+                   "case 0: {\n"
+                   "  return false;\n"
+                   "}\n"
+                   "case 1:\n"
+                   "  break;\n"
+                   "default: {\n"
+                   "  return true;\n"
+                   "}\n"
+                   "}",
+                   Style));
+  Style.IndentCaseLabels = true;
+  Style.IndentCaseBlocks = true;
+  EXPECT_EQ("switch (n)\n"
+            "{\n"
+            "  case 0:\n"
+            "    {\n"
+            "      return false;\n"
+            "    }\n"
+            "  case 1:\n"
+            "    break;\n"
+            "  default:\n"
+            "    {\n"
+            "      return true;\n"
+            "    }\n"
+            "}",
+            format("switch (n) {\n"
+                   "case 0: {\n"
+                   "  return false;\n"
+                   "}\n"
+                   "case 1:\n"
+                   "  break;\n"
+                   "default: {\n"
+                   "  return true;\n"
+                   "}\n"
+                   "}",
+                   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -12578,6 +12630,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
   CHECK_PARSE_BOOL(IndentCaseLabels);
+  CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);


        


More information about the cfe-commits mailing list