[llvm-branch-commits] [clang] a797306 - [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Oct 20 20:51:08 PDT 2021
Author: mydeveloperday
Date: 2021-10-20T20:50:41-07:00
New Revision: a797306b772160d7096f32380cb238c02df20ff8
URL: https://github.com/llvm/llvm-project/commit/a797306b772160d7096f32380cb238c02df20ff8
DIFF: https://github.com/llvm/llvm-project/commit/a797306b772160d7096f32380cb238c02df20ff8.diff
LOG: [clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change
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}
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#
Reviewed By: krasimir, owenpan
Differential Revision: https://reviews.llvm.org/D108810
(cherry picked from commit ed367b9dff10ee1df9ac1984eb2ad7544da7ab06)
Added:
Modified:
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestCSharp.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 11dc661abc24e..86c9ac4aa364a 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3604,6 +3604,16 @@ static bool isAllmanLambdaBrace(const FormatToken &Tok) {
!Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
}
+// Returns the first token on the line that is not a comment.
+static const FormatToken *getFirstNonComment(const AnnotatedLine &Line) {
+ const FormatToken *Next = Line.First;
+ if (!Next)
+ return Next;
+ if (Next->is(tok::comment))
+ Next = Next->getNextNonComment();
+ return Next;
+}
+
bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
const FormatToken &Right) {
const FormatToken &Left = *Right.Previous;
@@ -3785,12 +3795,34 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
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)) {
+ auto FirstNonComment = getFirstNonComment(Line);
+ bool AccessSpecifier =
+ FirstNonComment &&
+ FirstNonComment->isOneOf(Keywords.kw_internal, tok::kw_public,
+ tok::kw_private, tok::kw_protected);
+
+ if (Style.BraceWrapping.AfterEnum) {
+ if (Line.startsWith(tok::kw_enum) ||
+ Line.startsWith(tok::kw_typedef, tok::kw_enum))
+ return true;
+ // Ensure BraceWrapping for `public enum A {`.
+ if (AccessSpecifier && FirstNonComment->Next &&
+ FirstNonComment->Next->is(tok::kw_enum))
+ return true;
+ }
+
+ // Ensure BraceWrapping for `public interface A {`.
+ if (Style.BraceWrapping.AfterClass &&
+ ((AccessSpecifier && FirstNonComment->Next &&
+ FirstNonComment->Next->is(Keywords.kw_interface)) ||
+ Line.startsWith(Keywords.kw_interface)))
+ 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;
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 673986d16af2f..8487875064aa8 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2532,6 +2532,8 @@ bool UnwrappedLineParser::parseEnum() {
if (FormatTok->Tok.is(tok::kw_enum))
nextToken();
+ const FormatToken &InitialToken = *FormatTok;
+
// In TypeScript, "enum" can also be used as property name, e.g. in interface
// declarations. An "enum" keyword followed by a colon would be a syntax
// error and thus assume it is just an identifier.
@@ -2578,7 +2580,8 @@ bool UnwrappedLineParser::parseEnum() {
return true;
}
- if (!Style.AllowShortEnumsOnASingleLine)
+ if (!Style.AllowShortEnumsOnASingleLine &&
+ ShouldBreakBeforeBrace(Style, InitialToken))
addUnwrappedLine();
// Parse enum body.
nextToken();
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index c575f643b5a14..61a06294c8f94 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2451,6 +2451,14 @@ TEST_F(FormatTest, ShortEnums) {
Style.AllowShortEnumsOnASingleLine = true;
verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style);
Style.AllowShortEnumsOnASingleLine = false;
+ verifyFormat("enum {\n"
+ " A,\n"
+ " B,\n"
+ " C\n"
+ "} ShortEnum1, ShortEnum2;",
+ Style);
+ Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterEnum = true;
verifyFormat("enum\n"
"{\n"
" A,\n"
@@ -22205,8 +22213,7 @@ TEST_F(FormatTest, IndentAccessModifiers) {
Style);
// Enumerations are not records and should be unaffected.
Style.AllowShortEnumsOnASingleLine = false;
- verifyFormat("enum class E\n"
- "{\n"
+ verifyFormat("enum class E {\n"
" A,\n"
" B\n"
"};\n",
diff --git a/clang/unittests/Format/FormatTestCSharp.cpp b/clang/unittests/Format/FormatTestCSharp.cpp
index 3c990339cf748..b7ea8d3672a2a 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,6 +402,7 @@ TEST_F(FormatTestCSharp, CSharpRegions) {
}
TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
+ // AfterEnum is true by default.
verifyFormat("public enum var\n"
"{\n"
" none,\n"
@@ -1100,5 +1101,218 @@ class A {
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);
+ verifyFormat("/* Foo */ private enum MyEnum\n"
+ "{\n"
+ " Foo,\n"
+ " Bar,\n"
+ "}",
+ Style);
+ verifyFormat("/* Foo */ /* Bar */ 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);
+ verifyFormat("/* Foo */ private interface MyInterface\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+ verifyFormat("/* Foo */ /* Bar */ private interface MyInterface\n"
+ "{\n"
+ " int a;\n"
+ " int b;\n"
+ "}",
+ Style);
+}
+
} // namespace format
} // end namespace clang
More information about the llvm-branch-commits
mailing list