BreakBeforeBraces: GNU should break before open brace of lambda

[tromey]

I work on a GNU-style code base that uses C++. My test config uses BreakBeforeBraces: GNU because it seems impossible to replicate real GNU-style behavior with BreakBeforeBraces: Custom (in particular conditionally applying IndentBraces).

I think this setting should cause a line break before the { of a lambda.

Test code:

int f(int y) {
  auto x = [&]() {
    for (int i = 0; i < y; ++i)
      return 97;
  };
  return x();
}

clang-format config:

BreakBeforeBraces: GNU

Actual output:

int f(int y)
{
  auto x = [&]() {
    for (int i = 0; i < y; ++i)
      return 97;
  };
  return x();
}

Expected output:

int f(int y)
{
  auto x = [&]()
    {
      for (int i = 0; i < y; ++i)
	return 97;
    };
  return x();
}

[tromey]

Sorry, I should have mentioned this is a clang-format issue.

[llvmbot]

@llvm/issue-subscribers-clang-format

Author: Tom Tromey (tromey)

I work on a GNU-style code base that uses C++. My test config uses `BreakBeforeBraces: GNU` because it seems impossible to replicate real GNU-style behavior with `BreakBeforeBraces: Custom` (in particular conditionally applying `IndentBraces`).

I think this setting should cause a line break before the { of a lambda.

Test code:

int f(int y) {
  auto x = [&]() {
    for (int i = 0; i < y; ++i)
      return 97;
  };
  return x();
}

clang-format config:

BreakBeforeBraces: GNU

Actual output:

int f(int y)
{
  auto x = [&]() {
    for (int i = 0; i < y; ++i)
      return 97;
  };
  return x();
}

Expected output:

int f(int y)
{
  auto x = [&]()
    {
      for (int i = 0; i < y; ++i)
	return 97;
    };
  return x();
}

[owenca]

I agree that the opening brace should be wrapped. Does GNU style say anything about indenting lambda braces?

[rmarker]

I'm not sure about the official GNU style. I've only seen it used with C code myself.
However, at work, we have code that we want to use with clang-format that uses indented braces on lambdas following the spirit of the style.
So the functionality would be appreciated by us.

In fact, coincidentally, I have recently had some available time to look into it.
I think it should be fairly simple to add another option to LambdaBodyIndentation to support the indented style.

I've noticed that LambdaBodyIndentation only applies to nested lambdas.
In our case at work, we would only need the indentation on unnested lambdas. Maybe it would be worth converting LambdaBodyIndentation to a struct with separate options for nested and unnested lambdas, though just another enum value could be enough at this stage?

[tromey]

I agree that the opening brace should be wrapped. Does GNU style say anything about indenting lambda braces?

There's nothing official (GNU barely recognizes the existence of C++), but for gdb we do indent the open brace (just like for if etc) and also came up with https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Indentation_of_lambdas_as_parameters

[owenca]

It seems that there are some bugs that should be fixed first:

$ cat a.cc
for_each_thread ([] (thread_info *thread)
  {
    /* Lambda body.  */
  });
iterate_over_lwps (scope_ptid, [=] (struct lwp_info *info)
  {
    /* Lambda body.  */
  });
expand_symtabs_matching (nullptr, lookup_name, nullptr,
                         [&] (compunit_symtab *symtab)
                           {
                             add_symtab_completions (symtab,
                                                     tracker, mode, lookup_name,
                                                     sym_text, word, code);
                             return true;
                           },
                         SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
                         ALL_DOMAIN);
$ cat .clang-format
BraceWrapping:
  BeforeLambdaBody: true
BreakBeforeBraces: Custom
$ clang-format -version
clang-format version 20.1.2
$ clang-format a.cc
for_each_thread(
    [](thread_info *thread)
    {
      /* Lambda body.  */
    });
iterate_over_lwps(scope_ptid,
                  [=](struct lwp_info *info)
                  {
                    /* Lambda body.  */
                  });
expand_symtabs_matching(
    nullptr, lookup_name, nullptr,
    [&](compunit_symtab *symtab)
    {
      add_symtab_completions(symtab, tracker, mode, lookup_name, sym_text, word,
                             code);
      return true;
    },
    SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK, ALL_DOMAIN);
$ echo 'LambdaBodyIndentation: OuterScope' >> .clang-format
$ clang-format a.cc
for_each_thread([](thread_info *thread)
{
  /* Lambda body.  */
});
iterate_over_lwps(scope_ptid, [=](struct lwp_info *info)
{
  /* Lambda body.  */
});
expand_symtabs_matching(nullptr, lookup_name, nullptr,
                        [&](compunit_symtab *symtab)
{
  add_symtab_completions(symtab, tracker, mode, lookup_name, sym_text, word,
                         code);
  return true;
}, SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK, ALL_DOMAIN);
$ 

[rmarker]

Not sure if I'm just missing something obvious @owenca.
Where are the bugs in the example?

[owenca]

With the default LBI_Signature, there's always a break before the lambda argument whereas with LBI_OuterScope, there's no break after the },.

[rmarker]

I had a look into the first of the issues mentioned.
It looks like the break is caused by the following in ContinuationIndenter::canBreak():

  // Don't create a 'hanging' indent if there are multiple blocks in a single
  // statement and we are aligning lambda blocks to their signatures.
  if (Previous.is(tok::l_brace) && State.Stack.size() > 1 &&
      State.Stack[State.Stack.size() - 2].NestedBlockInlined &&
      State.Stack[State.Stack.size() - 2].HasMultipleNestedBlocks &&
      Style.LambdaBodyIndentation == FormatStyle::LBI_Signature) {
    return false;
  }

The fix could be as simple as adding !Style.BraceWrapping.BeforeLambdaBody to the conditions.
It does require an update to ContinuationIndenter::getNewLineColumn() to then get the braces lining up with the signature, though that seems like it could be handled with using CurrentState.LastSpace for the nested TT_LambdaLBrace.

The main thing that I ran into is that it causes a bunch of tests to change. It seems that several include this eager breaking.
Looking at the comment above, I wonder if there may have been some intention with reducing the preceding whitespace before the lambda body by breaking early?
Personally, I like the changes, but not sure if the formatting change is something that is desired.
Any thoughts @owenca? Would continuing to look into a possible PR be worthwhile in this case?