Author: Marek Kurdej Date: 2022-01-14T21:57:02+01:00 New Revision: 7af11989be21df00ad6a510a831ea2425e48fa90
URL: https://github.com/llvm/llvm-project/commit/7af11989be21df00ad6a510a831ea2425e48fa90 DIFF: https://github.com/llvm/llvm-project/commit/7af11989be21df00ad6a510a831ea2425e48fa90.diff LOG: [clang-format] Fix short functions being considered as inline inside an indented namespace. 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`. Reviewed By: MyDeveloperDay, HazardyKnusperkeks Differential Revision: https://reviews.llvm.org/D117142 Added: Modified: clang/lib/Format/UnwrappedLineFormatter.cpp clang/unittests/Format/FormatTest.cpp clang/unittests/Format/FormatTestCSharp.cpp clang/unittests/Format/FormatTestJava.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 18d61b4839526..7c447bbfaa5d3 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -262,14 +262,58 @@ class LineJoiner { } } - // 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 indented namespaces as short. + if ((*I)->Level != 0) { + if (I == B) + return false; + + // TODO: 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). + 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()) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index eb6975bbe77e8..27bf1d14e6664 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -3560,6 +3560,86 @@ TEST_F(FormatTest, FormatsNamespaces) { "} // 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" diff --git a/clang/unittests/Format/FormatTestCSharp.cpp b/clang/unittests/Format/FormatTestCSharp.cpp index 2cfec61d8f94b..26c62c145064d 100644 --- a/clang/unittests/Format/FormatTestCSharp.cpp +++ b/clang/unittests/Format/FormatTestCSharp.cpp @@ -1518,5 +1518,32 @@ TEST_F(FormatTestCSharp, EmptyShortBlock) { 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 diff --git a/clang/unittests/Format/FormatTestJava.cpp b/clang/unittests/Format/FormatTestJava.cpp index 84f6420b9e1ac..e778836e0fc9a 100644 --- a/clang/unittests/Format/FormatTestJava.cpp +++ b/clang/unittests/Format/FormatTestJava.cpp @@ -602,5 +602,16 @@ TEST_F(FormatTestJava, RetainsLogicalShifts) { "}"); } +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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
