owenpan added a comment. In D153205#4438782 <https://reviews.llvm.org/D153205#4438782>, @HazardyKnusperkeks wrote:
> In D153205#4437966 <https://reviews.llvm.org/D153205#4437966>, > @MyDeveloperDay wrote: > >> My only concern is that this changes behaviour without someone making the >> conscious decision to make that change, now as much as I can't understand >> why someone would want to put the `}` onto the same line, it will only take >> on person to say "hey I liked it like that" to complain and call it a >> regression, or that we somehow change the defaults (which people always >> complain about even through we don't normally do that) >> >> In the past our solution to this problem is to have an option with a "Leave" >> setting to sort of say, leave it as it currently is. In this case it feels >> like a bug, looks like a bug and is probably a bug, but I'd be interested in >> @owenpan and @HazardyKnusperkeks's view > > The github issue basically says all there's to say, if it those constructs > should be formatted like function calls, then that's what should happen. Or > we should adapt the documentation. There is a warning in the document. (See here <https://github.com/llvm/llvm-project/issues/57878#issuecomment-1603407177>.) It should be updated in this patch. I understand the concern raised by @MyDeveloperDay, but given that `BAS_AlwaysBreak` already covers the style of breaking after the opening brace but not before the closing brace, I'm okay with this change. ================ Comment at: clang/lib/Format/ContinuationIndenter.cpp:365-367 + (Current.is(tok::r_brace) && Style.Cpp11BracedListStyle && + Current.MatchingParen->isOneOf(BK_BracedInit, BK_ListInit) && + Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent))) { ---------------- And `isBlockIndentedInitRBrace()` returns true only if the matching `l_brace` is of `BK_BracedInit` or preceded by an `=`. ================ Comment at: clang/lib/Format/ContinuationIndenter.cpp:1175 + (Current.is(tok::r_brace) && + Current.MatchingParen->isOneOf(BK_BracedInit, BK_ListInit))) && + State.Stack.size() > 1) { ---------------- All unit tests still pass. ================ Comment at: clang/lib/Format/FormatToken.cpp:84-89 + // Indent C/C++ initializer lists as continuations. + if (is(tok::l_brace) && + (getBlockKind() == BK_BracedInit || getBlockKind() == BK_ListInit) && + Style.Cpp11BracedListStyle && Style.Language == FormatStyle::LK_Cpp) { + return false; + } ---------------- This seems redundant and can be deleted. ================ Comment at: clang/lib/Format/FormatToken.h:185 // Represents what type of block a set of braces open. -enum BraceBlockKind { BK_Unknown, BK_Block, BK_BracedInit }; +enum BraceBlockKind { BK_Unknown, BK_Block, BK_BracedInit, BK_ListInit }; ---------------- We don't need to add a new block kind. Instead, checking if the previous token is `=` should work. ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:859 Contexts.back().ColonIsDictLiteral = true; - if (OpeningBrace.is(BK_BracedInit)) + if (OpeningBrace.isOneOf(BK_BracedInit, BK_ListInit)) Contexts.back().IsExpression = true; ---------------- Either `BK_ListInit` is superfluous here or a unit test is missing. ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:4206 return Right.is(tok::coloncolon); - if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) && + if (Right.is(tok::l_brace) && Right.isOneOf(BK_BracedInit, BK_ListInit) && !Left.opensScope() && Style.SpaceBeforeCpp11BracedList) { ---------------- Ditto. ================ Comment at: clang/lib/Format/TokenAnnotator.cpp:5448-5450 + (Right.MatchingParen->isOneOf(BK_BracedInit, BK_ListInit) && + Style.Cpp11BracedListStyle && + Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent)); ---------------- ================ Comment at: clang/unittests/Format/FormatTest.cpp:5042 " SomeArrayT{},\n" - "}\n", + "};\n", Style); ---------------- Ditto below. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153205/new/ https://reviews.llvm.org/D153205 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
