This is an archive of the discontinued LLVM Phabricator instance.

Differential D82016

[clang-format] [PR462254] fix indentation of default and break correctly in whitesmiths style
ClosedPublic

Authored by MyDeveloperDay on Jun 17 2020, 7:54 AM.

Details

Reviewers
JakeMerdichAMD
curdeius
jbcoe
timwoj
Summary

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

In ‘clang-format’, when using "BreakBeforeBraces: Whitesmiths".
In a ‘switch’ block, the ‘default’ case is always missing and indentation level. See below.

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

   case 1:
      {
      goo(x % 42);
      }
   break;

default:
      {
      printf("the default case");
      }
   break;
   }

Diff Detail

Revision Contents

PathSize
clang/
lib/
Format/
3 lines
7 lines
unittests/
Format/
90 lines

Diff 271373

clang/lib/Format/UnwrappedLineFormatter.cpp

Show First 20 Lines • Show All 1,239 Lines • ▼ Show 20 Lines if (PreviousLine && PreviousLine->First->isAccessSpecifier() &&
(!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline)) (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline))
Newlines = std::min(1u, Newlines); Newlines = std::min(1u, Newlines);
if (Newlines) if (Newlines)
Indent = NewlineIndent; Indent = NewlineIndent;
// If in Whitemsmiths mode, indent start and end of blocks // If in Whitemsmiths mode, indent start and end of blocks
if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) { if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) {
if (RootToken.isOneOf(tok::l_brace, tok::r_brace, tok::kw_case)) if (RootToken.isOneOf(tok::l_brace, tok::r_brace, tok::kw_case,
tok::kw_default))
Indent += Style.IndentWidth; Indent += Style.IndentWidth;
} }
// Preprocessor directives get indented before the hash only if specified // Preprocessor directives get indented before the hash only if specified
if (Style.IndentPPDirectives != FormatStyle::PPDIS_BeforeHash && if (Style.IndentPPDirectives != FormatStyle::PPDIS_BeforeHash &&
(Line.Type == LT_PreprocessorDirective || (Line.Type == LT_PreprocessorDirective ||
Line.Type == LT_ImportStatement)) Line.Type == LT_ImportStatement))
Indent = 0; Indent = 0;
Show All 26 Lines

clang/lib/Format/UnwrappedLineParser.cpp

Show First 20 Lines • Show All 2,208 Lines • ▼ Show 20 Linesvoid UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() && if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
FormatTok->Tok.is(tok::l_brace)) { FormatTok->Tok.is(tok::l_brace)) {
CompoundStatementIndenter Indenter(this, Line->Level, CompoundStatementIndenter Indenter(this, Line->Level,
Style.BraceWrapping.AfterCaseLabel, Style.BraceWrapping.AfterCaseLabel,
Style.BraceWrapping.IndentBraces); Style.BraceWrapping.IndentBraces);
parseBlock(/*MustBeDeclaration=*/false); parseBlock(/*MustBeDeclaration=*/false);
if (FormatTok->Tok.is(tok::kw_break)) { if (FormatTok->Tok.is(tok::kw_break)) {
if (Style.BraceWrapping.AfterControlStatement == if (Style.BraceWrapping.AfterControlStatement ==
FormatStyle::BWACS_Always) FormatStyle::BWACS_Always) {
addUnwrappedLine(); addUnwrappedLine();
if (!Style.IndentCaseBlocks &&
Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) {
Line->Level++;
}
}
parseStructuralElement(); parseStructuralElement();
} }
addUnwrappedLine(); addUnwrappedLine();
} else { } else {
if (FormatTok->is(tok::semi)) if (FormatTok->is(tok::semi))
nextToken(); nextToken();
addUnwrappedLine(); addUnwrappedLine();
} }
▲ Show 20 Lines • Show All 834 Lines • Show Last 20 Lines

clang/unittests/Format/FormatTest.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 12,935 Lines • ▼ Show 20 Lines verifyFormat("void f()\n"
" }\n" " }\n"
" do\n" " do\n"
" {\n" " {\n"
" c();\n" " c();\n"
" } while (false)\n" " } while (false)\n"
" }\n", " }\n",
WhitesmithsBraceStyle); WhitesmithsBraceStyle);
// FIXME: the block and the break under case 2 in this test don't get indented WhitesmithsBraceStyle.IndentCaseBlocks = true;
// correctly
/*
verifyFormat("void switchTest1(int a)\n" verifyFormat("void switchTest1(int a)\n"
" {\n" " {\n"
" switch (a)\n" " switch (a)\n"
" {\n" " {\n"
" case 2:\n" " case 2:\n"
" {\n" " {\n"
" }\n" " }\n"
" break;\n" " break;\n"
" }\n" " }\n"
" }\n", " }\n",
WhitesmithsBraceStyle); WhitesmithsBraceStyle);
*/
// FIXME: the block and the break under case 2 in this test don't get indented
// correctly
/*
verifyFormat("void switchTest2(int a)\n" verifyFormat("void switchTest2(int a)\n"
" {\n" " {\n"
" switch (a)\n" " switch (a)\n"
" {\n" " {\n"
" case 0:\n" " case 0:\n"
" break;\n" " break;\n"
" case 1:\n" " case 1:\n"
" {\n" " {\n"
" break;\n" " break;\n"
" }\n" " }\n"
" case 2:\n" " case 2:\n"
" {\n" " {\n"
" }\n" " }\n"
" break;\n" " break;\n"
" default:\n" " default:\n"
" break;\n" " break;\n"
" }\n" " }\n"
" }\n", " }\n",
WhitesmithsBraceStyle); WhitesmithsBraceStyle);
*/
verifyFormat("void switchTest3(int a)\n"
" {\n"
" switch (a)\n"
" {\n"
" case 0:\n"
" {\n"
" foo(x);\n"
" }\n"
" break;\n"
" default:\n"
" {\n"
" foo(1);\n"
" }\n"
" break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
WhitesmithsBraceStyle.IndentCaseBlocks = false;
verifyFormat("void switchTest4(int a)\n"
" {\n"
" switch (a)\n"
" {\n"
" case 2:\n"
" {\n"
" }\n"
" break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
verifyFormat("void switchTest5(int a)\n"
" {\n"
" switch (a)\n"
" {\n"
" case 0:\n"
" break;\n"
" case 1:\n"
" {\n"
" foo();\n"
" break;\n"
" }\n"
" case 2:\n"
" {\n"
" }\n"
" break;\n"
" default:\n"
" break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
verifyFormat("void switchTest6(int a)\n"
" {\n"
" switch (a)\n"
" {\n"
" case 0:\n"
" {\n"
" foo(x);\n"
" }\n"
" break;\n"
" default:\n"
" {\n"
" foo(1);\n"
" }\n"
" break;\n"
" }\n"
" }\n",
WhitesmithsBraceStyle);
verifyFormat("enum X\n" verifyFormat("enum X\n"
" {\n" " {\n"
" Y = 0, // testing\n" " Y = 0, // testing\n"
" }\n", " }\n",
WhitesmithsBraceStyle); WhitesmithsBraceStyle);
verifyFormat("enum X\n" verifyFormat("enum X\n"
▲ Show 20 Lines • Show All 3,587 Lines • Show Last 20 Lines