[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

[Florian Kauer via Phabricator via cfe-commits]

koalo created this revision.
koalo added reviewers: djasper, rsmith.
Herald added a subscriber: cfe-commits.

Before, clang-format has tried to put enums on a single line whenever possible 
(unless in styles where the opening brace was put on a seperate line anyway).
AllowShortEnumsOnASingleLine that defaults to true (matching the conventional 
behaviour) can be set to false to force enums on multiple lines.

This feature is requested from time to time, e.g. here 
,
 and should be unobtrusive for users that do not set this option to false.


Repository:
  rC Clang

https://reviews.llvm.org/D54628

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1521,6 +1521,24 @@
   verifyFormat("enum ShortEnum { A, B, C };");
   verifyGoogleFormat("enum ShortEnum { A, B, C };");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum Enum {\n"
+   "};",Style);
+  verifyFormat("enum {\n"
+   "};",Style);
+  verifyFormat("enum X E {\n"
+   "} d;",Style);
+  verifyFormat("enum __attribute__((...)) E {\n"
+   "} d;",Style);
+  verifyFormat("enum __declspec__((...)) E {\n"
+   "} d;",Style);
+  verifyFormat("enum ShortEnum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "};",Style);
+
   EXPECT_EQ("enum KeepEmptyLines {\n"
 "  ONE,\n"
 "\n"
@@ -1589,6 +1607,19 @@
   verifyFormat("enum struct __attribute__((...)) E {} d;");
   verifyFormat("enum struct __declspec__((...)) E {} d;");
   verifyFormat("enum struct X f() {\n  a();\n  return 42;\n}");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum struct Enum {\n"
+   "};",Style);
+  verifyFormat("enum struct {\n"
+   "};",Style);
+  verifyFormat("enum struct X E {\n"
+   "} d;",Style);
+  verifyFormat("enum struct __attribute__((...)) E {\n"
+   "} d;",Style);
+  verifyFormat("enum struct __declspec__((...)) E {\n"
+   "} d;",Style);
 }
 
 TEST_F(FormatTest, FormatsEnumClass) {
@@ -1606,6 +1637,19 @@
   verifyFormat("enum class __attribute__((...)) E {} d;");
   verifyFormat("enum class __declspec__((...)) E {} d;");
   verifyFormat("enum class X f() {\n  a();\n  return 42;\n}");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum class Enum {\n"
+   "};",Style);
+  verifyFormat("enum class {\n"
+   "};",Style);
+  verifyFormat("enum class X E {\n"
+   "} d;",Style);
+  verifyFormat("enum class __attribute__((...)) E {\n"
+   "} d;",Style);
+  verifyFormat("enum class __declspec__((...)) E {\n"
+   "} d;",Style);
 }
 
 TEST_F(FormatTest, FormatsEnumTypes) {
@@ -1615,6 +1659,17 @@
"};");
   verifyFormat("enum X : int { A, B };");
   verifyFormat("enum X : std::uint32_t { A, B };");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum X : int {\n"
+   "  A,\n"
+	   "  B\n"
+	   "};",Style);
+  verifyFormat("enum X : std::uint32_t {\n"
+   "  A,\n"
+	   "  B\n"
+	   "};",Style);
 }
 
 TEST_F(FormatTest, FormatsTypedefEnum) {
@@ -1641,6 +1696,16 @@
"  THREE = 3\n"
"} LongEnum;",
Style);
+
+  Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("typedef enum {\n"
+   "} EmptyEnum;",Style);
+  verifyFormat("typedef enum {\n"
+   "  A,\n"
+	   "  B,\n"
+	   "  C\n"
+	   "} ShortEnum;",Style);
 }
 
 TEST_F(FormatTest, FormatsNSEnums) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2948,6 +2948,10 @@
   }
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
+  if (isAllmanBrace(Left) && !Style.AllowShortEnumsOnASingleLine &&
+  (Line.startsWith(tok::kw_enum) ||
+   Line.startsWith(tok::kw_typedef, tok::kw_enum)))
+return true;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
 return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Form

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

[Florian Kauer via Phabricator via cfe-commits]

koalo added a comment.

In D54628#1742781 , @MyDeveloperDay 
wrote:

> sorry searching through old issues, are you still interested in this patch?


Yes, would be nice to have either this or an equivalent possibility that 
implements the same feature.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54628



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

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

[Florian Kauer via Phabricator via cfe-commits]

koalo added a comment.

The difference is that BraceWrapping.AfterEnum also wraps before the brace.

With AfterEnum we have the following

  true:
  enum X : int
  {
B
  };
  
  false:
  enum X : int { B };

But what I want is this:

  enum X : int {
B
  };


Repository:
  rC Clang

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

https://reviews.llvm.org/D54628



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

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

[Florian Kauer via Phabricator via cfe-commits]

koalo added a comment.

Yes it does not. Therefore, I have followed the same pattern as the the other 
AllowShort* options.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54628



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

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

[Florian Kauer via Phabricator via cfe-commits]

koalo updated this revision to Diff 229298.
koalo added a comment.

Rebased to current master


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

https://reviews.llvm.org/D54628

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1798,6 +1798,24 @@
   verifyFormat("enum ShortEnum { A, B, C };");
   verifyGoogleFormat("enum ShortEnum { A, B, C };");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum Enum {\n"
+   "};",Style);
+  verifyFormat("enum {\n"
+   "};",Style);
+  verifyFormat("enum X E {\n"
+   "} d;",Style);
+  verifyFormat("enum __attribute__((...)) E {\n"
+   "} d;",Style);
+  verifyFormat("enum __declspec__((...)) E {\n"
+   "} d;",Style);
+  verifyFormat("enum ShortEnum {\n"
+   "  A,\n"
+   "  B,\n"
+   "  C\n"
+   "};",Style);
+
   EXPECT_EQ("enum KeepEmptyLines {\n"
 "  ONE,\n"
 "\n"
@@ -1866,6 +1884,19 @@
   verifyFormat("enum struct __attribute__((...)) E {} d;");
   verifyFormat("enum struct __declspec__((...)) E {} d;");
   verifyFormat("enum struct X f() {\n  a();\n  return 42;\n}");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum struct Enum {\n"
+   "};",Style);
+  verifyFormat("enum struct {\n"
+   "};",Style);
+  verifyFormat("enum struct X E {\n"
+   "} d;",Style);
+  verifyFormat("enum struct __attribute__((...)) E {\n"
+   "} d;",Style);
+  verifyFormat("enum struct __declspec__((...)) E {\n"
+   "} d;",Style);
 }
 
 TEST_F(FormatTest, FormatsEnumClass) {
@@ -1883,6 +1914,19 @@
   verifyFormat("enum class __attribute__((...)) E {} d;");
   verifyFormat("enum class __declspec__((...)) E {} d;");
   verifyFormat("enum class X f() {\n  a();\n  return 42;\n}");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum class Enum {\n"
+   "};",Style);
+  verifyFormat("enum class {\n"
+   "};",Style);
+  verifyFormat("enum class X E {\n"
+   "} d;",Style);
+  verifyFormat("enum class __attribute__((...)) E {\n"
+   "} d;",Style);
+  verifyFormat("enum class __declspec__((...)) E {\n"
+   "} d;",Style);
 }
 
 TEST_F(FormatTest, FormatsEnumTypes) {
@@ -1892,6 +1936,17 @@
"};");
   verifyFormat("enum X : int { A, B };");
   verifyFormat("enum X : std::uint32_t { A, B };");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum X : int {\n"
+   "  A,\n"
+	   "  B\n"
+	   "};",Style);
+  verifyFormat("enum X : std::uint32_t {\n"
+   "  A,\n"
+	   "  B\n"
+	   "};",Style);
 }
 
 TEST_F(FormatTest, FormatsTypedefEnum) {
@@ -1918,6 +1973,16 @@
"  THREE = 3\n"
"} LongEnum;",
Style);
+
+  Style = getLLVMStyle();
+  Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("typedef enum {\n"
+   "} EmptyEnum;",Style);
+  verifyFormat("typedef enum {\n"
+   "  A,\n"
+	   "  B,\n"
+	   "  C\n"
+	   "} ShortEnum;",Style);
 }
 
 TEST_F(FormatTest, FormatsNSEnums) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3106,6 +3106,10 @@
   }
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
+  if (isAllmanBrace(Left) && !Style.AllowShortEnumsOnASingleLine &&
+  (Line.startsWith(tok::kw_enum) ||
+   Line.startsWith(tok::kw_typedef, tok::kw_enum)))
+return true;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
 return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -401,6 +401,8 @@
Style.AllowShortIfStatementsOnASingleLine);
 IO.mapOptional("AllowShortLoopsOnASingleLine",
Style.AllowShortLoopsOnASingleLine);
+IO.mapOptional("AllowShortEnumsOnASingleLine",
+   Style.AllowShortEnumsOnASingleLine);
 IO.mapOptional("AlwaysBreakAfterDefinitionReturnType",
Style.AlwaysBreakAfterDefinitionReturnType);
 IO.mapOptional("Always

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

[Florian Kauer via Phabricator via cfe-commits]

koalo added a comment.

Yes, that is at least my understanding. I just rebased to master.

In my understanding, "short" means "put it on a single line if it fits 
considering the current maximum line length".


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

https://reviews.llvm.org/D54628



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