[clang-format] PR50525 doesn't handle AlignConsecutiveAssignments correctly in some situations

[MyDeveloperDay]

Summary

https://bugs.llvm.org/show_bug.cgi?id=50525

AlignConsecutiveAssignments/Declarations cause incorrect alignment in the presence of a DesignatedInitializerPeriod (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html)

static NTSTATUS stg(PLW_STREAM Stream, int identity)
{
     NTSTATUS             status;
     BYTE                 payload[256] = {'l', 'h', 'o', 't', 's', 'e'};
     struct dm_rpc_header header       = {.drh_magic        = DRH_MAGIC,
                                    .drh_op_code      = RPC_OP_ECHO,
                                    .drh_payload_size = sizeof(payload),
                                    .drh_body_size    = sizeof(payload),
                                    .drh_request_id   = 1};
     header.drh_version                = identity;

This fix addresses that by ensuring the period isn't ignored

[HazardyKnusperkeks]

Nice. :)

[darwin]

Sorry I haven't had a chance to look at this bug before it has closed. But I do have a question:

I observed that this code are formatted incorrectly by the same config:

llvm::Optional<std::string> CurrentCode = None;
auto                        Env = std::make_unique<Environment>(Code,
                                         FileName,
                                         Ranges,
                                         FirstStartColumn,
                                         NextStartColumn,
                                         LastStartColumn);

Can this solution solve this issue? I am afraid not.

If no, should I open a new bug for it?

[HazardyKnusperkeks]

In D104900#2842540, @darwin wrote:

Sorry I haven't had a chance to look at this bug before it has closed. But I do have a question:

I observed that this code are formatted incorrectly by the same config:

llvm::Optional<std::string> CurrentCode = None;
auto                        Env = std::make_unique<Environment>(Code,
                                         FileName,
                                         Ranges,
                                         FirstStartColumn,
                                         NextStartColumn,
                                         LastStartColumn);

Can this solution solve this issue? I am afraid not.

If no, should I open a new bug for it?

No it doesn't, this change is only about designated initializers.

[MyDeveloperDay]

In D104900#2842540, @darwin wrote:

Sorry I haven't had a chance to look at this bug before it has closed. But I do have a question:

I observed that this code are formatted incorrectly by the same config:

llvm::Optional<std::string> CurrentCode = None;
auto                        Env = std::make_unique<Environment>(Code,
                                         FileName,
                                         Ranges,
                                         FirstStartColumn,
                                         NextStartColumn,
                                         LastStartColumn);

Can this solution solve this issue? I am afraid not.

If no, should I open a new bug for it?

If this is indeed a bug you need to log its, I think it highly unlikey that its related, make sure you include your .clang-format file (and remove non impacting options)

I tested this example and it looked to work correctly (but it depends on your options I suspect):

Language: Cpp
BasedOnStyle: LLVM
AlignConsecutiveDeclarations: true
AlignConsecutiveAssignments: true
ColumnLimit: 80

llvm::Optional<std::string> CurrentCode = None;
auto                        Env         = std::make_unique<Environment>(
    Code, FileName, Ranges, FirstStartColumn, NextStartColumn, LastStartColumn);

clang/lib/Format/WhitespaceManager.cpp

if (InsideNestedScope && Changes[i].NewlinesBefore > 0) {
             Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
             Changes[ScopeStart - 1].Tok->is(tok::l_paren))
           return true;
 
         // Ternary operator
         if (Changes[i].Tok->is(TT_ConditionalExpr))
           return true;
 
+        // Period Initializer .XXX = 1.
+        if (Changes[i].Tok->is(TT_DesignatedInitializerPeriod))
+          return true;
+
         // Continued ternary operator
         if (Changes[i].Tok->Previous &&
             Changes[i].Tok->Previous->is(TT_ConditionalExpr))
           return true;
 
         return false;
       };
 

clang/unittests/Format/FormatTest.cpp

verifyFormat("void foo() {\n"
                "                                                  std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
                "  const X    newEntry2      = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
                "                                                  std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
                "}",
                Style);
   // clang-format on
 }
 
+TEST_F(FormatTest, AlignWithInitializerPeriods) {
+  auto Style = getLLVMStyleWithColumns(60);
+
+  verifyFormat("void foo1(void) {\n"
+               "  BYTE p[1] = 1;\n"
+               "  A B = {.one_foooooooooooooooo = 2,\n"
+               "         .two_fooooooooooooo = 3,\n"
+               "         .three_fooooooooooooo = 4};\n"
+               "  BYTE payload = 2;\n"
+               "}",
+               Style);
+
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
+  verifyFormat("void foo2(void) {\n"
+               "  BYTE p[1]    = 1;\n"
+               "  A B          = {.one_foooooooooooooooo = 2,\n"
+               "                  .two_fooooooooooooo    = 3,\n"
+               "                  .three_fooooooooooooo  = 4};\n"
+               "  BYTE payload = 2;\n"
+               "}",
+               Style);
+
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_None;
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("void foo3(void) {\n"
+               "  BYTE p[1] = 1;\n"
+               "  A    B = {.one_foooooooooooooooo = 2,\n"
+               "            .two_fooooooooooooo = 3,\n"
+               "            .three_fooooooooooooo = 4};\n"
+               "  BYTE payload = 2;\n"
+               "}",
+               Style);
+
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("void foo4(void) {\n"
+               "  BYTE p[1]    = 1;\n"
+               "  A    B       = {.one_foooooooooooooooo = 2,\n"
+               "                  .two_fooooooooooooo    = 3,\n"
+               "                  .three_fooooooooooooo  = 4};\n"
+               "  BYTE payload = 2;\n"
+               "}",
+               Style);
+}
+
 TEST_F(FormatTest, LinuxBraceBreaking) {
   FormatStyle LinuxBraceStyle = getLLVMStyle();
   LinuxBraceStyle.BreakBeforeBraces = FormatStyle::BS_Linux;
   verifyFormat("namespace a\n"
                "{\n"
                "class A\n"
                "{\n"
                "  void f()\n"