[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Björn Schäpers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG966f24e5a62a: [clang-format] Add a BlockIndent option to 
AlignAfterOpenBracket (authored by csmulhern, committed by HazardyKnusperkeks).

Changed prior to commit:
  https://reviews.llvm.org/D109557?vs=400435=400452#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19167,6 +19167,8 @@
   FormatStyle::BAS_DontAlign);
   CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracket,
   FormatStyle::BAS_AlwaysBreak);
+  CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracket,
+  FormatStyle::BAS_BlockIndent);
   // For backward compatibility:
   CHECK_PARSE("AlignAfterOpenBracket: false", AlignAfterOpenBracket,
   FormatStyle::BAS_DontAlign);
@@ -23696,6 +23698,224 @@
Style);
 }
 
+TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
+  auto Style = getLLVMStyle();
+
+  StringRef Short = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int a, int b, int c);";
+
+  StringRef Medium = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+ "paramF, paramG, paramH, paramI);\n"
+ "void functionDecl(int argumentA, int argumentB, int "
+ "argumentC, int argumentD, int argumentE);";
+
+  verifyFormat(Short, Style);
+
+  StringRef NoBreak = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+  "paramF, paramG, paramH,\n"
+  " paramI);\n"
+  "void functionDecl(int argumentA, int argumentB, int "
+  "argumentC, int argumentD,\n"
+  "  int argumentE);";
+
+  verifyFormat(NoBreak, Medium, Style);
+  verifyFormat(NoBreak,
+   "functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Style);
+
+  verifyFormat("outerFunctionCall(nestedFunctionCall(argument1),\n"
+   "  nestedLongFunctionCall(argument1, "
+   "argument2, argument3,\n"
+   " argument4, "
+   "argument5));",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+
+  verifyFormat(Short, Style);
+
+  verifyFormat("functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern added a comment.

In D109557#3247167 , 
@HazardyKnusperkeks wrote:

> Could you please rebase the patch? I promise I will commit it **very** 
> shortly afterwards.

Done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern updated this revision to Diff 400435.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19167,6 +19167,8 @@
   FormatStyle::BAS_DontAlign);
   CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracket,
   FormatStyle::BAS_AlwaysBreak);
+  CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracket,
+  FormatStyle::BAS_BlockIndent);
   // For backward compatibility:
   CHECK_PARSE("AlignAfterOpenBracket: false", AlignAfterOpenBracket,
   FormatStyle::BAS_DontAlign);
@@ -23696,6 +23698,224 @@
Style);
 }
 
+TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
+  auto Style = getLLVMStyle();
+
+  StringRef Short = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int a, int b, int c);";
+
+  StringRef Medium = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+ "paramF, paramG, paramH, paramI);\n"
+ "void functionDecl(int argumentA, int argumentB, int "
+ "argumentC, int argumentD, int argumentE);";
+
+  verifyFormat(Short, Style);
+
+  StringRef NoBreak = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+  "paramF, paramG, paramH,\n"
+  " paramI);\n"
+  "void functionDecl(int argumentA, int argumentB, int "
+  "argumentC, int argumentD,\n"
+  "  int argumentE);";
+
+  verifyFormat(NoBreak, Medium, Style);
+  verifyFormat(NoBreak,
+   "functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Style);
+
+  verifyFormat("outerFunctionCall(nestedFunctionCall(argument1),\n"
+   "  nestedLongFunctionCall(argument1, "
+   "argument2, argument3,\n"
+   " argument4, "
+   "argument5));",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+
+  verifyFormat(Short, Style);
+
+  verifyFormat("functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Medium, Style);
+
+  verifyFormat("outerFunctionCall(\n"
+   "nestedFunctionCall(argument1),\n"
+   "nestedLongFunctionCall(\n"
+   "argument1,\n"
+   "argument2,\n"
+   "argument3,\n"
+  

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Could you please rebase the patch? I promise I will commit it **very** shortly 
afterwards.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D109557#3242274 , @fdwr wrote:

>> I think no one just had thought about it. :)
>
> @HazardyKnusperkeks Happy new year. Just in case it gets forgotten, I'm 
> squeaking on behalf of a few stackoverflowers  (#52158077 
> ,
>  #50689027 
> ).
>  Thanks.

You're completely correct, it went under. I'll definitely commit this weekend. 
:)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Dwayne Robinson via Phabricator via cfe-commits]

fdwr added a comment.

> I think no one just had thought about it. :)

@HazardyKnusperkeks Happy new year. Just in case it gets forgotten, I'm 
squeaking on behalf of a few stackoverflowers  (#52158077 
,
 #50689027 
).
 Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Actually no. I think no one just had thought about it. :)
I can try to commit it next week, this one I'm rather busy. Maybe someone else 
might do it before that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern added a comment.

Is there anything outstanding you'd like me to address, or can we move ahead 
with committing this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern marked an inline comment as done.
csmulhern added a comment.

I don't have commit access, so please submit this change on my behalf.

Thanks for all your help on landing this!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern marked 4 inline comments as done.
csmulhern added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:951
 
+  if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
+State.Stack.back().BreakBeforeClosingParen =

owenpan wrote:
> HazardyKnusperkeks wrote:
> > Remove the braces
> I think the braces should not be removed here. The [[ 
> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
>  | LLVM Coding Standards ]] says "braces should be used when a 
> single-statement body is complex enough" which is rather subjective, but line 
> wrapping is an objective metric and perhaps the only one we can apply.
> 
> 
> 
I'm not sure either way, but the prevailing style in this file seems like it 
would support having no braces, so I went ahead and made the change.



Comment at: clang/unittests/Format/FormatTest.cpp:22453
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllConstructorInitializersOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;

HazardyKnusperkeks wrote:
> How does it behave without these?
It should put the closing parenthesis on a newline as long as it was opened 
with a newline. I've added additional test cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern updated this revision to Diff 387025.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18706,6 +18706,8 @@
   FormatStyle::BAS_DontAlign);
   CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracket,
   FormatStyle::BAS_AlwaysBreak);
+  CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracket,
+  FormatStyle::BAS_BlockIndent);
   // For backward compatibility:
   CHECK_PARSE("AlignAfterOpenBracket: false", AlignAfterOpenBracket,
   FormatStyle::BAS_DontAlign);
@@ -22566,6 +22568,224 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
+  auto Style = getLLVMStyle();
+
+  StringRef Short = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int a, int b, int c);";
+
+  StringRef Medium = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+ "paramF, paramG, paramH, paramI);\n"
+ "void functionDecl(int argumentA, int argumentB, int "
+ "argumentC, int argumentD, int argumentE);";
+
+  verifyFormat(Short, Style);
+
+  StringRef NoBreak = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+  "paramF, paramG, paramH,\n"
+  " paramI);\n"
+  "void functionDecl(int argumentA, int argumentB, int "
+  "argumentC, int argumentD,\n"
+  "  int argumentE);";
+
+  verifyFormat(NoBreak, Medium, Style);
+  verifyFormat(NoBreak,
+   "functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Style);
+
+  verifyFormat("outerFunctionCall(nestedFunctionCall(argument1),\n"
+   "  nestedLongFunctionCall(argument1, "
+   "argument2, argument3,\n"
+   " argument4, "
+   "argument5));",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+
+  verifyFormat(Short, Style);
+
+  verifyFormat("functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Medium, Style);
+
+  verifyFormat("outerFunctionCall(\n"
+   "nestedFunctionCall(argument1),\n"
+   "nestedLongFunctionCall(\n"
+   "argument1,\n"
+   "argument2,\n"
+   "

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:951
 
+  if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
+State.Stack.back().BreakBeforeClosingParen =

HazardyKnusperkeks wrote:
> Remove the braces
I think the braces should not be removed here. The [[ 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
 | LLVM Coding Standards ]] says "braces should be used when a single-statement 
body is complex enough" which is rather subjective, but line wrapping is an 
objective metric and perhaps the only one we can apply.





Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D109557#3066854 , @csmulhern wrote:

> In D109557#3063681 , 
> @MyDeveloperDay wrote:
>
>> Personally I'm not convinced there wouldn't be people who want this for 
>> function declarations and definitions
>>
>>   Function(
>>   param1,
>>   param2,
>>   param3
>>   );
>>
>> but don't want this
>>
>>   if (
>>  foo()
>>   )
>>   
>
> To be clear, the if styling above would only occur if `foo()` was long enough 
> to line wrap. Not all instances of `if`. However, I agree that could be true, 
> and the existing clang-format code clearly treats indentation inside if / for 
> / while differently than e.g. function calls. The existing 
> AlignAfterOpenBracket options for example do not apply to the former for 
> instance, which is why we see the weird indentation in the current revision 
> where the opening brace is not followed by a line break, but the closing 
> brace is preceded by one.

So we would still have the chance for a breaking if condition? There should be 
a test to document that! But I'm also not sure this should happen, but lets 
see, nobody is forced to chose `BlockIndent`.

Otherwise this is fine for me.




Comment at: clang/include/clang/Format/Format.h:100
+///
+/// Note: This currently only applies to parentheses.
+BAS_BlockIndent,

Maybe highlight this with the warning block?
Or correct it. ;)



Comment at: clang/lib/Format/ContinuationIndenter.cpp:951
 
+  if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
+State.Stack.back().BreakBeforeClosingParen =

Remove the braces



Comment at: clang/unittests/Format/FormatTest.cpp:22453
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllConstructorInitializersOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;

How does it behave without these?



Comment at: clang/unittests/Format/FormatTest.cpp:22560
+
+  verifyFormat("if (quitelongarg !=\n"
+   "(alsolongarg - 1)) { // ABC is a very long "

I think you should add a test without the comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern added a comment.

Any further feedback on the latest iteration?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern updated this revision to Diff 381974.
csmulhern marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18539,6 +18539,8 @@
   FormatStyle::BAS_DontAlign);
   CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracket,
   FormatStyle::BAS_AlwaysBreak);
+  CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracket,
+  FormatStyle::BAS_BlockIndent);
   // For backward compatibility:
   CHECK_PARSE("AlignAfterOpenBracket: false", AlignAfterOpenBracket,
   FormatStyle::BAS_DontAlign);
@@ -22397,6 +22399,192 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
+  auto Style = getLLVMStyle();
+
+  StringRef Short = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int a, int b, int c);";
+
+  StringRef Medium = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+ "paramF, paramG, paramH, paramI);\n"
+ "void functionDecl(int argumentA, int argumentB, int "
+ "argumentC, int argumentD, int argumentE);";
+
+  verifyFormat(Short, Style);
+
+  StringRef NoBreak = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+  "paramF, paramG, paramH,\n"
+  " paramI);\n"
+  "void functionDecl(int argumentA, int argumentB, int "
+  "argumentC, int argumentD,\n"
+  "  int argumentE);";
+
+  verifyFormat(NoBreak, Medium, Style);
+  verifyFormat(NoBreak,
+   "functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Style);
+
+  verifyFormat("outerFunctionCall(nestedFunctionCall(argument1),\n"
+   "  nestedLongFunctionCall(argument1, "
+   "argument2, argument3,\n"
+   " argument4, "
+   "argument5));",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllConstructorInitializersOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+
+  verifyFormat(Short, Style);
+
+  verifyFormat("functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Medium, Style);
+
+  verifyFormat("outerFunctionCall(\n"
+   "nestedFunctionCall(argument1),\n"
+   "nestedLongFunctionCall(\n"
+   "argument1,\n"
+   "argument2,\n"
+   "argument3,\n"
+   "argument4,\n"
+   "argument5\n"
+   ")\n"
+   ");",
+   Style);
+
+  verifyFormat("int a = (int)b;", Style);
+  verifyFormat("int a = (int)b;",
+

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:227
   correctly places the opening brace according to ``BraceWrapping.AfterEnum``.
+- Option ``AlignAfterOpenBracket: BlockIndent`` has been added. If set, it will
+  always break after an open bracket, if the parameters don't fit on a single

Add a new line before.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4207-4212
+  if (Right.is(tok::r_paren) && Right.MatchingParen &&
+  !(Right.MatchingParen->Previous &&
+(Right.MatchingParen->Previous->is(tok::kw_for) ||
+ Right.MatchingParen->Previous->isIf(
+return Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent;
+

I'm not sure if this is sufficient. Before we returned always false. Shouldn't 
we use 2 ifs here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Cameron Mulhern via Phabricator via cfe-commits]

csmulhern updated this revision to Diff 381765.
csmulhern added a comment.

Absent feedback, I have gone ahead and added a BlockIndent option to 
AlignAfterOpenBracket.

Please take a look.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109557/new/

https://reviews.llvm.org/D109557

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18539,6 +18539,8 @@
   FormatStyle::BAS_DontAlign);
   CHECK_PARSE("AlignAfterOpenBracket: AlwaysBreak", AlignAfterOpenBracket,
   FormatStyle::BAS_AlwaysBreak);
+  CHECK_PARSE("AlignAfterOpenBracket: BlockIndent", AlignAfterOpenBracket,
+  FormatStyle::BAS_BlockIndent);
   // For backward compatibility:
   CHECK_PARSE("AlignAfterOpenBracket: false", AlignAfterOpenBracket,
   FormatStyle::BAS_DontAlign);
@@ -22397,6 +22399,192 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, AlignAfterOpenBracketBlockIndent) {
+  auto Style = getLLVMStyle();
+
+  StringRef Short = "functionCall(paramA, paramB, paramC);\n"
+"void functionDecl(int a, int b, int c);";
+
+  StringRef Medium = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+ "paramF, paramG, paramH, paramI);\n"
+ "void functionDecl(int argumentA, int argumentB, int "
+ "argumentC, int argumentD, int argumentE);";
+
+  verifyFormat(Short, Style);
+
+  StringRef NoBreak = "functionCall(paramA, paramB, paramC, paramD, paramE, "
+  "paramF, paramG, paramH,\n"
+  " paramI);\n"
+  "void functionDecl(int argumentA, int argumentB, int "
+  "argumentC, int argumentD,\n"
+  "  int argumentE);";
+
+  verifyFormat(NoBreak, Medium, Style);
+  verifyFormat(NoBreak,
+   "functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Style);
+
+  verifyFormat("outerFunctionCall(nestedFunctionCall(argument1),\n"
+   "  nestedLongFunctionCall(argument1, "
+   "argument2, argument3,\n"
+   " argument4, "
+   "argument5));",
+   Style);
+
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_BlockIndent;
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllConstructorInitializersOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
+
+  verifyFormat(Short, Style);
+  verifyFormat(
+  "functionCall(\n"
+  "paramA, paramB, paramC, paramD, paramE, paramF, paramG, paramH, "
+  "paramI\n"
+  ");\n"
+  "void functionDecl(\n"
+  "int argumentA, int argumentB, int argumentC, int argumentD, int "
+  "argumentE\n"
+  ");",
+  Medium, Style);
+
+  Style.BinPackArguments = false;
+  Style.BinPackParameters = false;
+
+  verifyFormat(Short, Style);
+
+  verifyFormat("functionCall(\n"
+   "paramA,\n"
+   "paramB,\n"
+   "paramC,\n"
+   "paramD,\n"
+   "paramE,\n"
+   "paramF,\n"
+   "paramG,\n"
+   "paramH,\n"
+   "paramI\n"
+   ");\n"
+   "void functionDecl(\n"
+   "int argumentA,\n"
+   "int argumentB,\n"
+   "int argumentC,\n"
+   "int argumentD,\n"
+   "int argumentE\n"
+   ");",
+   Medium, Style);
+
+  verifyFormat("outerFunctionCall(\n"
+   "nestedFunctionCall(argument1),\n"
+   "nestedLongFunctionCall(\n"
+   "argument1,\n"
+   "argument2,\n"
+   "argument3,\n"
+   "argument4,\n"
+   "argument5\n"
+   ")\n"
+   ");",
+