[PATCH] D77682: [clang-format] Always break line after enum opening brace
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
[PATCH] D77682: [clang-format] Always break line after enum opening brace
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
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
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] D105835: [Driver] Let -fno-integrated-as -gdwarf-5 use -fdwarf-directory-asm
osandov added a comment. I tested my reproducer and it also fixes it, thanks. Should it be an error to specify `-fno-dwarf-directory-asm` together with `-gdwarf-5`, since that produces incorrect results? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105835/new/ https://reviews.llvm.org/D105835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D105835: [Driver] Let -fno-integrated-as -gdwarf-5 use -fdwarf-directory-asm
osandov accepted this revision. osandov added a comment. This revision is now accepted and ready to land. That's fair enough. I don't know if I'm qualified to review this, but this I think this is a better solution than my fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105835/new/ https://reviews.llvm.org/D105835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
