[clang] 36622c4 - [clang-format] Fix AllowShortFunctionsOnASingleLine: InlineOnly with wrapping after record.
Marek Kurdej via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 27 09:06:37 PST 2022
Author: Marek Kurdej
Date: 2022-01-27T18:06:31+01:00
New Revision: 36622c4e1a48cd02209524592abb9d929fff1e22
URL: https://github.com/llvm/llvm-project/commit/36622c4e1a48cd02209524592abb9d929fff1e22
DIFF: https://github.com/llvm/llvm-project/commit/36622c4e1a48cd02209524592abb9d929fff1e22.diff
LOG: [clang-format] Fix AllowShortFunctionsOnASingleLine: InlineOnly with wrapping after record.
Fixes https://github.com/llvm/llvm-project/issues/53430.
Initially, I had a quick and dirty approach, but it led to a myriad of special cases handling comments (that may add unwrapped lines).
So I added TT_RecordLBrace type annotations and it seems like a much nicer solution.
I think that in the future it will allow us to clean up some convoluted code that detects records.
Reviewed By: MyDeveloperDay, HazardyKnusperkeks
Differential Revision: https://reviews.llvm.org/D118337
Added:
Modified:
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 7cc090cb77de..a64329802ee3 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -96,6 +96,7 @@ namespace format {
TYPE(PointerOrReference) \
TYPE(PureVirtualSpecifier) \
TYPE(RangeBasedForLoopColon) \
+ TYPE(RecordLBrace) \
TYPE(RegexLiteral) \
TYPE(SelectorName) \
TYPE(StartOfName) \
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index a8cd1e30f74e..9d130dbb02eb 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1423,7 +1423,8 @@ class AnnotatingParser {
TT_LambdaArrow, TT_NamespaceMacro, TT_OverloadedOperator,
TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral,
TT_UntouchableMacroFunc, TT_ConstraintJunctions,
- TT_StatementAttributeLikeMacro, TT_FunctionLikeOrFreestandingMacro))
+ TT_StatementAttributeLikeMacro, TT_FunctionLikeOrFreestandingMacro,
+ TT_RecordLBrace))
CurrentToken->setType(TT_Unknown);
CurrentToken->Role.reset();
CurrentToken->MatchingParen = nullptr;
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 293a693fd481..0172a224335c 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -276,6 +276,9 @@ class LineJoiner {
FormatStyle::SFS_InlineOnly) {
// Just checking TheLine->Level != 0 is not enough, because it
// provokes treating functions inside indented namespaces as short.
+ if (Style.isJavaScript() && (*I)->Last->is(TT_FunctionLBrace))
+ return true;
+
if ((*I)->Level != 0) {
if (I == B)
return false;
@@ -288,23 +291,10 @@ class LineJoiner {
break;
// Check if the found line starts a record.
- auto *RecordTok = (*J)->First;
- while (RecordTok) {
- // TODO: Refactor to isRecord(RecordTok).
- if (RecordTok->isOneOf(tok::kw_class, tok::kw_struct))
- return true;
- if (Style.isCpp() && RecordTok->is(tok::kw_union))
- return true;
- if (Style.isCSharp() && RecordTok->is(Keywords.kw_interface))
- return true;
- if (Style.Language == FormatStyle::LK_Java &&
- RecordTok->is(tok::kw_enum))
- return true;
- if (Style.isJavaScript() && RecordTok->is(Keywords.kw_function))
- return true;
-
- RecordTok = RecordTok->Next;
- }
+ for (const FormatToken *RecordTok = (*J)->Last; RecordTok;
+ RecordTok = RecordTok->Previous)
+ if (RecordTok->is(tok::l_brace))
+ return RecordTok->is(TT_RecordLBrace);
return false;
}
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index b0588d92ab00..35be2fa3eb62 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2482,7 +2482,8 @@ void UnwrappedLineParser::parseNamespace() {
parseParens();
} else {
while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
- tok::l_square, tok::period)) {
+ tok::l_square, tok::period) ||
+ (Style.isCSharp() && FormatTok->is(tok::kw_union))) {
if (FormatTok->is(tok::l_square))
parseSquare();
else
@@ -2868,6 +2869,7 @@ bool UnwrappedLineParser::parseEnum() {
// Just a declaration or something is wrong.
if (FormatTok->isNot(tok::l_brace))
return true;
+ FormatTok->setType(TT_RecordLBrace);
FormatTok->setBlockKind(BK_Block);
if (Style.Language == FormatStyle::LK_Java) {
@@ -3108,6 +3110,7 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
}
}
if (FormatTok->Tok.is(tok::l_brace)) {
+ FormatTok->setType(TT_RecordLBrace);
if (ParseAsExpr) {
parseChildBlock();
} else {
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 9185de023ccf..40db603a4941 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12316,6 +12316,48 @@ TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
" int f() {}\n"
"};",
MergeInlineOnly);
+
+ MergeInlineOnly.BraceWrapping.AfterClass = true;
+ MergeInlineOnly.BraceWrapping.AfterStruct = true;
+ verifyFormat("class C\n"
+ "{\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("struct C\n"
+ "{\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f()\n"
+ "{\n"
+ " return 42;\n"
+ "}",
+ MergeInlineOnly);
+ verifyFormat("int f() {}", MergeInlineOnly);
+ verifyFormat("class C\n"
+ "{\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("struct C\n"
+ "{\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("struct C\n"
+ "// comment\n"
+ "/* comment */\n"
+ "// comment\n"
+ "{\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("/* comment */ struct C\n"
+ "{\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
}
TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index e87270d63865..1684e815b061 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -67,6 +67,30 @@ TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) {
EXPECT_TOKEN(Tokens[11], tok::star, TT_PointerOrReference);
}
+TEST_F(TokenAnnotatorTest, UnderstandsClasses) {
+ auto Tokens = annotate("class C {};");
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
+ auto Tokens = annotate("struct S {};");
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsUnions) {
+ auto Tokens = annotate("union U {};");
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsEnums) {
+ auto Tokens = annotate("enum E {};");
+ EXPECT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_RecordLBrace);
+}
+
} // namespace
} // namespace format
} // namespace clang
More information about the cfe-commits
mailing list