Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-format] AlignAfterOpenBracket = BlockIndent doesn't affect braced lists when Cpp11BracedListStyle = true #57878

Closed
jgcodes2020 opened this issue Sep 21, 2022 · 7 comments
Assignees

Comments

@jgcodes2020
Copy link

jgcodes2020 commented Sep 21, 2022

Description

The documentation states the following:

Fundamentally, C++11 braced lists are formatted exactly like function calls would be formatted in their place. If the braced list follows a name (e.g. a type or variable name), clang-format formats as if the {} were the parentheses of a function call with that name. If there is no name, a zero-length name is assumed.

When setting AlignAfterOpenBracket = BlockIndent, functions that can't fit on one line become:

my_function(
  my_long_parameter_1,
  my_long_parameter_2,
  my_long_parameter_3,
  my_long_parameter_4,
);

If the documentation is actually correct, then setting Cpp11BracedListStyle = true should make braced lists look like:

  constexpr uint32_t cts[64] = {
    my_really_extremely_long_1st_value,
    my_really_extremely_long_2nd_value,
    my_really_extremely_long_3rd_value,
    my_really_extremely_long_4th_value
  };

Instead, it does this:

  constexpr uint32_t cts[64] = {
    my_really_extremely_long_1st_value,
    my_really_extremely_long_2nd_value,
    my_really_extremely_long_3rd_value,
    my_really_extremely_long_4th_value};

Specs

  • clang-format version: 14.0.6
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 21, 2022

@llvm/issue-subscribers-clang-format

@gedare
Copy link
Contributor

gedare commented Nov 14, 2022

@jgcodes2020 this example uses a C89-style array initializer and not a C++11 braced list. I'm not sure this is valid.

I will however note that I think this is a real bug with the following example:

constexpr unsigned int cts[64] { 111111111, 222222222, 333333333, 444444444, 55555555, 666666666, 777777777};

being formatted as

constexpr unsigned int cts[64]{
    111111111,
    222222222,
    333333333,
    444444444,
    55555555,
    666666666,
    777777777,
    888888888};

with the command:
clang-format -style="{BasedOnStyle: LLVM, AlignAfterOpenBracket: BlockIndent, Cpp11BracedListStyle: true, BinPackArguments: false, BinPackParameters: false }"

I don't know that there's any way currently to achieve the style given in your example.

@eternalphane
Copy link

The following patch works for me, but I didn't test extensively.

diff --git a/clang/lib/Format/FormatToken.cpp b/clang/lib/Format/FormatToken.cpp
index f9f0d712b..f46e05b7c 100644
--- a/clang/lib/Format/FormatToken.cpp
+++ b/clang/lib/Format/FormatToken.cpp
@@ -84,7 +84,9 @@ bool FormatToken::opensBlockOrBlockTypeList(const FormatStyle &Style) const {
   return is(TT_ArrayInitializerLSquare) || is(TT_ProtoExtensionLSquare) ||
          (is(tok::l_brace) &&
           (getBlockKind() == BK_Block || is(TT_DictLiteral) ||
-           (!Style.Cpp11BracedListStyle && NestingLevel == 0))) ||
+           ((!Style.Cpp11BracedListStyle ||
+             Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
+            NestingLevel == 0))) ||
          (is(tok::less) && (Style.Language == FormatStyle::LK_Proto ||
                             Style.Language == FormatStyle::LK_TextProto));
 }

@gedare
Copy link
Contributor

gedare commented Jun 17, 2023

That patch doesn't work right. It makes the braced initializers act like blocks, and indents based on IndentWidth rather than on ContinuationIndentWidth. I have a patch in progress. Unfortunately, it is not as simple as treating this = { ... } as a braced list, because the braced list initialization has some peculiar style rules relating to the first line comment in the list, for example implemented at https://github.com/llvm/llvm-project/blob/1797ab36efc9c90c921cd725831f8c3f6a7125a2/clang/lib/Format/ContinuationIndenter.cpp#LL752C74-L752C74
and at

// The first comment in a braced lists is always interpreted as belonging to

This in part seems to cause the first comment inside of { ... } identified as a braced list to be consumed/aligned according to the rules associated with the first element of the braced list. From what I can tell, that behavior is not desired for = { ... } code. So it requires some care to separate these cases.

Furthermore, how this should interact with the array of structure initializers (AlignArrayOfStructures) is unclear to me. I suspect they should be handled separately.

@gedare
Copy link
Contributor

gedare commented Jun 17, 2023

I have submitted https://reviews.llvm.org/D153205 that adds this support.

@HazardyKnusperkeks HazardyKnusperkeks added the awaiting-review Has pending Phabricator review label Jun 21, 2023
@owenca
Copy link
Contributor

owenca commented Jun 22, 2023

Description

The documentation states the following:

Fundamentally, C++11 braced lists are formatted exactly like function calls would be formatted in their place. If the braced list follows a name (e.g. a type or variable name), clang-format formats as if the {} were the parentheses of a function call with that name. If there is no name, a zero-length name is assumed.

When setting AlignAfterOpenBracket = BlockIndent, functions that can't fit on one line become:

my_function(
  my_long_parameter_1,
  my_long_parameter_2,
  my_long_parameter_3,
  my_long_parameter_4,
);

If the documentation is actually correct, then setting Cpp11BracedListStyle = true should make braced lists look like:

  constexpr uint32_t cts[64] = {
    my_really_extremely_long_1st_value,
    my_really_extremely_long_2nd_value,
    my_really_extremely_long_3rd_value,
    my_really_extremely_long_4th_value
  };

Instead, it does this:

  constexpr uint32_t cts[64] = {
    my_really_extremely_long_1st_value,
    my_really_extremely_long_2nd_value,
    my_really_extremely_long_3rd_value,
    my_really_extremely_long_4th_value};

Specs

  • clang-format version: 14.0.6

BlockIndent does not apply to curly braces. Please see the warning at the end of this section.

@eternalphane
Copy link

BlockIndent does not apply to curly braces. Please see the warning at the end of this section.

However clang-format will treat braces as parentheses when Cpp11BracedListStyle = true (as quoted below).

Fundamentally, C++11 braced lists are formatted exactly like function calls would be formatted in their place. If the braced list follows a name (e.g. a type or variable name), clang-format formats as if the {} were the parentheses of a function call with that name. If there is no name, a zero-length name is assumed.

@owenca owenca closed this as completed in 413a7cb Jul 6, 2023
@owenca owenca removed the awaiting-review Has pending Phabricator review label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants