[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371719: [clang-format] Add new style option IndentGotoLabels 
(authored by paulhoad, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67037?vs=219582=219871#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67037

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

Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -1527,6 +1527,23 @@
plop();  plop();
  }  }
 
+**IndentGotoLabels** (``bool``)
+  Indent goto labels.
+
+  When ``false``, goto labels are flushed left.
+
+  .. code-block:: c++
+
+ true:  false:
+ int f() {  vs. int f() {
+   if (foo()) {   if (foo()) {
+   label1:  label1:
+ bar(); bar();
+   }  }
+ label2:label2:
+   return 1;  return 1;
+ }  }
+
 **IndentPPDirectives** (``PPDirectiveIndentStyle``)
   The preprocessor directive indenting style to use.
 
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1265,6 +1265,22 @@
   /// \endcode
   bool IndentCaseLabels;
 
+  /// Indent goto labels.
+  ///
+  /// When ``false``, goto labels are flushed left.
+  /// \code
+  ///true:  false:
+  ///int f() {  vs. int f() {
+  ///  if (foo()) {   if (foo()) {
+  ///  label1:  label1:
+  ///bar(); bar();
+  ///  }  }
+  ///label2:label2:
+  ///  return 1;  return 1;
+  ///}  }
+  /// \endcode
+  bool IndentGotoLabels;
+
   /// Options for indenting preprocessor directives.
   enum PPDirectiveIndentStyle {
 /// Does not indent any directives.
@@ -1990,6 +2006,7 @@
IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks &&
IncludeStyle.IncludeCategories == R.IncludeStyle.IncludeCategories &&
IndentCaseLabels == R.IndentCaseLabels &&
+   IndentGotoLabels == R.IndentGotoLabels &&
IndentPPDirectives == R.IndentPPDirectives &&
IndentWidth == R.IndentWidth && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
Index: cfe/trunk/lib/Format/UnwrappedLineParser.h
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.h
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h
@@ -106,7 +106,7 @@
   void parseTryCatch();
   void parseForOrWhileLoop();
   void parseDoWhile();
-  void parseLabel();
+  void parseLabel(bool LeftAlignLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1351,7 +1351,7 @@
   (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
-  parseLabel();
+  parseLabel(!Style.IndentGotoLabels);
   return;
 }
 // Recognize function-like macro usages without trailing semicolon as
@@ -1970,11 +1970,13 @@
   parseStructuralElement();
 }
 
-void UnwrappedLineParser::parseLabel() {
+void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
+  if (LeftAlignLabel)
+Line->Level = 0;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Line->Level,

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

@MyDeveloperDay
Thanks for your suggestions! I think it's better now.
This is my first LLVM patch (hopefully first of many) so I don't have commit 
permissions yet. If you're still happy with this change, could you please 
commit on my behalf?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037



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

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp updated this revision to Diff 219582.
tetsuo-cpp added a comment.

Addressed review comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1408,6 +1408,30 @@
"test_label:;\n"
"  int i = 0;\n"
"}");
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentGotoLabels = false;
+  verifyFormat("void f() {\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "  some_other_code();\n"
+   "  {\n"
+   "some_more_code();\n"
+   "another_label:\n"
+   "some_more_code();\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "  some_other_code();\n"
+   "}",
+   Style);
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:;\n"
+   "  int i = 0;\n"
+   "}");
 }
 
 //===--===//
@@ -11769,6 +11793,7 @@
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
   CHECK_PARSE_BOOL(IndentCaseLabels);
+  CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -106,7 +106,7 @@
   void parseTryCatch();
   void parseForOrWhileLoop();
   void parseDoWhile();
-  void parseLabel();
+  void parseLabel(bool LeftAlignLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1351,7 +1351,7 @@
   (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
-  parseLabel();
+  parseLabel(!Style.IndentGotoLabels);
   return;
 }
 // Recognize function-like macro usages without trailing semicolon as
@@ -1970,11 +1970,13 @@
   parseStructuralElement();
 }
 
-void UnwrappedLineParser::parseLabel() {
+void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
+  if (LeftAlignLabel)
+Line->Level = 0;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Line->Level,
Style.BraceWrapping.AfterCaseLabel,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -453,6 +453,7 @@
 IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories);
 IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex);
 IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
+IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
 IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
 IO.mapOptional("IndentWidth", Style.IndentWidth);
 IO.mapOptional("IndentWrappedFunctionNames",
@@ -725,6 +726,7 @@
   LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
   LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
   LLVMStyle.IndentCaseLabels = false;
+  LLVMStyle.IndentGotoLabels = true;
   LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1265,6 +1265,22 @@
   /// \endcode
   bool IndentCaseLabels;
 
+  /// Indent goto labels.
+  ///
+  /// When ``false``, goto labels are flushed left.
+  /// \code
+  ///   

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

This LGTM, just some minor Nits




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1354
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
-  parseLabel();
+  parseLabel(true);
   return;

could this just be 


```
 // Align goto labels.
parseLabel(!Style.IndentGotoLabels);
```







Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1973
 
-void UnwrappedLineParser::parseLabel() {
+void UnwrappedLineParser::parseLabel(bool GotoLabel) {
   nextToken();

I think this could just be `LeftAlignLabel`, I'm not sure this function needs 
to know anything about `goto` specifically



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1978
 --Line->Level;
+  if (GotoLabel && !Style.IndentGotoLabels)
+Line->Level = 0;

then this would just be 


```
if (LeftAlignLabel){
Line->level = 0;
}
```



Comment at: clang/lib/Format/UnwrappedLineParser.h:109
   void parseDoWhile();
-  void parseLabel();
+  void parseLabel(bool GotoLabel = false);
   void parseCaseLabel();

Nit: maybe rename to just `LeftAlignLabel`


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037



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

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

Friendly ping. Could I please get this looked at?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037



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

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp created this revision.
tetsuo-cpp added a reviewer: klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This option determines whether goto labels are indented according to scope. 
Setting this option to false causes goto labels to be flushed to the left.
This is mostly copied from this patch 
 submitted 
by Christian Neukirchen that didn't make its way into trunk.

  true:  false:
  int f() {  vs. int f() {
if (foo()) {   if (foo()) {
label1:  label1:
  bar(); bar();
}  }
  label2:label2:
return 1;  return 1;
  }  }


Repository:
  rC Clang

https://reviews.llvm.org/D67037

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1408,6 +1408,30 @@
"test_label:;\n"
"  int i = 0;\n"
"}");
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentGotoLabels = false;
+  verifyFormat("void f() {\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "  some_other_code();\n"
+   "  {\n"
+   "some_more_code();\n"
+   "another_label:\n"
+   "some_more_code();\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "  some_other_code();\n"
+   "}",
+   Style);
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:;\n"
+   "  int i = 0;\n"
+   "}");
 }
 
 //===--===//
@@ -11769,6 +11793,7 @@
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
   CHECK_PARSE_BOOL(IndentCaseLabels);
+  CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -106,7 +106,7 @@
   void parseTryCatch();
   void parseForOrWhileLoop();
   void parseDoWhile();
-  void parseLabel();
+  void parseLabel(bool GotoLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1351,7 +1351,7 @@
   (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
-  parseLabel();
+  parseLabel(true);
   return;
 }
 // Recognize function-like macro usages without trailing semicolon as
@@ -1970,11 +1970,13 @@
   parseStructuralElement();
 }
 
-void UnwrappedLineParser::parseLabel() {
+void UnwrappedLineParser::parseLabel(bool GotoLabel) {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
+  if (GotoLabel && !Style.IndentGotoLabels)
+Line->Level = 0;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Line->Level,
Style.BraceWrapping.AfterCaseLabel,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -453,6 +453,7 @@
 IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories);
 IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex);
 IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
+IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
 IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
 IO.mapOptional("IndentWidth", Style.IndentWidth);