Skip to content

Commit

Permalink
[clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for te…
Browse files Browse the repository at this point in the history
…mplates

https://bugs.llvm.org/show_bug.cgi?id=48594

Empty or small templates were not being treated the same way as small classes especially when SplitEmptyRecord was set to true

This revision aims to help this by identifying a case when we should try not to merge the lines together

Reviewed By: curdeius, JohelEGP

Differential Revision: https://reviews.llvm.org/D93839
  • Loading branch information
mydeveloperday committed Jan 17, 2021
1 parent e7bc6c5 commit 00dc97f
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 16 deletions.
29 changes: 29 additions & 0 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,11 @@ class LineJoiner {
return !Style.BraceWrapping.SplitEmptyRecord && EmptyBlock
? tryMergeSimpleBlock(I, E, Limit)
: 0;

if (Tok && Tok->is(tok::kw_template) &&
Style.BraceWrapping.SplitEmptyRecord && EmptyBlock) {
return 0;
}
}

// FIXME: TheLine->Level != 0 might or might not be the right check to do.
Expand Down Expand Up @@ -355,6 +360,30 @@ class LineJoiner {
if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
return 0;

// Don't merge an empty template class or struct if SplitEmptyRecords
// is defined.
if (Style.BraceWrapping.SplitEmptyRecord &&
TheLine->Last->is(tok::l_brace) && I != AnnotatedLines.begin() &&
I[-1]->Last) {
const FormatToken *Previous = I[-1]->Last;
if (Previous) {
if (Previous->is(tok::comment))
Previous = Previous->getPreviousNonComment();
if (Previous) {
if (Previous->is(tok::greater))
return 0;
if (Previous->is(tok::identifier)) {
const FormatToken *PreviousPrevious =
Previous->getPreviousNonComment();
if (PreviousPrevious &&
PreviousPrevious->isOneOf(tok::kw_class, tok::kw_struct))
return 0;
}
}
}
}

// Try to merge a block with left brace wrapped that wasn't yet covered
if (TheLine->Last->is(tok::l_brace)) {
return !Style.BraceWrapping.AfterFunction ||
Expand Down
65 changes: 65 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9891,6 +9891,71 @@ TEST_F(FormatTest, SplitEmptyClass) {
"{\n"
"} Foo_t;",
Style);

Style.BraceWrapping.SplitEmptyRecord = true;
Style.BraceWrapping.AfterStruct = true;
verifyFormat("class rep\n"
"{\n"
"};",
Style);
verifyFormat("struct rep\n"
"{\n"
"};",
Style);
verifyFormat("template <typename T> class rep\n"
"{\n"
"};",
Style);
verifyFormat("template <typename T> struct rep\n"
"{\n"
"};",
Style);
verifyFormat("class rep\n"
"{\n"
" int x;\n"
"};",
Style);
verifyFormat("struct rep\n"
"{\n"
" int x;\n"
"};",
Style);
verifyFormat("template <typename T> class rep\n"
"{\n"
" int x;\n"
"};",
Style);
verifyFormat("template <typename T> struct rep\n"
"{\n"
" int x;\n"
"};",
Style);
verifyFormat("template <typename T> class rep // Foo\n"
"{\n"
" int x;\n"
"};",
Style);
verifyFormat("template <typename T> struct rep // Bar\n"
"{\n"
" int x;\n"
"};",
Style);

verifyFormat("template <typename T> class rep<T>\n"
"{\n"
" int x;\n"
"};",
Style);

verifyFormat("template <typename T> class rep<std::complex<T>>\n"
"{\n"
" int x;\n"
"};",
Style);
verifyFormat("template <typename T> class rep<std::complex<T>>\n"
"{\n"
"};",
Style);
}

TEST_F(FormatTest, SplitEmptyStruct) {
Expand Down
35 changes: 19 additions & 16 deletions clang/unittests/Format/FormatTestCSharp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -835,25 +835,27 @@ if (someThings[i][j][k].Contains(myThing)) {
TEST_F(FormatTestCSharp, CSharpGenericTypeConstraints) {
FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);

verifyFormat(R"(//
class ItemFactory<T>
where T : new() {})",
EXPECT_TRUE(Style.BraceWrapping.SplitEmptyRecord);

verifyFormat("class ItemFactory<T>\n"
" where T : new() {\n"
"}",
Style);

verifyFormat(R"(//
class Dictionary<TKey, TVal>
where TKey : IComparable<TKey>
where TVal : IMyInterface {
public void MyMethod<T>(T t)
where T : IMyInterface {
doThing();
}
})",
verifyFormat("class Dictionary<TKey, TVal>\n"
" where TKey : IComparable<TKey>\n"
" where TVal : IMyInterface {\n"
" public void MyMethod<T>(T t)\n"
" where T : IMyInterface {\n"
" doThing();\n"
" }\n"
"}",
Style);

verifyFormat(R"(//
class ItemFactory<T>
where T : new(), IAnInterface<T>, IAnotherInterface<T>, IAnotherInterfaceStill<T> {})",
verifyFormat("class ItemFactory<T>\n"
" where T : new(), IAnInterface<T>, IAnotherInterface<T>, "
"IAnotherInterfaceStill<T> {\n"
"}",
Style);

Style.ColumnLimit = 50; // Force lines to be wrapped.
Expand All @@ -862,7 +864,8 @@ class ItemFactory<T, U>
where T : new(),
IAnInterface<T>,
IAnotherInterface<T, U>,
IAnotherInterfaceStill<T, U> {})",
IAnotherInterfaceStill<T, U> {
})",
Style);

// In other languages `where` can be used as a normal identifier.
Expand Down

0 comments on commit 00dc97f

Please sign in to comment.