[clang-format] add option for dangling parenthesis

[stringham]

Summary

This adds an option to clang-format to support dangling parenthesis.

If you have a parameter list like

Foo(
    param1,
    param2,
    param3,
);

clang-format currently only supports putting the closing paren on the same line as param3:

Foo(
    param1,
    param2,
    param3, );

This makes it difficult to see the change from the function signature to the function body, for example:

class Foo extends Bar {
    constructor(
        param1,
        param2,
        param3, ) {
        super(param1, 'x');
        this.param2 = param2;
    }
}

would now be formatted like:

class Foo extends Bar {
    constructor(
        param1,
        param2,
        param3, 
    ) {
        super(param1, 'x');
        this.param2 = param2;
    }
}

and it is much easier to see the difference between the parameter list and the function body.

[owenpan]

@stringham Here is what I did to run the unit tests:

cd build
make FormatTests
tools/clang/unittests/Format/FormatTests

[stringham]

@djasper can you give some direction here?

Would you be okay with me extending the BracketAlignmentStyle option?
What do I need to do to get this change merged?

[sciencemanx]

@djasper bump -- this feature is also really important to our team.

[DevAlone]

So, will it be added?

[MyDeveloperDay]

This review lack unit tests, please add some for FormatTest to show its use cases, otherwise someone else will come along and break this later

[stringham]

@MyDeveloperDay I am happy to write unit tests and clean up the change set but I don't want to spend more time on this if it has no chance of being merged.

I am looking for some agreement that this feature is worth adding to clang format and then I'd like to know that my general solution is reasonable and agree on what we should call the option.

Currently, we're just using a fork with this patch applied. It would be nice to get the changes merged upstream, but I don't know how to make that happen.

[MyDeveloperDay]

Adding the unit tests lets us see how this option will work in various cases, it will let us understand that its not breaking anything else.

I personally don't like to see revisions like this sit for 2 years with nothing happening, I don't see anything wrong with this that would prevent it going in so I don't understand whats blocking it?,

if you had some tests and a release note I'd give it a LGTM (but as I've said before I'm not the code owner, but someone wanting to address defects and add capabilities. but I think we need to be able to move forward, people will object soon enough if they don't like it.)

Generally I don't understand why clang-format is so reluctant to change anything, As such we don't have many people involved and getting anything done (even defects) is extremely hard.

It looks like you met the criteria, and reviewers have been given ample opportunity to make an objection. the number of subscribers and like tokens would suggest this is wanted,

Please also add a line the in the release notes to say what you are adding. In the absence of any other constructive input all we can do is follow the guidance on doing a review, for what its worth I notice in the rest of LLVM there seems to be a much larger amount of commits that go in even without a review, I'm not sure what makes this area so strict, so reluctant to change especailly when revisions do seem to be reviewed.

[lebedev.ri]

In D33029#1423944, @MyDeveloperDay wrote:

Adding the unit tests lets us see how this option will work in various cases, it will let us understand that its not breaking anything else.

I personally don't like to see revisions like this sit for 2 years with nothing happening, I don't see anything wrong with this that would prevent it going in so I don't understand whats blocking it?,

if you had some tests and a release note I'd give it a LGTM (but as I've said before I'm not the code owner, but someone wanting to address defects and add capabilities. but I think we need to be able to move forward, people will object soon enough if they don't like it.)

Generally I don't understand why clang-format is so reluctant to change anything, As such we don't have many people involved and getting anything done (even defects) is extremely hard.

It looks like you met the criteria, and reviewers have been given ample opportunity to make an objection. the number of subscribers and like tokens would suggest this is wanted,

Please also add a line the in the release notes to say what you are adding. In the absence of any other constructive input all we can do is follow the guidance on doing a review, for what its worth I notice in the rest of LLVM there seems to be a much larger amount of commits that go in even without a review, I'm not sure what makes this area so strict, so reluctant to change especailly when revisions do seem to be reviewed.

I don't have any stake here, but i just want to point out that no tool (including clang-format)
will ever support all the things all the people out there will want it to support. But every
new knob is not just a single knob, it needs to work well with all the other existing knobs
(and all of the combination of knob params), and every new knob after that.

It's a snowball effect. Things can (and likely will, unless there is at least a *very* strict testing/quality policy
(which is https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options about))
get out of hand real quickly..

Just 2c.

[MyDeveloperDay]

In D33029#1423947, @lebedev.ri wrote:

In D33029#1423944, @MyDeveloperDay wrote:

Adding the unit tests lets us see how this option will work in various cases, it will let us understand that its not breaking anything else.

I personally don't like to see revisions like this sit for 2 years with nothing happening, I don't see anything wrong with this that would prevent it going in so I don't understand whats blocking it?,

if you had some tests and a release note I'd give it a LGTM (but as I've said before I'm not the code owner, but someone wanting to address defects and add capabilities. but I think we need to be able to move forward, people will object soon enough if they don't like it.)

Generally I don't understand why clang-format is so reluctant to change anything, As such we don't have many people involved and getting anything done (even defects) is extremely hard.

It looks like you met the criteria, and reviewers have been given ample opportunity to make an objection. the number of subscribers and like tokens would suggest this is wanted,

Please also add a line the in the release notes to say what you are adding. In the absence of any other constructive input all we can do is follow the guidance on doing a review, for what its worth I notice in the rest of LLVM there seems to be a much larger amount of commits that go in even without a review, I'm not sure what makes this area so strict, so reluctant to change especailly when revisions do seem to be reviewed.

I don't have any stake here, but i just want to point out that no tool (including clang-format)
will ever support all the things all the people out there will want it to support. But every
new knob is not just a single knob, it needs to work well with all the other existing knobs
(and all of the combination of knob params), and every new knob after that.

It's a snowball effect. Things can (and likely will, unless there is at least a *very* strict testing/quality policy
(which is https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options about))
get out of hand real quickly..

Just 2c.

Correct we should always be adding tests and show we don't break existing tests... We need to apply the "Beyonce Rule".. "if you liked it you should have put a test on it."

We shouldn't just give up making improvements or fixing defects because something is hard or complex.

[klimek]

In D33029#1423949, @MyDeveloperDay wrote:

In D33029#1423947, @lebedev.ri wrote:

In D33029#1423944, @MyDeveloperDay wrote:

Adding the unit tests lets us see how this option will work in various cases, it will let us understand that its not breaking anything else.

I personally don't like to see revisions like this sit for 2 years with nothing happening, I don't see anything wrong with this that would prevent it going in so I don't understand whats blocking it?,

if you had some tests and a release note I'd give it a LGTM (but as I've said before I'm not the code owner, but someone wanting to address defects and add capabilities. but I think we need to be able to move forward, people will object soon enough if they don't like it.)

Generally I don't understand why clang-format is so reluctant to change anything, As such we don't have many people involved and getting anything done (even defects) is extremely hard.

It looks like you met the criteria, and reviewers have been given ample opportunity to make an objection. the number of subscribers and like tokens would suggest this is wanted,

Please also add a line the in the release notes to say what you are adding. In the absence of any other constructive input all we can do is follow the guidance on doing a review, for what its worth I notice in the rest of LLVM there seems to be a much larger amount of commits that go in even without a review, I'm not sure what makes this area so strict, so reluctant to change especailly when revisions do seem to be reviewed.

I don't have any stake here, but i just want to point out that no tool (including clang-format)
will ever support all the things all the people out there will want it to support. But every
new knob is not just a single knob, it needs to work well with all the other existing knobs
(and all of the combination of knob params), and every new knob after that.

It's a snowball effect. Things can (and likely will, unless there is at least a *very* strict testing/quality policy
(which is https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options about))
get out of hand real quickly..

Just 2c.

Correct we should always be adding tests and show we don't break existing tests... We need to apply the "Beyonce Rule".. "if you liked it you should have put a test on it."

We shouldn't just give up making improvements or fixing defects because something is hard or complex.

The problem is that clang-format is already at a complexity level where most patches for new things we see are overly complex and will make adding new things *even harder*, reducing the ability to move at all.
As I've written before, I agree that we're in a bad state, and I'm truly sorry for it - we need to get active contributors into a state where they can make changes aligned with the architectural spirit of clang-format, which make clang-format at least not more complex :)

For this specific patch, yes, please change it as djasper suggested to be configurable in the BracketAlignmentStyle.

[gracicot]

Will this option also work with dangling braces from initializers?

I have some code that looks like this:

return some_type_with_a_long_name{
    get_param_number_one(),
    get_param_number_two()
};

Clang format will put the brace at the end of the line:

return some_type_with_a_long_name{
    get_param_number_one(),
    get_param_number_two()};

I would like to keep the style of the first snippet.

[MyDeveloperDay]

@stringham you seemingly have marked this revision so that no one else can edit it, so I cannot add it to the clang-format project or reassign reviewers, could you please change it to be public, it also means others cannot request changes or approve it (if they wanted to)

[stringham]

@MyDeveloperDay done

[MyDeveloperDay]

I'd like to assess where we are at with this revision, despite our concerns over additional complexity of clang-format, I don't think this is adding too much (I've seen far worse patches)

It appears to me that these changes are mainly in mustBreak,canBreak etc... however if you haven't added any unit tests I'm pretty sure things are going to go wrong when we run the other tests.

I'm not sure if you ever rebased with @mprobst changes or if those changes help you in any way

Given the level of likes of this feature, I think its worth at least looking at again, the quickness of your response yesterday suggests this might still be on your radar somewhere? it might be worth rebasing just to ensure it would go in cleanly, I could help by adding some unit tests if this is still of interest

[MyDeveloperDay]

I tried the patch and rebased it locally

rebased_patch.diff1 MBDownload

(as there were conflicts with the current trunk)

The change caused other tests to fail (which doesn't completely surprise me)

most tests failures look associated with positioning of trailing brace for example

@@ -1,4 +1,4 @@
 void f() {
   bar([]() {} // Did not respect SpacesBeforeTrailingComments
-  );
+ );
 }

Below is a list of the tests that fail

[==========] 700 tests from 21 test cases ran. (6687 ms total)
[  PASSED  ] 692 tests.
[  FAILED  ] 8 tests, listed below:
[  FAILED  ] FormatTest.WrapsTemplateDeclarations
[  FAILED  ] FormatTest.BreaksStringLiteralsWithin_TMacro
[  FAILED  ] FormatTest.FormatsLambdas
[  FAILED  ] FormatTest.WrappedClosingParenthesisIndent
[  FAILED  ] FormatTestJS.FunctionParametersTrailingComma
[  FAILED  ] FormatTestJS.FunctionLiterals
[  FAILED  ] FormatTestJS.ArrowFunctions
[  FAILED  ] FormatTestJS.NestedLiterals

 8 FAILED TESTS

[iolojz]

In D33029#1538200, @gracicot wrote:

Will this option also work with dangling braces from initializers?

I have some code that looks like this:

return some_type_with_a_long_name{
    get_param_number_one(),
    get_param_number_two()
};

Clang format will put the brace at the end of the line:

return some_type_with_a_long_name{
    get_param_number_one(),
    get_param_number_two()};

I would like to keep the style of the first snippet.

I would also love that (y)!

It should also be noted that the CLion formatter also supports this for all kinds of brackets (function declaration/call, template declaration/instantiation parameters, lambda capture lists, for statements, binary expressions as well as initializer lists).

[bbassi]

@MyDeveloperDay Is someone working on fixing the breaking tests and merging it? I need this feature so if someone isn't working on it already, I can take it.

[stringham]

@bbassi, I don't have plans to address this, and am supportive of you finishing it up.

The plan was to move this as one of the options in the BracketAlignmentStyle configuration (value of AlwaysBreakWithDanglingParenthesis) rather than have it be a brand new config property.

So we need to do that switch, and write new tests for this feature.

[MyDeveloperDay]

In D33029#1944919, @bbassi wrote:

@MyDeveloperDay Is someone working on fixing the breaking tests and merging it? I need this feature so if someone isn't working on it already, I can take it.

as @stringham isn't working on it feel free to take it. Include me as a reviewer and I'll try and give you a timely response.

[catskul]

@bbassi @MyDeveloperDay I'll buy whoever manages to land this feature a case of their favorite beer/beverage (max $50). Or $50 worth of girlscout cookies. : )

Just ping me when it lands and we can arrange the shipment.

[stringham]

Here's a commit rebased as of yesterday that is still adding DanglingParenthesis option instead of extending the BracketAlignmentStyle configuration.

https://github.com/lucidsoftware/llvm-project/commit/3c04de1feffaa9787234da8fe3cf288eebc979d6

[bbassi]

@stringham @MyDeveloperDay I have some questions.

• As some have pointed out DanglingParenthesis might be a confusing name, so should we try to call it something like BreakBeforeClosingBracket? When this option when is set to true we will always break before closing bracket.

• My use case is slightly different. I want each argument/parameter on it's own line and always have a break after opening bracket and before closing bracket. In other words, I don't want it to try to bin packing at all. What do you think about incorporating that use-case in current one? or if that should be a separate change.

[stringham]

I think that adding AlwaysBreakWithDanglingParenthesis as an option to BracketAlignmentStyle should not impact any of the binpacking settings - they should be distinct configuration.

[bbassi]

I don't think that's quite right. Then you will also have to have a AlignWithDanglingParenthesis for cases when people still want closing parenthesis on new line but want parameters as well as closing parenthesis to be aligned with opening parenthesis. I think we need a separate option, something like BreakBeforeClosingBracket.

[bbassi]

@MyDeveloperDay Can you please share your thoughts on my comment above?

[MarcusJohnson91]

In D33029#1946761, @bbassi wrote:

I don't think that's quite right. Then you will also have to have a AlignWithDanglingParenthesis for cases when people still want closing parenthesis on new line but want parameters as well as closing parenthesis to be aligned with opening parenthesis. I think we need a separate option, something like BreakBeforeClosingBracket.

I think that's the biggest problem with making changes to Clang-Format, every name does things that the name doesn't imply.

Here it's very similar to the bin packing options, in my case it's BraceWrapping.AfterExternBlock also indenting by default, making adding a IndentExternBlock a real pain.

I personally say fuck backwards compatibility when maintaining it requires ever more workarounds to fix an obviously flawed abstraction, but I'm just some dude and AFAIK the guys in charge support the status quo.

[MyDeveloperDay]

I personally say fuck backwards compatibility when maintaining it requires ever more workarounds to fix an obviously flawed abstraction, but I'm just some dude and AFAIK the guys in charge support the status quo.

When LLVM itself isn't prepared to run clang-format over the the whole project because of the churn and the annoyance it would cause for existing forks and "git blame" (despite their being work around to ignore whitespace change commits), Then I feel we have to keep one eye on ensuring we allow people to incrementally upgrade their clang-format.

Even including a new option to the .clang-format file is a problem because it forces all users of that project to upgrade to a version of clang-format that recognises the option or clang-format will complain. With this in mind it means we need to allow existing users to use new binaries which where possible has zero changes to the code UNLESS they use the option. (even if that means many people delaying in using an option until its been supported for some time)

With Visual Studio and ClangPower tools always lagging behind the current capabilities due to the binaries they ship, we need to understand it may be a year+ before the majority of users are on a version that supports such changes.

Then we can multiply that annoyance to every other project using clang-format (both public and private projects), and so whilst a commit that changes such things are good by their own merit, that goodness might be lost in the noise of the complaints that come from time to time that clang-format causes huge changes version to version (even when those changes are bug fixes, let alone new features)

This is not about supporting the status quo, but about ensuring we aren't fairly/unfairly the brunt of peoples complaints. (https://travisdowns.github.io/blog/2019/11/19/toupper.html, http://lists.llvm.org/pipermail/cfe-dev/2017-June/054342.html)

[MyDeveloperDay]

In D33029#1951346, @bbassi wrote:

@MyDeveloperDay Can you please share your thoughts on my comment above?

I'm tempted to agree that perhaps having as an enumeration BreakBeforeClosingBracket with various options might give us greater control

its recently also come to my attention that perhaps we should NEVER use a boolean as an option, as nearly every option that started out as a boolean ended up either needing to add another additional new option or being changed from a boolean to an enumeration later.

[bbassi]

@MyDeveloperDay hey, I am currently working on this, and adding a new option called BreakBeforeClosingBracket. I have some questions to understand the existing code, they might not be directly linked to this change so I am not sure if this the best place to ask those questions. What do you think? e.g. I don't understand what FakeLParens is and have some questions about that.

[MyDeveloperDay]

In D33029#1976981, @bbassi wrote:

@MyDeveloperDay hey, I am currently working on this, and adding a new option called BreakBeforeClosingBracket. I have some questions to understand the existing code, they might not be directly linked to this change so I am not sure if this the best place to ask those questions. What do you think? e.g. I don't understand what FakeLParens is and have some questions about that.

@bbassi, you might want to consider starting a new revision if you are going to add a different option. Its probably best to discuss questions about adding that option in its own revision rather than confusing the conversation here. (you can always cross reference with a mention.

[bbassi]

@MyDeveloperDay Thanks. This would be my first revision and I have few questions before I start coding. Would you be able to answer those over email? They are mainly about the design of clang-format and some existing options.

[MyDeveloperDay]

In D33029#2028254, @bbassi wrote:

@MyDeveloperDay Thanks. This would be my first revision and I have few questions before I start coding. Would you be able to answer those over email? They are mainly about the design of clang-format and some existing options.

ask away

[catskul]

@MyDeveloperDay I'm going to upload a re-based version of this. Should I rebase it off the top of master? Tip of 11? and/or create a new diff/review for each?

[catskul]

include/clang/Format/Format.h

793

@djasper I made the changes to @stringham's diff to implement your request to replace the bool with new item of BracketAlignmentStyle enum, and wondered if it might be backing us into a corner.

As strange as some of these may be I've seen a few similar to:

return_t myfunc(int x,
                int y,
                int z
                );

return_t myfunc(int x,
                int somelongy,
                int z         );

return_t myfunc(
             int x,
             int y,
             int z
         );

and even my least favorite

return_t myfunc(
    int x,
    int y,
    int z
                );

Without advocating for supporting all such styles it would seem desireable to avoid an NxM enum of two, at least theoretically, independent style parameters. With that in mind should I instead create a different parameter AlignClosingBracket with a respective enum which includes AfterFinalParameter by default, and NextLineWhenMultiline to handle the variations this diff was opened for?

On the other hand, I can just stick with adding a new variation to BracketAlignmentStyle and deal with potential variations in the future if they're requested.

[blandcr]

Hi, this is a pretty desirable feature for me and I see there hasn't been any work for a few months. Is this still being worked on and/or is there anything I can do to help it along?

[• FelixBenning]

If I understood it correctly there is a BraceWrapping group where BraceWrappingAfterControlStatementStyle AfterControlStatement is quite similar. It has a Never, MultiLine, and Always options. There is also a bool AfterFunction option which is currently only a bool. If it were changed to an enum it could provide this option as the MultiLine option. I do not really understand why there is a different option for control statements and functions though...

[jakar]

@catskul can you share your changes somehow? If you did, sorry, I'm new to Phabricator.

Before I saw @catskul's comment, I rebased @stringham's diff and got the existing unit tests to work. But after some experimentation, I found that it didn't always behave like I expected. It would sometimes do things like this:

someFunction(anotherLongFunctionName(arg1, arg2, "this is a description")
);

void SomeClass::someMemberFunction(const std::string& whatever, int blah
) const {
}

I think it should only break before closing when the matching opening was followed by a new line (with possible whitespace or comment).

And it also didn't handle brace-initialized vars consistently:

LongClassName longVarName1(
  somethingElseThatIsLong(arg1, arg2, arg3, arg4),
  anotherLongVarName
);

LongClassName longVarName2{
  somethingElseThatIsLong(arg1, arg2, arg3, arg4),
  anotherLongVarName};

So I think there is still a bit of work here to be done. And unit tests could definitely help.

I can take my best stab at this, when I get time, but I'm honestly not familiar with any of it. Any suggestions would be great. Please let me know how I can help.

Anyhow, I'm excited about this feature, mostly because I find it more readable, but also because it allows consistency with other languages, like Python (see PEP8):

my_list = [
    1, 2, 3,
    4, 5, 6,
]
result = some_function_that_takes_arguments(
    'a', 'b', 'c',
    'd', 'e', 'f',
)

[blandcr]

@jakar I did a bit of sleuthing (google catskul github, which I feel a little weird about but whatever) and I think I found their changes in a fork:
https://github.com/catskul/llvm-project/tree/catskul/dangling-parenthesis-as-bracket-alignment
It seems there are a few different branches that might be what you're looking for. I think what probably happened was they never received a response for what to rebase the changes off of so this end up falling by the wayside.

@catskul I'm not sure what the proper etiquette is for this kind of searching but given the easily associated identity I'm going to assume it's probably OK. If you do want me to edit the comment to remove the link to your GH profile just let me know.

[catskul]

@blandcr @jakar It's all good for me. Sorry for slow responses. My time to work on this is few and far between. I'm happy if anyone picks it up or any energy put into this. Would love to see it land.

[seesemichaelj]

include/clang/Format/Format.h

793

@catskul, are we waiting for a response from @djasper (or other maintainer) concerning your last question about whether or not to keep BracketAligngmentStyle or to use a new parameter AlignClosingBracket that you proposed? I'm just throwing my hand in the pile seeing what I can do to help push this issue towards landing without creating duplicate work (or just forking your code and compiling it for myself selfishly haha).

From what information I can gather, it sounds like you have a solution @catskul that meets the requested changes by @djasper, and if those changes are still desired provided your last comment here, a revision could be posted for review (after rebase, tests pass, etc).

Am I understanding correctly here?

[• Det87]

@catskul yeah, are we waiting for a response, or is this good to go?

Tagging everybody who might know something. @seesemichaelj @djasper @jakar @blandcr @MyDeveloperDay @bbassi

[catskul]

include/clang/Format/Format.h

793

@seesemichaelj (@Det87) Yes, though this diff belongs to @stringham I just jumped in to try to respond to @djasper's comments and keep the ball rolling.

I can post my changes that @blandcr and @jakar already found*, but would want to get the answer regarding AlignClosingBracket before putting any more energy into this as it's expensive to get spun back up and build all of this each time (I don't think I still have the build env around).

The second hurdle is that if I post a new diff I'm not sure if I'm adopting this whole thing and/or violating ettiquette, which I don't feel like I have the time & energy to do without some confidence that it will move forward.

Honestly I think the llvm project is leaving a lot of good work and good will on the table by how clumsy the contribution process is (partially because of how clumsy I feel like Phabricator is)

*https://github.com/catskul/llvm-project/commits/catskul/dangling-parenthesis-as-bracket-alignment

[MyDeveloperDay]

I understand the frustration (I'm not convinced that its Phabricators fault to be honest, that's our process and plenty of people follow it without issues) , Our incredible original code owners have moved on to do I assume other things (rightly so as their contribution was already massive) and we are trying to hold the fort. But waiting for them to comment might mean you are waiting a long time.

We are tracking this work via D109557: Adds a BlockIndent option to AlignAfterOpenBracket which was an attempt to resuscitate this request. Honestly I'm grateful to @csmulhern for taking it on.

As this current review D33029 is 3 years since the last diff its hideously out of date and won't merge cleanly, It also is completely lacking in unit tests and that was always a massive "no-no"

I recommend we put our efforts into getting D109557 over the line? given that they have addressed those issues (I have given it an "Accept")

I'm was not a massive fan of the term DanglingParenthesis as a technical directive, and think BreakBeforeClosingParen from D109557 is a better name.

I have some reservations about the ) moving too much in my view on an if etc.. but as long as we can get a consensus as to what you all (I say all as this has 87 subscribers!) expect it to do I'm ok (as I won't be using this personally, I don't feel my opinion matters that much other than from a maintenance perspective! but I need those of you that will to comment if you don't like what its trying to do)

A good set of unit tests really helps to flesh this out.

void foo() {
  if (quitelongarg != (alsolongarg - 1)
  ) { // ABC is a very longgggggggggggg comment
    return;
  }
}

Its definitely not about about LLVM leaving good will on the table, its about having people being persistent enough to push it over the line (which believe me I recognize is difficult because lots of us have other jobs and do this in free time)

Contributors need to remain at the table long enough to finish the meal. There is a support burden that has to be considered especially 6 months down the line when the feature gets branched out and exposed to the masses,

But to be honest its not fair on those of us trying to look after it to just rock up every 6 months and ask why we've not pushed your feature over the line for you. You contribution is appreciated, but I would rather people hung out here a little longer.

My 2c worth.

[seesemichaelj]

@MyDeveloperDay Great, sounds like this revision, D33029, is stale and being superseded by the work in D109557. It doesn't sound like there is anyone here that is wanting to keep pushing forward this specific implementation at this point. I feel like any debates on naming, and implementation can happen over at D109557 since that is recent and getting pushed forward.

I'm obviously not a maintainer, but I think it's totally fair at this point to just close this revision in favor of D109557, is this something you can do?

docs/ClangFormatStyleOptions.rst

.. code-block:: c++
 
     false:
     SomeClass::Constructor()
         : aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa),
           aaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaa) {
       return 0;
     }
 
+**DanglingParenthesis** (``bool``)
+  If there is a break after the opening parenthesis, also break before the closing parenthesis
+
+  .. code-block:: c++
+
+    true:
+    someLongFunction(
+          argument1, argument2
+    );
+
+    false:
+    someLongFunction(
+          argument1, argument2);
+
 **ConstructorInitializerIndentWidth** (``unsigned``)
   The number of characters to use for indentation of constructor
   initializer lists.
 
 **ContinuationIndentWidth** (``unsigned``)
   Indent width for line continuations.
 
   .. code-block:: c++

include/clang/Format/Format.h

struct FormatStyle {
   ///   SomeClass::Constructor()
   ///       : aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa),
   ///         aaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaa) {
   ///     return 0;
   ///   }
   /// \endcode
   bool ConstructorInitializerAllOnOneLineOrOnePerLine;
 
+  /// \brief If there is a break after the opening parenthesis, also break
+  /// before the closing parenthesis
+  /// \code
+  ///   true:
+  ///   someLongFunction(
+  ///       argument1, argument2
+  ///   );
+  ///
+  ///   false:
+  ///   someLongFunction(
+  ///       argument1, argument2);
+  /// \endcode
+  bool DanglingParenthesis;
+
   /// \brief The number of characters to use for indentation of constructor
   /// initializer lists.
   unsigned ConstructorInitializerIndentWidth;
 
   /// \brief Indent width for line continuations.
   /// \code
   ///    ContinuationIndentWidth: 2
   ///

lib/Format/ContinuationIndenter.cpp

bool ContinuationIndenter::canBreak(const LineState &State) {
   }
 
   // If binary operators are moved to the next line (including commas for some
   // styles of constructor initializers), that's always ok.
   if (!Current.isOneOf(TT_BinaryOperator, tok::comma) &&
       State.Stack.back().NoLineBreakInOperand)
     return false;
 
+  if (Style.DanglingParenthesis && Current.is(tok::r_paren) &&
+      Current.MatchingParen &&
+      Current.MatchingParen->LastNewlineOffset ==
+          Current.MatchingParen->Next->LastNewlineOffset)
+    return false;
+
   return !State.Stack.back().NoLineBreak;
 }
 
 bool ContinuationIndenter::mustBreak(const LineState &State) {
   const FormatToken &Current = *State.NextToken;
   const FormatToken &Previous = *Current.Previous;
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
     return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
       Current.closesBlockOrBlockTypeList(Style))
     return true;
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
     return true;
+  if (Style.DanglingParenthesis && Current.is(tok::r_paren) &&
+      Current.MatchingParen &&
+      Current.MatchingParen->LastNewlineOffset <
+          Current.MatchingParen->Next->LastNewlineOffset)
+    return true;
   if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
        (Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
         Style.isCpp() &&
         // FIXME: This is a temporary workaround for the case where clang-format
         // sets BreakBeforeParameter to avoid bin packing and this creates a
         // completely unnecessary line break after a template type that isn't
         // line-wrapped.
         (Previous.NestingLevel == 1 || Style.BinPackParameters)) ||
unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
   if (Current.isOneOf(tok::r_brace, tok::r_square) && State.Stack.size() > 1) {
     if (Current.closesBlockOrBlockTypeList(Style))
       return State.Stack[State.Stack.size() - 2].NestedBlockIndent;
     if (Current.MatchingParen &&
         Current.MatchingParen->BlockKind == BK_BracedInit)
       return State.Stack[State.Stack.size() - 2].LastSpace;
     return State.FirstIndent;
   }
+  if (Style.DanglingParenthesis && Current.is(tok::r_paren) && State.Stack.size() > 1) {
+    return State.Stack[State.Stack.size() - 2].LastSpace;
+  }
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
     return State.Stack[State.Stack.size() - 2].LastSpace;
   if (Current.is(tok::identifier) && Current.Next &&
       Current.Next->is(TT_DictLiteral))
     return State.Stack.back().Indent;
   if (NextNonComment->is(TT_ObjCStringLiteral) &&
       State.StartOfStringLiteral != 0)
     return State.StartOfStringLiteral - 1;

lib/Format/Format.cpp

IO.mapOptional("BreakAfterJavaFieldAnnotations",
                    Style.BreakAfterJavaFieldAnnotations);
     IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
     IO.mapOptional("ColumnLimit", Style.ColumnLimit);
     IO.mapOptional("CommentPragmas", Style.CommentPragmas);
     IO.mapOptional("BreakBeforeInheritanceComma",
                    Style.BreakBeforeInheritanceComma);
     IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
                    Style.ConstructorInitializerAllOnOneLineOrOnePerLine);
+    IO.mapOptional("DanglingParenthesis", Style.DanglingParenthesis);
     IO.mapOptional("ConstructorInitializerIndentWidth",
                    Style.ConstructorInitializerIndentWidth);
     IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
     IO.mapOptional("Cpp11BracedListStyle", Style.Cpp11BracedListStyle);
     IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment);
     IO.mapOptional("DisableFormat", Style.DisableFormat);
     IO.mapOptional("ExperimentalAutoDetectBinPacking",
                    Style.ExperimentalAutoDetectBinPacking);
LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
                              false, false, false, false, false};
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializersBeforeComma = false;
   LLVMStyle.BreakBeforeInheritanceComma = false;
   LLVMStyle.BreakStringLiterals = true;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
+  LLVMStyle.DanglingParenthesis = false;
   LLVMStyle.ConstructorInitializerIndentWidth = 4;
   LLVMStyle.ContinuationIndentWidth = 4;
   LLVMStyle.Cpp11BracedListStyle = true;
   LLVMStyle.DerivePointerAlignment = false;
   LLVMStyle.ExperimentalAutoDetectBinPacking = false;
   LLVMStyle.FixNamespaceComments = true;
   LLVMStyle.ForEachMacros.push_back("foreach");
   LLVMStyle.ForEachMacros.push_back("Q_FOREACH");
FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
 
   GoogleStyle.AccessModifierOffset = -1;
   GoogleStyle.AlignEscapedNewlines = FormatStyle::ENAS_Left;
   GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
   GoogleStyle.AllowShortLoopsOnASingleLine = true;
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
   GoogleStyle.AlwaysBreakTemplateDeclarations = true;
   GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  GoogleStyle.DanglingParenthesis = false;
   GoogleStyle.DerivePointerAlignment = true;
   GoogleStyle.IncludeCategories = {{"^<.*\\.h>", 1}, {"^<.*", 2}, {".*", 3}};
   GoogleStyle.IncludeIsMainRegex = "([-_](test|unittest))?$";
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false;
   GoogleStyle.ObjCSpaceAfterProperty = false;
   GoogleStyle.ObjCSpaceBeforeProtocolList = false;
   GoogleStyle.PointerAlignment = FormatStyle::PAS_Left;

lib/Format/TokenAnnotator.cpp

bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
   if (Left.is(tok::l_paren) && Left.is(TT_AttributeParen))
     return false;
   if (Left.is(tok::l_paren) && Left.Previous &&
       (Left.Previous->isOneOf(TT_BinaryOperator, TT_CastRParen)))
     return false;
   if (Right.is(TT_ImplicitStringLiteral))
     return false;
 
-  if (Right.is(tok::r_paren) || Right.is(TT_TemplateCloser))
+  if (Right.is(tok::r_paren)) {
+    return Style.DanglingParenthesis;
+  }
+  if (Right.is(TT_TemplateCloser))
     return false;
   if (Right.is(tok::r_square) && Right.MatchingParen &&
       Right.MatchingParen->is(TT_LambdaLSquare))
     return false;
 
   // We only break before r_brace if there was a corresponding break before
   // the l_brace, which is tracked by BreakBeforeClosingBrace.
   if (Right.is(tok::r_brace))

unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakConstructorInitializersBeforeComma);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(BreakBeforeInheritanceComma)
   CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
+  CHECK_PARSE_BOOL(DanglingParenthesis);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);