[PATCH] D153205: [clang-format] Add new block type ListInit

[Owen Pan via Phabricator via cfe-commits]

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