[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-13 Thread MyDeveloperDay via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG072ae7c1e64f: [clang-format] Always break line after enum 
opening brace (authored by MyDeveloperDay, committed by paulhoad 
paul.h...@gmail.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77682

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1929,6 +1929,24 @@
"  TWO\n"
"};\n"
"int i;");
+
+  FormatStyle EightIndent = getLLVMStyle();
+  EightIndent.IndentWidth = 8;
+  verifyFormat("enum {\n"
+   "VOID,\n"
+   "CHAR,\n"
+   "SHORT,\n"
+   "INT,\n"
+   "LONG,\n"
+   "SIGNED,\n"
+   "UNSIGNED,\n"
+   "BOOL,\n"
+   "FLOAT,\n"
+   "DOUBLE,\n"
+   "COMPLEX\n"
+   "};",
+   EightIndent);
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -423,7 +423,7 @@
   State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
-  if (State.Column <= NewLineColumn)
+  if (!State.Line->First->is(tok::kw_enum) && State.Column <= NewLineColumn)
 return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1929,6 +1929,24 @@
"  TWO\n"
"};\n"
"int i;");
+
+  FormatStyle EightIndent = getLLVMStyle();
+  EightIndent.IndentWidth = 8;
+  verifyFormat("enum {\n"
+   "VOID,\n"
+   "CHAR,\n"
+   "SHORT,\n"
+   "INT,\n"
+   "LONG,\n"
+   "SIGNED,\n"
+   "UNSIGNED,\n"
+   "BOOL,\n"
+   "FLOAT,\n"
+   "DOUBLE,\n"
+   "COMPLEX\n"
+   "};",
+   EightIndent);
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -423,7 +423,7 @@
   State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
-  if (State.Column <= NewLineColumn)
+  if (!State.Line->First->is(tok::kw_enum) && State.Column <= NewLineColumn)
 return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-10 Thread Omar Sandoval via Phabricator via cfe-commits 
osandov added a comment.

Thank you! I don't have commit access. How can I get this committed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77682



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

[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-10 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77682



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

[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-09 Thread Omar Sandoval via Phabricator via cfe-commits 
osandov updated this revision to Diff 256352.
osandov edited the summary of this revision.
osandov added a comment.

Update summary and test case to better reflect the issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77682

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1929,6 +1929,24 @@
"  TWO\n"
"};\n"
"int i;");
+
+  FormatStyle EightIndent = getLLVMStyle();
+  EightIndent.IndentWidth = 8;
+  verifyFormat("enum {\n"
+   "VOID,\n"
+   "CHAR,\n"
+   "SHORT,\n"
+   "INT,\n"
+   "LONG,\n"
+   "SIGNED,\n"
+   "UNSIGNED,\n"
+   "BOOL,\n"
+   "FLOAT,\n"
+   "DOUBLE,\n"
+   "COMPLEX\n"
+   "};",
+   EightIndent);
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -423,7 +423,7 @@
   State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
-  if (State.Column <= NewLineColumn)
+  if (!State.Line->First->is(tok::kw_enum) && State.Column <= NewLineColumn)
 return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1929,6 +1929,24 @@
"  TWO\n"
"};\n"
"int i;");
+
+  FormatStyle EightIndent = getLLVMStyle();
+  EightIndent.IndentWidth = 8;
+  verifyFormat("enum {\n"
+   "VOID,\n"
+   "CHAR,\n"
+   "SHORT,\n"
+   "INT,\n"
+   "LONG,\n"
+   "SIGNED,\n"
+   "UNSIGNED,\n"
+   "BOOL,\n"
+   "FLOAT,\n"
+   "DOUBLE,\n"
+   "COMPLEX\n"
+   "};",
+   EightIndent);
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -423,7 +423,7 @@
   State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
-  if (State.Column <= NewLineColumn)
+  if (!State.Line->First->is(tok::kw_enum) && State.Column <= NewLineColumn)
 return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-09 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

I understand now what you are trying to achieve, could you capture something 
more like that in your tests too.. (just so we don't break this change later)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77682



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

[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-08 Thread Omar Sandoval via Phabricator via cfe-commits 
osandov added a comment.

The style guide I'm following (the Linux kernel style) wants `AfterEnum: 
false`. A cursory search suggests that people treat this trailing comma 
behavior as a feature 
(https://stackoverflow.com/questions/23072223/clang-format-style-options-for-enums).
 However, I think this is separate from my issue. Consider the following 
example which doesn't have a trailing comma and doesn't fit on one line:

  $ cat enum.cpp
  enum { EOF, VOID, CHAR, SHORT, INT, LONG, SIGNED, UNSIGNED, BOOL, FLOAT, 
DOUBLE, COMPLEX, CONST, RESTRICT, VOLATILE, ATOMIC, STRUCT, UNION, ENUM, 
LPAREN, RPAREN, LBRACKET, RBRACKET, ASTERISK, DOT, NUMBER, IDENTIFIER };

The current code formats this very strangely:

  $ clang-format -style="{BasedOnStyle: llvm, IndentWidth: 8}" enum.cpp 
   
  enum { EOF,
 VOID,
 CHAR,
 SHORT,
 INT,
 LONG,
 SIGNED,
 UNSIGNED,
 BOOL,
 FLOAT,
 DOUBLE,
 COMPLEX,
 CONST,
 RESTRICT,
 VOLATILE,
 ATOMIC,
 STRUCT,
 UNION,
 ENUM,
 LPAREN,
 RPAREN,
 LBRACKET,
 RBRACKET,
 ASTERISK,
 DOT,
 NUMBER,
 IDENTIFIER };

With this change, I get what I expect:

  $ ./bin/clang-format -style="{BasedOnStyle: llvm, IndentWidth: 8}" enum.cpp
  enum {
  EOF,
  VOID,
  CHAR,
  SHORT,
  INT,
  LONG,
  SIGNED,
  UNSIGNED,
  BOOL,
  FLOAT,
  DOUBLE,
  COMPLEX,
  CONST,
  RESTRICT,
  VOLATILE,
  ATOMIC,
  STRUCT,
  UNION,
  ENUM,
  LPAREN,
  RPAREN,
  LBRACKET,
  RBRACKET,
  ASTERISK,
  DOT,
  NUMBER,
  IDENTIFIER
  };


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77682



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

[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-08 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

I think what is interesting is

  BreakBeforeBraces: Custom
  BraceWrapping:
  AfterEnum: false

gets messed up completely by the `,` and that IS a bug in my view

  $ clang-format enum.cpp
  enum {
A,
  };
  
  enum { B, C, D };
  
  enum {
B,
C,
D,
  };
  
  enum { E };


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77682



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

[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-08 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

Did you try:

  BreakBeforeBraces: Custom
  BraceWrapping:
AfterEnum: true

with AfterEnum:true

  $ clang-format enum.cpp
  enum
  {
A,
  };
  
  enum
  {
B,
C,
D
  };
  
  enum
  {
E
  };

with AfterEnum:false

  $ clang-format enum.cpp
  enum {
A,
  };
  
  enum { B, C, D };
  
  enum { E };

I believe example is not following the style of BraceWrapping.AfterEnum by 
virtue of the trailing `,`

I think I'd expect the correct behaviour to be

  enum { A, };

or

  enum { 
 A, 
  };

depending on that setting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77682



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

[PATCH] D77682: [clang-format] Always break line after enum opening brace

2020-04-07 Thread Omar Sandoval via Phabricator via cfe-commits 
osandov created this revision.
osandov added reviewers: MyDeveloperDay, krasimir.
osandov added projects: clang-format, clang.
Herald added a subscriber: cfe-commits.

clang-format currently puts the first enumerator on the same line as the
enum keyword and opening brace if it fits (for example, for anonymous
enums if IndentWidth is 8):

  $ echo "enum { A, };" | clang-format -style="{BasedOnStyle: llvm, 
IndentWidth: 8}"
  enum { A,
  };

This doesn't seem to be intentional, as I can't find any style guide that
suggests wrapping enums this way. Always force the enumerator to be on a new
line, which gets us the desired result:

  $ echo "enum { A, };" | ./bin/clang-format -style="{BasedOnStyle: llvm, 
IndentWidth: 8}"
  enum {
  A,
  };

Test Plan:

New test added. Confirmed test failed without change and passed with change by
running:

  $ ninja FormatTests && ./tools/clang/unittests/Format/FormatTests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77682

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1929,6 +1929,14 @@
"  TWO\n"
"};\n"
"int i;");
+
+  FormatStyle EightIndent = getLLVMStyle();
+  EightIndent.IndentWidth = 8;
+  verifyFormat("enum {\n"
+   "A,\n"
+   "};",
+   EightIndent);
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -423,7 +423,7 @@
   State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
-  if (State.Column <= NewLineColumn)
+  if (!State.Line->First->is(tok::kw_enum) && State.Column <= NewLineColumn)
 return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1929,6 +1929,14 @@
"  TWO\n"
"};\n"
"int i;");
+
+  FormatStyle EightIndent = getLLVMStyle();
+  EightIndent.IndentWidth = 8;
+  verifyFormat("enum {\n"
+   "A,\n"
+   "};",
+   EightIndent);
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -423,7 +423,7 @@
   State.Stack.back().BreakBeforeParameter && Current.CanBreakBefore)
 return true;
 
-  if (State.Column <= NewLineColumn)
+  if (!State.Line->First->is(tok::kw_enum) && State.Column <= NewLineColumn)
 return false;
 
   if (Style.AlwaysBreakBeforeMultilineStrings &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits