Author: Galen Elias Date: 2025-02-13T20:14:39-08:00 New Revision: 083f099a345f02390d00a8196d4ffa36ae71c82f
URL: https://github.com/llvm/llvm-project/commit/083f099a345f02390d00a8196d4ffa36ae71c82f DIFF: https://github.com/llvm/llvm-project/commit/083f099a345f02390d00a8196d4ffa36ae71c82f.diff LOG: [clang-format] Support BraceWrapping.AfterNamespace with AllowShortNamespacesOnASingleLine (#123010) AllowShortNamespacesOnASingleLine assumes that there is no newline before the namespace brace, however, there is no actual reason this shouldn't be compatible with BraceWrapping.AfterNamespace = true. This is a little tricky in the implementation because UnwrappedLineFormatter works on lines, so being flexible about the offsets is awkward. Not sure if there is a better pattern for combining the 'AllowShort' options with the various configurations of BraceWrapping, but this seemed mostly reasonable. Really, it would almost be preferable to just pattern match on the direct token stream, rather than the AnnotatedLines, but I'm not seeing a straightforward way to do that. --------- Co-authored-by: Owen Pan <owenpi...@gmail.com> Added: Modified: clang/lib/Format/UnwrappedLineFormatter.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index a5b30c85974c7..dd667a9944515 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -366,8 +366,7 @@ class LineJoiner { // instead of TheLine->First. if (Style.AllowShortNamespacesOnASingleLine && - TheLine->First->is(tok::kw_namespace) && - TheLine->Last->is(tok::l_brace)) { + TheLine->First->is(tok::kw_namespace)) { const auto result = tryMergeNamespace(I, E, Limit); if (result > 0) return result; @@ -633,24 +632,37 @@ class LineJoiner { if (Limit == 0) return 0; - assert(I[1]); - const auto &L1 = *I[1]; + // The merging code is relative to the opening namespace brace, which could + // be either on the first or second line due to the brace wrapping rules. + const bool OpenBraceWrapped = Style.BraceWrapping.AfterNamespace; + const auto *BraceOpenLine = I + OpenBraceWrapped; + + assert(*BraceOpenLine); + if (BraceOpenLine[0]->Last->isNot(TT_NamespaceLBrace)) + return 0; + + if (std::distance(BraceOpenLine, E) <= 2) + return 0; + + if (BraceOpenLine[0]->Last->is(tok::comment)) + return 0; + + assert(BraceOpenLine[1]); + const auto &L1 = *BraceOpenLine[1]; if (L1.InPPDirective != (*I)->InPPDirective || (L1.InPPDirective && L1.First->HasUnescapedNewline)) { return 0; } - if (std::distance(I, E) <= 2) - return 0; - - assert(I[2]); - const auto &L2 = *I[2]; + assert(BraceOpenLine[2]); + const auto &L2 = *BraceOpenLine[2]; if (L2.Type == LT_Invalid) return 0; Limit = limitConsideringMacros(I + 1, E, Limit); - if (!nextTwoLinesFitInto(I, Limit)) + const auto LinesToBeMerged = OpenBraceWrapped + 2; + if (!nextNLinesFitInto(I, I + LinesToBeMerged, Limit)) return 0; // Check if it's a namespace inside a namespace, and call recursively if so. @@ -661,17 +673,19 @@ class LineJoiner { assert(Limit >= L1.Last->TotalLength + 3); const auto InnerLimit = Limit - L1.Last->TotalLength - 3; - const auto MergedLines = tryMergeNamespace(I + 1, E, InnerLimit); + const auto MergedLines = + tryMergeNamespace(BraceOpenLine + 1, E, InnerLimit); if (MergedLines == 0) return 0; - const auto N = MergedLines + 2; + const auto N = MergedLines + LinesToBeMerged; // Check if there is even a line after the inner result. if (std::distance(I, E) <= N) return 0; // Check that the line after the inner result starts with a closing brace // which we are permitted to merge into one line. - if (I[N]->First->is(tok::r_brace) && !I[N]->First->MustBreakBefore && - I[MergedLines + 1]->Last->isNot(tok::comment) && + if (I[N]->First->is(TT_NamespaceRBrace) && + !I[N]->First->MustBreakBefore && + BraceOpenLine[MergedLines + 1]->Last->isNot(tok::comment) && nextNLinesFitInto(I, I + N + 1, Limit)) { return N; } @@ -686,11 +700,11 @@ class LineJoiner { return 0; // Last, check that the third line starts with a closing brace. - if (L2.First->isNot(tok::r_brace) || L2.First->MustBreakBefore) + if (L2.First->isNot(TT_NamespaceRBrace) || L2.First->MustBreakBefore) return 0; - // If so, merge all three lines. - return 2; + // If so, merge all lines. + return LinesToBeMerged; } unsigned diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 9b9ce35f83bc5..2365a7c40bf76 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -28867,6 +28867,35 @@ TEST_F(FormatTest, ShortNamespacesOption) { "}}} // namespace foo::bar::baz", "namespace foo { namespace bar { namespace baz { class qux; } } }", Style); + Style.FixNamespaceComments = false; + + Style.BreakBeforeBraces = FormatStyle::BS_Custom; + Style.BraceWrapping.AfterNamespace = true; + verifyFormat("namespace foo { class bar; }", Style); + verifyFormat("namespace foo { namespace bar { class baz; } }", Style); + verifyFormat("namespace foo\n" + "{ // comment\n" + "class bar;\n" + "}", + Style); + verifyFormat("namespace foo { class bar; }", + "namespace foo {\n" + "class bar;\n" + "}", + Style); + verifyFormat("namespace foo\n" + "{\n" + "namespace bar\n" + "{ // comment\n" + "class baz;\n" + "}\n" + "}", + Style); + verifyFormat("namespace foo // comment\n" + "{\n" + "class baz;\n" + "}", + Style); } TEST_F(FormatTest, WrapNamespaceBodyWithEmptyLinesNever) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
