MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, curdeius, HazardyKnusperkeks.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.
LLVM 13.0.0-rc2 shows change of behaviour in enum and interface BraceWrapping
(likely before we simply didn't wrap) but may be related to D99840:
[clang-format] Correctly attach enum braces with ShortEnums disabled
<https://reviews.llvm.org/D99840>
Logged as https://bugs.llvm.org/show_bug.cgi?id=51640
This change ensure AfterEnum works for
`internal|public|protected|private enum A {` in the same way as it works for
`enum A {` in C++
A similar issue was also observed with `interface` in C#
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D108810
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestCSharp.cpp
Index: clang/unittests/Format/FormatTestCSharp.cpp
===================================================================
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,7 +402,9 @@
}
TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
- verifyFormat("public enum var {\n"
+ // AfterEnum is true by default.
+ verifyFormat("public enum var\n"
+ "{\n"
" none,\n"
" @string,\n"
" bool,\n"
@@ -1099,5 +1101,194 @@
getGoogleStyle(FormatStyle::LK_Cpp));
}
+TEST_F(FormatTestCSharp, CSharpAfterEnum) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+ Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterEnum = false;
+ Style.AllowShortEnumsOnASingleLine = false;
+
+ verifyFormat("enum MyEnum {\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("internal enum MyEnum {\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("public enum MyEnum {\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("protected enum MyEnum {\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("private enum MyEnum {\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+
+ Style.BraceWrapping.AfterEnum = true;
+ Style.AllowShortEnumsOnASingleLine = false;
+
+ verifyFormat("enum MyEnum\n"
+ "{\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("internal enum MyEnum\n"
+ "{\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("public enum MyEnum\n"
+ "{\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("protected enum MyEnum\n"
+ "{\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("private enum MyEnum\n"
+ "{\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpAfterClass) {
+ FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+ Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterClass = false;
+
+ verifyFormat("class MyClass {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("internal class MyClass {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("public class MyClass {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("protected class MyClass {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("private class MyClass {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+
+ verifyFormat("interface Interface {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("internal interface Interface {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("public interface Interface {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("protected interface Interface {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("private interface Interface {\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+
+ Style.BraceWrapping.AfterClass = true;
+
+ verifyFormat("class MyClass\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("internal class MyClass\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("public class MyClass\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("protected class MyClass\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("private class MyClass\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+
+ verifyFormat("interface MyInterface\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("internal interface MyInterface\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("public interface MyInterface\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("protected interface MyInterface\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("private interface MyInterface\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+}
+
} // namespace format
} // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3778,12 +3778,34 @@
if (Right.is(TT_InlineASMBrace))
return Right.HasUnescapedNewline;
- if (isAllmanBrace(Left) || isAllmanBrace(Right))
- return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
- (Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
- Style.BraceWrapping.AfterEnum) ||
- (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
+ if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
+ if (Style.BraceWrapping.AfterEnum) {
+ if (Line.startsWith(tok::kw_enum) ||
+ Line.startsWith(tok::kw_typedef, tok::kw_enum))
+ return true;
+ if (Style.isCSharp()) {
+ if (Line.First && Line.First->Next &&
+ Line.First->isOneOf(Keywords.kw_internal, tok::kw_public,
+ tok::kw_private, tok::kw_protected) &&
+ Line.First->Next->is(tok::kw_enum))
+ return true;
+ }
+ }
+
+ // Ensure BraceWrapping for public interface A {
+ if (Style.BraceWrapping.AfterClass && Style.isCSharp()) {
+ if (Line.First && Line.First->Next &&
+ Line.First->isOneOf(Keywords.kw_internal, tok::kw_public,
+ tok::kw_private, tok::kw_protected) &&
+ Line.First->Next->is(Keywords.kw_interface))
+ return true;
+ if (Line.startsWith(Keywords.kw_interface) &&
+ Style.BraceWrapping.AfterClass)
+ return true;
+ }
+ return (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
(Line.startsWith(tok::kw_struct) && Style.BraceWrapping.AfterStruct);
+ }
if (Left.is(TT_ObjCBlockLBrace) &&
Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
return true;
@@ -3816,18 +3838,17 @@
if (Right.is(TT_ProtoExtensionLSquare))
return true;
- // In text proto instances if a submessage contains at least 2 entries and at
- // least one of them is a submessage, like A { ... B { ... } ... },
- // put all of the entries of A on separate lines by forcing the selector of
- // the submessage B to be put on a newline.
+ // In text proto instances if a submessage contains at least 2 entries and
+ // at least one of them is a submessage, like A { ... B { ... } ... }, put
+ // all of the entries of A on separate lines by forcing the selector of the
+ // submessage B to be put on a newline.
//
// Example: these can stay on one line:
// a { scalar_1: 1 scalar_2: 2 }
// a { b { key: value } }
//
- // and these entries need to be on a new line even if putting them all in one
- // line is under the column limit:
- // a {
+ // and these entries need to be on a new line even if putting them all in
+ // one line is under the column limit: a {
// scalar: 1
// b { key: value }
// }
@@ -3836,11 +3857,12 @@
// siblings, *and* breaking before a field that follows a submessage field.
//
// Be careful to exclude the case [proto.ext] { ... } since the `]` is
- // the TT_SelectorName there, but we don't want to break inside the brackets.
+ // the TT_SelectorName there, but we don't want to break inside the
+ // brackets.
//
// Another edge case is @submessage { key: value }, which is a common
- // substitution placeholder. In this case we want to keep `@` and `submessage`
- // together.
+ // substitution placeholder. In this case we want to keep `@` and
+ // `submessage` together.
//
// We ensure elsewhere that extensions are always on their own line.
if ((Style.Language == FormatStyle::LK_Proto ||
@@ -3877,11 +3899,12 @@
(LBrace->Next && LBrace->Next->is(tok::r_brace)))) ||
LBrace->is(TT_ArrayInitializerLSquare) || LBrace->is(tok::less))) {
// If Left.ParameterCount is 0, then this submessage entry is not the
- // first in its parent submessage, and we want to break before this entry.
- // If Left.ParameterCount is greater than 0, then its parent submessage
- // might contain 1 or more entries and we want to break before this entry
- // if it contains at least 2 entries. We deal with this case later by
- // detecting and breaking before the next entry in the parent submessage.
+ // first in its parent submessage, and we want to break before this
+ // entry. If Left.ParameterCount is greater than 0, then its parent
+ // submessage might contain 1 or more entries and we want to break
+ // before this entry if it contains at least 2 entries. We deal with
+ // this case later by detecting and breaking before the next entry in
+ // the parent submessage.
if (Left.ParameterCount == 0)
return true;
// However, if this submessage is the first entry in its parent
@@ -3901,9 +3924,10 @@
return true;
}
- // Deal with lambda arguments in C++ - we want consistent line breaks whether
- // they happen to be at arg0, arg1 or argN. The selection is a bit nuanced
- // as aggressive line breaks are placed when the lambda is not the last arg.
+ // Deal with lambda arguments in C++ - we want consistent line breaks
+ // whether they happen to be at arg0, arg1 or argN. The selection is a bit
+ // nuanced as aggressive line breaks are placed when the lambda is not the
+ // last arg.
if ((Style.Language == FormatStyle::LK_Cpp ||
Style.Language == FormatStyle::LK_ObjC) &&
Left.is(tok::l_paren) && Left.BlockParameterCount > 0 &&
@@ -4047,9 +4071,9 @@
if (Right.isTrailingComment())
// We rely on MustBreakBefore being set correctly here as we should not
// change the "binding" behavior of a comment.
- // The first comment in a braced lists is always interpreted as belonging to
- // the first list element. Otherwise, it should be placed outside of the
- // list.
+ // The first comment in a braced lists is always interpreted as belonging
+ // to the first list element. Otherwise, it should be placed outside of
+ // the list.
return Left.is(BK_BracedInit) ||
(Left.is(TT_CtorInitializerColon) &&
Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
