clang-format: introduce `CaseBlockIndent` to control indent in switch

[Typz]

Summary

When a block is started after a case label, clang-format does add extra
indent to the content of this block: the block content is indented one
level (with respect to the switch) while the closing brace is not
indented, and closes at switch level.

This gives the following code:

switch (x) {
case A: {
  stuff();
} break;
}

This makes it easy to confuse the closing brace of the 'case' with that
the whole 'switch', especially if more code is added after the brace:

switch (x) {
case A: {
  stuff();
}
  moreStuff();
  break;
}

This patch introduces a new CaseBlockIndent switch, which provides
alternative formatting for these cases:

• CaseBlockIndent = None : default behavior, same behavior as before

• CaseBlockIndent = ClosingBrace : indent the closing brace to the

same level as its content.

switch (x) {
case A: {
  stuff();
  } break;
}

• CaseBlockIndent = Block : add an extra level of indent for the

content of the block.

switch (x) {
case A: {
    stuff();
  } break;
}

[djasper]

Do you have a reference to style guides recommending any of this?

[Typz]

Do you have a reference to style guides recommending any of this?

Unfortunately not... I think this is a really advanced topic, and often not recommended way to format code at all (e.g. "if you need a block, you may as well use a function"). I can find the current implementation documented in google style guide, where there is an extra indent inside switch [e.g. IndentCaseLabels], which alleviates the ambiguity. But it is not documented (or shown) in the LLVM coding style, where it makes the code IMO really difficult to follow.

I believe the 2 extra modes I introduce should provide for most cases, leaving the decision to user:

• None mode matches google-style, and is perfectly fine when IndentCaseLabels == true
• ClosingBrace mode is "logical" with what people may want (e.g. have a scope for the whole case block), but is not syntaxically correct and may thus mislead
• Block mode is syntaxically correct, but somewhat breaks the switch layout by moving break to different indentation levels depending on the use of braces

Which mode is use is then left to the user, depending on the "goals" of their coding style.

Also, maybe this is a small-enough detail that it would be better moved to the BraceWrapping field, to allow finely tuning the expected behavior (e.g. in Custom brace mode) while keeping the 'main' options simple (even though it is not technically a way to wrap the braces).

[djasper]

To me none of these options make sense. For the case you are describing, I agree that the current behavior is not ideal, but neither are any of the alternatives. However, I think that's fine. We'll just format those cases in a somewhat weird way and users can either accept that or change their code to not need it. I don't particularly care which of these options we go with, but I would be really hesitant to introduce an style flag for it. This is so rare, that almost no one needs this flag (or should need this flag). Thus, the cost of the flag (discoverability of flags for users, increased maintenance cost, etc.) strongly outweigh the benefit. IMO, for the same reason, this specific issue will not become part of the style guide of projects.

If you want to change something, I'd vote for making clang fall back to this behavior:

case A:
  {
    stuff();
  }
  moreStuff();
  break;
}

i.e. not let it put the "{" on the same line as the "case ..." if there is a trailing statement (other than "break;" maybe).

[Typz]

We'll just format those cases in a somewhat weird way and users can either accept that or change their code to not need it.

I think we have a really diverging opinion on this. From my experience, people will mostly accept the output of the formatter without question : therefore I think we should strive to have the formatter format things as "correctly" as possible. And looking at the existing options and various complex formatting algorithms implemented I think this reasoning has been applied to some extent :-)
So I personnally would be willing to add some code/options even for such kind of corner cases; but then I am not the one maintaining the clang-format project.

I don't particularly care which of these options we go with, but I would be really hesitant to introduce an style flag for it. This is so rare, that almost no one needs this flag (or should need this flag). Thus, the cost of the flag (discoverability of flags for users, increased maintenance cost, etc.) strongly outweigh the benefit.

Any change will still require a new flag somewhere, since the currently implemented behavior is :
1- the documented way to format in Google style
2- the expected format in LLVM style, according to the clang-format unit test

It should thus probably not be changed.

IMO, for the same reason, this specific issue will not become part of the style guide of projects.

I definitely agree on this, but it is actually part of some styles (e.g. Google's)

If you want to change something, I'd vote for making clang fall back to this behavior:

case A:
  {
    stuff();
  }
  moreStuff();
  break;
}

i.e. not let it put the "{" on the same line as the "case ..." if there is a trailing statement (other than "break;" maybe).

I am fine with that formatting (though I did not implement it since it requires tweaking the code in more places, instead of essentially modifying a single function like I did).

[djasper]

In D43183#1006170, @Typz wrote:

We'll just format those cases in a somewhat weird way and users can either accept that or change their code to not need it.

I think we have a really diverging opinion on this. From my experience, people will mostly accept the output of the formatter without question : therefore I think we should strive to have the formatter format things as "correctly" as possible. And looking at the existing options and various complex formatting algorithms implemented I think this reasoning has been applied to some extent :-)
So I personnally would be willing to add some code/options even for such kind of corner cases; but then I am not the one maintaining the clang-format project.

I have no problem with making clang-format format things "correctly" and I am happy to add code. But I think we are doing the average clang-format user a disservice if we provide options for every such corner case. Let's settle on one good-enough behavior and go with that for everything/everyone.

I don't particularly care which of these options we go with, but I would be really hesitant to introduce an style flag for it. This is so rare, that almost no one needs this flag (or should need this flag). Thus, the cost of the flag (discoverability of flags for users, increased maintenance cost, etc.) strongly outweigh the benefit.

Any change will still require a new flag somewhere, since the currently implemented behavior is :
1- the documented way to format in Google style
2- the expected format in LLVM style, according to the clang-format unit test

It should thus probably not be changed.

I don't think that's true for the alternative I am suggesting.

IMO, for the same reason, this specific issue will not become part of the style guide of projects.

I definitely agree on this, but it is actually part of some styles (e.g. Google's)

If you want to change something, I'd vote for making clang fall back to this behavior:

case A:
  {
    stuff();
  }
  moreStuff();
  break;
}

i.e. not let it put the "{" on the same line as the "case ..." if there is a trailing statement (other than "break;" maybe).

I am fine with that formatting (though I did not implement it since it requires tweaking the code in more places, instead of essentially modifying a single function like I did).

As we can implement this without additional flags (it doesn't contradict any style guide I know of), I think this would be strictly preferable.

[Typz]

It is explicitly documented in google style guide: https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements :

case blocks in switch statements can have curly braces or not, depending on your preference. If you do include curly braces they should be placed as shown below.

If not conditional on an enumerated value, switch statements should always have a default case (in the case of an enumerated value, the compiler will warn you if any values are not handled). If the default case should never execute, simply assert:

switch (var) {

case 0: {  // 2 space indent
  ...      // 4 space indent
  break;
}
case 1: {
  ...
  break;
}
default: {
  assert(false);
}

}

So IMHO we cannot just change the current (or default) behaviour.

[djasper]

In D43183#1006224, @Typz wrote:

It is explicitly documented in google style guide: https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements :

case blocks in switch statements can have curly braces or not, depending on your preference. If you do include curly braces they should be placed as shown below.

If not conditional on an enumerated value, switch statements should always have a default case (in the case of an enumerated value, the compiler will warn you if any values are not handled). If the default case should never execute, simply assert:

switch (var) {

case 0: {  // 2 space indent
  ...      // 4 space indent
  break;
}
case 1: {
  ...
  break;
}
default: {
  assert(false);
}

}

So IMHO we cannot just change the current (or default) behaviour.

My proposal does not contradict this style guide as the case in question is not included in the example. It gives absolutely no guidance on how to format this case.

If anything, you could argue that it tells the user never to have something outside of the braces that make up a case statement (keep in mind that the style guide does not only give formatting advise).

[Typz]

Hum, not sure I fully got your proposal. So you actually mean that clang-format you format like this (with 4-space indent for clarity):

switch (x) {
case 0:
    break;
case 1: {
    foo();
    break;
}
case 2: {
    foo();
} break;
case 3:
    {
        foo();
    }
    bar();
    break;
}

Is this right?

If so it does not really help me: I don't care so much how it is formatted, but I think the current way is way too error prone (and I cannot change the style to indent the case blocks themself). So i'll have to keep a patch in my fork :-(
Or maybe the behavior should be dependant on IndentCaseLabels (though this would change LLVM style formatting) ?

[djasper]

Yes, that's what I mean. What do you mean, the style is too error prone?

[Typz]

In D43183#1008632, @djasper wrote:

Yes, that's what I mean. What do you mean, the style is too error prone?

When IndentCaseLabels is false, the code gets formatted in a way that "hides" the structure of the code, by indenting the end of the case in the exact same way as the end of a switch, which is error prone IMHO.
Indentation should "highlight" the structure of the code, to make it easier to understand, and thus avoid this kind of confusion.

[djasper]

That doesn't explain to me how this is error prone. I can't think how you'd create an error by this that would not be caught by the compiler. Much less if you consistently use clang-format.

It's fundamentally what you get for IndentCaseLabels: false. Even without braces you have this issue to some extend. But LLVM has several thousand switches, about a thousand with braces formatted this way and I am not aware of a single time this has caused a bug or even confusion.

And to me (but this is obviously objective), your suggestions hide the structure of the code even more as they lead to a state where the closing brace is not aligned with the start of the line/statement that contains the opening braces. That looks like a bug to me more than anything else and I don't think there is another situation where clang-format would do that.

[Typz]

A user can create an error by reasoning like this, after the code has been formatted this way (a long time ago) : "oh, I need to make an extra function call, but in all cases.... ah, here is the end of the switch, let's put my function call here".

I am not saying it happens often, I am just saying it breaks indentation : i.e. that two nested blocks should not close at the same depth. Breaking such things makes code harder to read/understand, and when you don't properly get the code you can more easily make a mistake. Obviously it should be caught in testing, but it is not always.

That said, I am not trying to say in any way that my approach is better. I think both CaseBlockIndent = Block or your variant (with the extra line break before opening brace; but applying it all the time) will indeed be consistent with the code and not break indentation; keeping the current behavior when IndentCaseLabels = true is also not a problem IMHO.

And to me (but this is obviously objective), your suggestions hide the structure of the code even more as they lead to a state where the closing brace is not aligned with the start of the line/statement that contains the opening braces. That looks like a bug to me more than anything else and I don't think there is another situation where clang-format would do that.

The exact same thing happens for functions blocks, class blocks and control statements, depending on BraceWrapping modes. Actually, IMO wrapping the opening brace should probably be done according to BraceWrapping.AfterControlStatement flag.

// BraceWrapping.AfterControlStatement = false
switch (x) {
case 0: {
    foo();
    break;
  }
}
// BraceWrapping.AfterControlStatement = true
switch (x)
{
case 0:
  {
    foo();
    break;
  }
}

But I agree the CaseBlockIndent = ClosingBrace mode is definitely not consistent with the code. I think it is clearer this way, but that is definitely my own subjective opinion: in this case, I accept to lose the extra indent for the sake of compactness and to somehow mimic the "regular" case blocks (e.g. which do not include an actual block), but that is indeed really my personnal way to read code.

[djasper]

In D43183#1008784, @Typz wrote:

A user can create an error by reasoning like this, after the code has been formatted this way (a long time ago) : "oh, I need to make an extra function call, but in all cases.... ah, here is the end of the switch, let's put my function call here".

And then trying to format it with clang-format they'll be immediately thrown off because clang-format gets the indentation wrong :).
But I don't want to argue this to death. I understand what you are saying. I just don't think any of your suggested formats make this situation better.

I am not saying it happens often, I am just saying it breaks indentation : i.e. that two nested blocks should not close at the same depth. Breaking such things makes code harder to read/understand, and when you don't properly get the code you can more easily make a mistake. Obviously it should be caught in testing, but it is not always.

That said, I am not trying to say in any way that my approach is better. I think both CaseBlockIndent = Block or your variant (with the extra line break before opening brace; but applying it all the time) will indeed be consistent with the code and not break indentation; keeping the current behavior when IndentCaseLabels = true is also not a problem IMHO.

An extra thing to consider is that my approach is also consistent with having something before this block:

case A:
  {
    f();
    g();
  }
  h();
  break;
case B:
  f();
  {
    g();
  }
  h();
  break;

And to me (but this is obviously objective), your suggestions hide the structure of the code even more as they lead to a state where the closing brace is not aligned with the start of the line/statement that contains the opening braces. That looks like a bug to me more than anything else and I don't think there is another situation where clang-format would do that.

The exact same thing happens for functions blocks, class blocks and control statements, depending on BraceWrapping modes. Actually, IMO wrapping the opening brace should probably be done according to BraceWrapping.AfterControlStatement flag.

// BraceWrapping.AfterControlStatement = false
switch (x) {
case 0: {
    foo();
    break;
  }
}
// BraceWrapping.AfterControlStatement = true
switch (x)
{
case 0:
  {
    foo();
    break;
  }
}

But I agree the CaseBlockIndent = ClosingBrace mode is definitely not consistent with the code. I think it is clearer this way, but that is definitely my own subjective opinion: in this case, I accept to lose the extra indent for the sake of compactness and to somehow mimic the "regular" case blocks (e.g. which do not include an actual block), but that is indeed really my personnal way to read code.

I don't agree that that's the same thing. The closing brace is still neatly aligned with the line of the opening brace (which happens to be just the opening brace).

[Typz]

I don't agree that that's the same thing. The closing brace is still neatly aligned with the line of the opening brace (which happens to be just the opening brace).

This invariant is not really applicable to switch statements, where code of each "branch" is already indented with no opening brace. :-)

I can try to change the mode so that we get either "google" style:

switch (x) {
case 0: {
  foo():
} break;
}

or "regular indentation" mode (e.g. what would look like no special indentation rule):

switch (x) {
case 0:
  {
    foo():
  }
  break;
}

Generally, would a flag for CaseBlockIndent be acceptable for this purpose ?
Or should the behavior simply be selected depending on IndentCaseLabels ?

[djasper]

New options for this would not be acceptable IMO. Too much cost for too little benefit.

I'd suggest to first make the change to fall back to the style with a regular block when there are statements other than break after the closing brace. That is always bad, no matter the indentation of the case labels:

switch (x) {
  case 1: {
    doSomething();
  }
    doSomethingElse();
    break;
}

Fixing this is good no matter what.

And then the second question is whether this style should be used or not (with IndentCaseLabels: false):

switch (x) {
case 1: {
  doSomething();
}
}

Pro: Saves horizontal and vertical space.
Con: It's weird to have to braces in the same column.

I don't personally have an opinion here, but I'll check with a few LLVM developers who work with LLVM style.

[djasper]

Got two responses so far.

1. Having to closing braces in the same column is weird, but not that weird. Consider

namespace n {
void f() {
  ...
}
}

2. Richard Smith (code owner of Clang) says that he doesn't really like the two closing braces in the same column, but he always cheats by removing the braces for the last case label / default. You never need them. In any case he'd strongly prefer the current behavior over adding an extra break and more indentation.

In conclusion, I don't think we want to move forward with making clang-format do

switch (x) {
case 1:
  {
    doSomething();
  }
}

even with IndentCaseLabels: false.

[Typz]

rebase

[Typz]

Regenerate documentation

docs/ClangFormatStyleOptions.rst

Constructor() :
         initializer1(),
         initializer2()
 
 
 
 **BreakStringLiterals** (``bool``)
   Allow breaking string literals when formatting.
 
+**CaseBlockIndent** (``CaseBlockIndentStyle``)
+  Control indentation of blocks immediately after case/default label
+  in switch statements.
+
+  Possible values:
+
+  * ``CBIS_None`` (in configuration: ``None``)
+    No extra indent, closing-brace in un-indented back to switch
+    level.
+
+    .. code-block:: c++
+
+       switch (a) {
+       case 0: {
+         f();
+       } break;
+       default:
+       }
+
+  * ``CBIS_ClosingBrace`` (in configuration: ``ClosingBrace``)
+    No extra indent, but closing-brace in indented at the same
+    level as the content of the block.
+
+    .. code-block:: c++
+
+       switch (a) {
+       case 0: {
+         f();
+         } break;
+       default:
+       }
+
+  * ``CBIS_Block`` (in configuration: ``Block``)
+    The block adds one extra level of indent.
+
+    .. code-block:: c++
+
+       switch (a) {
+       case 0: {
+           f();
+         } break;
+       default:
+       }
+
+
+
 **ColumnLimit** (``unsigned``)
   The column limit.
 
   A column limit of ``0`` means that there is no column limit. In this case,
   clang-format will respect the input's line breaking decisions within
   statements unless they contradict other rules.
 
 **CommentPragmas** (``std::string``)

include/clang/Format/Format.h

struct FormatStyle {
   ///    @Mock
   ///    DataLoad loader;
   /// \endcode
   bool BreakAfterJavaFieldAnnotations;
 
   /// Allow breaking string literals when formatting.
   bool BreakStringLiterals;
 
+  /// Different styles for formatting blocks after case/default label in switch
+  /// statements.
+  enum CaseBlockIndentStyle {
+    /// No extra indent, closing-brace in unindented back to switch level.
+    /// \code
+    ///    switch (a) {
+    ///    case 0: {
+    ///      f();
+    ///    } break;
+    ///    default:
+    ///    }
+    /// \endcode
+    CBIS_None,
+    /// No extra indent, but closing-brace in indented at the same level as
+    /// the content of the block.
+    /// \code
+    ///    switch (a) {
+    ///    case 0: {
+    ///      f();
+    ///      } break;
+    ///    default:
+    ///    }
+    /// \endcode
+    CBIS_ClosingBrace,
+    /// The block adds one extra level of indent.
+    /// \code
+    ///    switch (a) {
+    ///    case 0: {
+    ///        f();
+    ///      } break;
+    ///    default:
+    ///    }
+    /// \endcode
+    CBIS_Block
+  };
+
+  /// Control indentation of blocks immediately after case/default label in
+  /// switch statements.
+  CaseBlockIndentStyle CaseBlockIndent;
+
   /// The column limit.
   ///
   /// A column limit of ``0`` means that there is no column limit. In this case,
   /// clang-format will respect the input's line breaking decisions within
   /// statements unless they contradict other rules.
   unsigned ColumnLimit;
 
   /// A regular expression that describes comments with special meaning,
return AccessModifierOffset == R.AccessModifierOffset &&
            BinPackParameters == R.BinPackParameters &&
            BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators &&
            BreakBeforeBraces == R.BreakBeforeBraces &&
            BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
            BreakConstructorInitializers == R.BreakConstructorInitializers &&
            CompactNamespaces == R.CompactNamespaces &&
            BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations &&
            BreakStringLiterals == R.BreakStringLiterals &&
+           CaseBlockIndent == R.CaseBlockIndent &&
            ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
            BreakBeforeInheritanceComma == R.BreakBeforeInheritanceComma &&
            ConstructorInitializerAllOnOneLineOrOnePerLine ==
                R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
            ConstructorInitializerIndentWidth ==
                R.ConstructorInitializerIndentWidth &&
            ContinuationIndentWidth == R.ContinuationIndentWidth &&
            Cpp11BracedListStyle == R.Cpp11BracedListStyle &&

lib/Format/Format.cpp

static void enumeration(IO &IO, FormatStyle::BreakTemplateDeclarationsStyle &Value) {
     IO.enumCase(Value, "Yes", FormatStyle::BTDS_Yes);
 
     // For backward compatibility.
     IO.enumCase(Value, "false", FormatStyle::BTDS_MultiLine);
     IO.enumCase(Value, "true", FormatStyle::BTDS_Yes);
   }
 };
 
-template <>
+template<>
+struct ScalarEnumerationTraits<FormatStyle::CaseBlockIndentStyle> {
+  static void
+  enumeration(IO &IO, FormatStyle::CaseBlockIndentStyle &Value) {
+    IO.enumCase(Value, "None", FormatStyle::CBIS_None);
+    IO.enumCase(Value, "ClosingBrace", FormatStyle::CBIS_ClosingBrace);
+    IO.enumCase(Value, "Block", FormatStyle::CBIS_Block);
+  }
+};
+
+template <>
 struct ScalarEnumerationTraits<FormatStyle::DefinitionReturnTypeBreakingStyle> {
   static void
   enumeration(IO &IO, FormatStyle::DefinitionReturnTypeBreakingStyle &Value) {
     IO.enumCase(Value, "None", FormatStyle::DRTBS_None);
     IO.enumCase(Value, "All", FormatStyle::DRTBS_All);
     IO.enumCase(Value, "TopLevel", FormatStyle::DRTBS_TopLevel);
 
     // For backward compatibility.
static void mapping(IO &IO, FormatStyle &Style) {
     // former for backwards compatibility.
     if (BreakConstructorInitializersBeforeComma &&
         Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeColon)
       Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
 
     IO.mapOptional("BreakAfterJavaFieldAnnotations",
                    Style.BreakAfterJavaFieldAnnotations);
     IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
+    IO.mapOptional("CaseBlockIndent", Style.CaseBlockIndent);
     IO.mapOptional("ColumnLimit", Style.ColumnLimit);
     IO.mapOptional("CommentPragmas", Style.CommentPragmas);
     IO.mapOptional("CompactNamespaces", Style.CompactNamespaces);
     IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
                    Style.ConstructorInitializerAllOnOneLineOrOnePerLine);
     IO.mapOptional("ConstructorInitializerIndentWidth",
                    Style.ConstructorInitializerIndentWidth);
     IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
FormatStyle getLLVMStyle() {
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BraceWrapping = {false, false, false, false, false,
                              false, false, false, false, false,
                              false, false, true,  true,  true};
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakBeforeInheritanceComma = false;
   LLVMStyle.BreakStringLiterals = true;
+  LLVMStyle.CaseBlockIndent = FormatStyle::CBIS_None;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
   LLVMStyle.CompactNamespaces = false;
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
   LLVMStyle.ConstructorInitializerIndentWidth = 4;
   LLVMStyle.ContinuationIndentWidth = 4;
   LLVMStyle.Cpp11BracedListStyle = true;
   LLVMStyle.DerivePointerAlignment = false;

lib/Format/UnwrappedLineParser.h

public:
 
   void parse();
 
 private:
   void reset();
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
+                  bool MunchSemi = true) {
+    parseBlock(MustBeDeclaration, AddLevel ? 1 : 0, MunchSemi);
+  }
+  void parseBlock(bool MustBeDeclaration, int AddLevel,
                   bool MunchSemi = true);
   void parseChildBlock();
   void parsePPDirective();
   void parsePPDefine();
   void parsePPIf(bool IfDef);
   void parsePPElIf();
   void parsePPElse();
   void parsePPEndIf();

lib/Format/UnwrappedLineParser.cpp

size_t UnwrappedLineParser::computePPHash() const {
   size_t h = 0;
   for (const auto &i : PPStack) {
     hash_combine(h, size_t(i.Kind));
     hash_combine(h, i.Line);
   }
   return h;
 }
 
-void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
+void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, int AddLevel,
                                      bool MunchSemi) {
   assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
          "'{' or macro block token expected");
   const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
   FormatTok->BlockKind = BK_Block;
 
   size_t PPStartHash = computePPHash();
 
   unsigned InitialLevel = Line->Level;
-  nextToken(/*LevelDifference=*/AddLevel ? 1 : 0);
+  nextToken(/*LevelDifference=*/AddLevel);
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
     parseParens();
 
   size_t NbPreprocessorDirectives =
       CurrentLines == &Lines ? PreprocessorDirectives.size() : 0;
   addUnwrappedLine();
   size_t OpeningLineIndex =
       CurrentLines->empty()
           ? (UnwrappedLine::kInvalidIndex)
           : (CurrentLines->size() - 1 - NbPreprocessorDirectives);
 
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
                                           MustBeDeclaration);
-  if (AddLevel)
-    ++Line->Level;
+  Line->Level += static_cast<unsigned>(AddLevel);
   parseLevel(/*HasOpeningBrace=*/true);
 
   if (eof())
     return;
 
   if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
                  : !FormatTok->is(tok::r_brace)) {
     Line->Level = InitialLevel;
     FormatTok->BlockKind = BK_Block;
     return;
   }
 
   size_t PPEndHash = computePPHash();
 
   // Munch the closing brace.
-  nextToken(/*LevelDifference=*/AddLevel ? -1 : 0);
+  nextToken(/*LevelDifference=*/-AddLevel);
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
     parseParens();
 
   if (MunchSemi && FormatTok->Tok.is(tok::semi))
     nextToken();
   Line->Level = InitialLevel;
 

 
 void UnwrappedLineParser::parseLabel() {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
     --Line->Level;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
-    parseBlock(/*MustBeDeclaration=*/false);
+    parseBlock(/*MustBeDeclaration=*/false,
+               /*AddLevel=*/Style.CaseBlockIndent == FormatStyle::CBIS_Block
+                   ? 2 : 1);
+    if (Style.CaseBlockIndent != FormatStyle::CBIS_None)
+        Line->Level++;
     if (FormatTok->Tok.is(tok::kw_break)) {
       if (Style.BraceWrapping.AfterControlStatement)
         addUnwrappedLine();
       parseStructuralElement();
     }
     addUnwrappedLine();
   } else {
     if (FormatTok->is(tok::semi))

unittests/Format/FormatTest.cpp

verifyFormat("switch (a) {\n"
                "}");
 
   verifyFormat("switch (a) {\n"
                "case some_namespace::\n"
                "    some_constant:\n"
                "  return;\n"
                "}",
                getLLVMStyleWithColumns(34));
+
+  FormatStyle indentClosingBrace = getLLVMStyle();
+  indentClosingBrace.CaseBlockIndent = FormatStyle::CBIS_ClosingBrace;
+  verifyFormat("switch (x) {\n"
+               "case 1:\n"
+               "  f();\n"
+               "  break;\n"
+               "}",
+               indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+               "case 1: {\n"
+               "  f();\n"
+               "  break;\n"
+               "  }\n"
+               "case 2: {\n"
+               "  break;\n"
+               "  }\n"
+               "}",
+               indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+               "case 1: {\n"
+               "  f();\n"
+               "  } break;\n"
+               "case 2: {\n"
+               "  } break;\n"
+               "}",
+               indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+               "case 1: {\n"
+               "  f();\n"
+               "  }\n"
+               "  g();\n"
+               "  break;\n"
+               "}",
+               indentClosingBrace);
+  verifyFormat("switch (x) {\n"
+               "case 1:\n"
+               "  f();\n"
+               "  {\n"
+               "    g();\n"
+               "    h();\n"
+               "  }\n"
+               "  break;\n"
+               "}",
+               indentClosingBrace);
+
+  FormatStyle indentBlock = getLLVMStyle();
+  indentBlock.CaseBlockIndent = FormatStyle::CBIS_Block;
+  verifyFormat("switch (x) {\n"
+               "case 1:\n"
+               "  f();\n"
+               "  break;\n"
+               "}",
+               indentBlock);
+  verifyFormat("switch (x) {\n"
+               "case 1: {\n"
+               "    f();\n"
+               "    break;\n"
+               "  }\n"
+               "case 2: {\n"
+               "    break;\n"
+               "  }\n"
+               "}",
+               indentBlock);
+  verifyFormat("switch (x) {\n"
+               "case 1: {\n"
+               "    f();\n"
+               "  } break;\n"
+               "case 2: {\n"
+               "  } break;\n"
+               "}",
+               indentBlock);
+  verifyFormat("switch (x) {\n"
+               "case 1: {\n"
+               "    f();\n"
+               "  }\n"
+               "  g();\n"
+               "  break;\n"
+               "}",
+               indentBlock);
+  verifyFormat("switch (x) {\n"
+               "case 1:\n"
+               "  f();\n"
+               "  {\n"
+               "    g();\n"
+               "    h();\n"
+               "  }\n"
+               "  break;\n"
+               "}",
+               indentBlock);
 }
 
 TEST_F(FormatTest, CaseRanges) {
   verifyFormat("switch (x) {\n"
                "case 'A' ... 'Z':\n"
                "case 1 ... 5:\n"
                "case a ... b:\n"
                "  break;\n"
TEST_F(FormatTest, ParsesConfiguration) {
   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.CaseBlockIndent = FormatStyle::CBIS_Block;
+  CHECK_PARSE("CaseBlockIndent: None", CaseBlockIndent, FormatStyle::CBIS_None);
+  CHECK_PARSE("CaseBlockIndent: ClosingBrace", CaseBlockIndent,
+              FormatStyle::CBIS_ClosingBrace);
+  CHECK_PARSE("CaseBlockIndent: Block", CaseBlockIndent,
+              FormatStyle::CBIS_Block);
+
   Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
   CHECK_PARSE("SpaceBeforeParens: Never", SpaceBeforeParens,
               FormatStyle::SBPO_Never);
   CHECK_PARSE("SpaceBeforeParens: Always", SpaceBeforeParens,
               FormatStyle::SBPO_Always);
   CHECK_PARSE("SpaceBeforeParens: ControlStatements", SpaceBeforeParens,
               FormatStyle::SBPO_ControlStatements);
   // For backward compatibility: