curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fixes https://github.com/llvm/llvm-project/issues/24784.
With config:
AllowShortFunctionsOnASingleLine: Inline
NamespaceIndentation: All
The code:
namespace Test
{
void f()
{
return;
}
}
was incorrectly formatted to:
namespace Test
{
void f() { return; }
}
since the function `f` was considered being inside a class/struct/record.
That's because the check was simplistic and only checked for a non-zero
indentation level of the line starting `f`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D117142
Files:
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestCSharp.cpp
clang/unittests/Format/FormatTestJava.cpp
Index: clang/unittests/Format/FormatTestJava.cpp
===================================================================
--- clang/unittests/Format/FormatTestJava.cpp
+++ clang/unittests/Format/FormatTestJava.cpp
@@ -602,5 +602,16 @@
"}");
}
+TEST_F(FormatTestJava, ShortFunctions) {
+ FormatStyle Style = getLLVMStyle(FormatStyle::LK_Java);
+ Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+ verifyFormat("enum Enum {\n"
+ " E1,\n"
+ " E2;\n"
+ " void f() { return; }\n"
+ "}",
+ Style);
+}
+
} // namespace format
} // end namespace clang
Index: clang/unittests/Format/FormatTestCSharp.cpp
===================================================================
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -1518,5 +1518,32 @@
Style);
}
+TEST_F(FormatTestCSharp, ShortFunctions) {
+ FormatStyle Style = getLLVMStyle(FormatStyle::LK_CSharp);
+ Style.NamespaceIndentation = FormatStyle::NI_All;
+ Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+ verifyFormat("interface Interface {\n"
+ " void f() { return; }\n"
+ "};",
+ Style);
+ verifyFormat("public interface Interface {\n"
+ " void f() { return; }\n"
+ "};",
+ Style);
+ verifyFormat("namespace {\n"
+ " void f() {\n"
+ " return;\n"
+ " }\n"
+ "};",
+ Style);
+ // "union" is not a keyword in C#.
+ verifyFormat("namespace union {\n"
+ " void f() {\n"
+ " return;\n"
+ " }\n"
+ "};",
+ Style);
+}
+
} // namespace format
} // end namespace clang
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3554,6 +3554,86 @@
"} // namespace out",
Style));
+ FormatStyle ShortInlineFunctions = getLLVMStyle();
+ ShortInlineFunctions.NamespaceIndentation = FormatStyle::NI_All;
+ ShortInlineFunctions.AllowShortFunctionsOnASingleLine =
+ FormatStyle::SFS_Inline;
+ verifyFormat("namespace {\n"
+ " void f() {\n"
+ " return;\n"
+ " }\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace {\n"
+ " int some_int;\n"
+ " void f() {\n"
+ " return;\n"
+ " }\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace interface {\n"
+ " void f() {\n"
+ " return;\n"
+ " }\n"
+ "} // namespace interface\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace {\n"
+ " class X {\n"
+ " void f() { return; }\n"
+ " };\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace {\n"
+ " struct X {\n"
+ " void f() { return; }\n"
+ " };\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace {\n"
+ " union X {\n"
+ " void f() { return; }\n"
+ " };\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("extern \"C\" {\n"
+ "void f() {\n"
+ " return;\n"
+ "}\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace {\n"
+ " class X {\n"
+ " void f() { return; }\n"
+ " } x;\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace {\n"
+ " [[nodiscard]] class X {\n"
+ " void f() { return; }\n"
+ " };\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace {\n"
+ " static class X {\n"
+ " void f() { return; }\n"
+ " } x;\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+ verifyFormat("namespace {\n"
+ " constexpr class X {\n"
+ " void f() { return; }\n"
+ " } x;\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+
+ ShortInlineFunctions.IndentExternBlock = FormatStyle::IEBS_Indent;
+ verifyFormat("extern \"C\" {\n"
+ " void f() {\n"
+ " return;\n"
+ " }\n"
+ "} // namespace\n",
+ ShortInlineFunctions);
+
Style.NamespaceIndentation = FormatStyle::NI_Inner;
EXPECT_EQ("namespace out {\n"
"int i;\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -262,14 +262,59 @@
}
}
- // FIXME: TheLine->Level != 0 might or might not be the right check to do.
- // If necessary, change to something smarter.
- bool MergeShortFunctions =
- Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
- (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
- I[1]->First->is(tok::r_brace)) ||
- (Style.AllowShortFunctionsOnASingleLine & FormatStyle::SFS_InlineOnly &&
- TheLine->Level != 0);
+ auto ShouldMergeShortFunctions =
+ [this, B = AnnotatedLines.begin()](
+ SmallVectorImpl<AnnotatedLine *>::const_iterator I) {
+ if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
+ return true;
+ if (Style.AllowShortFunctionsOnASingleLine >=
+ FormatStyle::SFS_Empty &&
+ I[1]->First->is(tok::r_brace))
+ return true;
+
+ if (Style.AllowShortFunctionsOnASingleLine &
+ FormatStyle::SFS_InlineOnly) {
+ // Just checking TheLine->Level != 0 is not enough, because it
+ // provokes treating functions inside intended namespaces as short.
+ if ((*I)->Level != 0) {
+ if (I == B)
+ return false;
+
+ // FIXME: Use IndentTracker to avoid loop?
+ // Find the last line with lower level.
+ auto J = I - 1;
+ for (; J != B; --J)
+ if ((*J)->Level < (*I)->Level)
+ break;
+
+ // Check if the found line starts a record.
+ auto *RecordTok = (*J)->First;
+ while (RecordTok) {
+ // TODO: Refactor to isRecord(RecordTok).
+ // Enums cannot have methods in C and C++, but how about Java?
+ 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;
+ }
+
+ return false;
+ }
+ }
+
+ return false;
+ };
+
+ bool MergeShortFunctions = ShouldMergeShortFunctions(I);
if (Style.CompactNamespaces) {
if (auto nsToken = TheLine->First->getNamespaceToken()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
