[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

[Nicolas Capens via Phabricator via cfe-commits]

capn created this revision.
capn added a reviewer: clang-format.
capn added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72276

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1211,6 +1211,28 @@
"  }\n"
"}",
Style));
+  Style.IndentCaseLabels = false;
+  Style.IndentCaseBlocks = true;
+  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"
+   "  return true;\n"
+   "}\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -12578,6 +12600,7 @@
   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);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1985,7 +1985,8 @@
 --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);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -480,6 +480,7 @@
 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 @@
   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;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1314,7 +1314,8 @@
   ///
   /// 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 @@
   /// \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

[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

[Nicolas Capens via Phabricator via cfe-commits]

capn updated this revision to Diff 236690.
capn added a comment.

Added release note update and test for both case labels and blocks being 
indented.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72276/new/

https://reviews.llvm.org/D72276

Files:
  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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1211,6 +1211,50 @@
"  }\n"
"}",
Style));
+  Style.IndentCaseLabels = false;
+  Style.IndentCaseBlocks = true;
+  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"
+   "  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"
+"  default:\n"
+"{\n"
+"  return true;\n"
+"}\n"
+"}",
+format("switch (n) {\n"
+   "case 0: {\n"
+   "  return false;\n"
+   "}\n"
+   "default: {\n"
+   "  return true;\n"
+   "}\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -12578,6 +12622,7 @@
   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);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1985,7 +1985,8 @@
 --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);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -480,6 +480,7 @@
 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 @@
   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;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1314,7 +1314,8 @@
   ///
   /// 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 @@
   /// \endcode
   bool IndentCaseLabels;
 
+  /// Indent case label blocks one level from the case label.
+  ///
+  /// Whe

[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

[Nicolas Capens via Phabricator via cfe-commits]

capn updated this revision to Diff 236740.
capn marked an inline comment as done.
capn added a comment.

Added case without {} to tests to ensure that still works as expected too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72276/new/

https://reviews.llvm.org/D72276

Files:
  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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1211,6 +1211,58 @@
"  }\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 @@
   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);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1985,7 +1985,8 @@
 --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);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -480,6 +480,7 @@
 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 @@
   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;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1314,7 +1314,8 @@
   ///
   /// 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
 

[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

[Nicolas Capens via Phabricator via cfe-commits]

capn added a comment.

Thanks for the review! I don't have committer access, feel free to land at your 
convenience.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72276/new/

https://reviews.llvm.org/D72276



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

[Nicolas Capens via Phabricator via cfe-commits]

capn added a comment.

Ping. Please land on my behalf.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72276/new/

https://reviews.llvm.org/D72276



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95299: Fix truncated __OPENMP_NVPTX__ preprocessor condition

[Nicolas Capens via Phabricator via cfe-commits]

capn added a comment.

Can a committer go ahead and land this please? I'm running into this warning 
for any CUDA code compiled by Clang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95299/new/

https://reviews.llvm.org/D95299

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits