llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.41k stars 12.15k forks source link

Extend options for parameter/argument wrapping #23796

Open llvmbot opened 9 years ago

llvmbot commented 9 years ago
Bugzilla Link 23422
Version 3.6
OS Linux
Attachments clang-format file used in the example
Reporter LLVM Bugzilla Contributor
CC @catskul

Extended Description

clang-format has 2 options called BinPackParameters and BinPackArguments. They seem to control how function declarations and function calls are indented.

BinPackParameters seems to provide the expected result for a function declaration but BinPackArguments does not seem to work as one would expect for a function call.

Here is a simple test file:


include

void function_with_a_huge_name_that_should_just_not_be(unsigned int a, char *b, unsigned int c, unsigned int d, unsigned int e) { return; }

int main() { function_with_a_huge_name_that_should_just_not_be(13, "bb", 1234234, 4324324, 2355345);

When formatted with clang-format the result is:


include

void function_with_a_huge_name_that_should_just_not_be(unsigned int a, char *b, unsigned int c, unsigned int d, unsigned int e) { return; }

int main() { function_with_a_huge_name_that_should_just_not_be( 13, "bb", 1234234, 4324324, 2355345); }

My clang-format version is: 3.6.0 (tags/RELEASE_360/final)

With both BinPackParameters and BinPackArguments being false I would have expected to get the same indentation for the function call as I am getting for the function declaration.

Is this a bug or is this expected behaviour?

And if it's expected behaviour how can I get formatting of both function declarations and function calls to work in a consistent manner?

Attached you can find the .clang-format file

llvmbot commented 7 years ago

The AlignAfterOpenBracket: AlwaysBreak setting now controls the placement of the first param on the line after the function call. But BinPackArguments is still behaving incorrectly (or at least unexpectedly), per the original bug report.

llvmbot commented 9 years ago

Thanks for taking a look at this Daniel!

The enum values you are mentioning are not all related to parameter packing.

I agree that whether or not to break after the open parenthesis is related to parameter packing, whether or not to align the parameters and whether or not to break before the ")" (btw., what style is doing that??) are different things that I don't want to merge into these options.

I'm curious how breaking after the last argument is unrelated to parameter packing, but breaking before the first argument is. I'm sure there's a good reason for it; It just isn't immediately obvious to me. Note that I'm not advocating for adding this option.

Regarding how to encode a new "BinPackIfLessThanNumLines" option, it seems like we've got the frequent "we've got an enum, but we need a variant" problem. Perhaps we could have 'BinPackParameters' as an enum as you suggested and if it set to "BinPackIfLessThanNumLines", then the system will also look at the value with name "BinPackParameters_BinPackIfLessThanNLines_NumLines".

Using this as a pattern, we could enable option-specific parameters in general.

llvmbot commented 9 years ago

I have seen style guides that do all these options. The enum I gave above is just an example. The point would be for the user to be able to check for and enforce any of the above styles. If it would take other parameters outside of the enum to do it then no problem. As long as it is possible.

llvmbot commented 9 years ago

The enum values you are mentioning are not all related to parameter packing.

I agree that whether or not to break after the open parenthesis is related to parameter packing, whether or not to align the parameters and whether or not to break before the ")" (btw., what style is doing that??) are different things that I don't want to merge into these options.

Also, I think it is impossible to describe any of the packing options based on a single example. They essentially describe what happens depending on how the different parameters fit on a line.

llvmbot commented 9 years ago

I like the idea of keeping the BinPackParameters and BinPackArguments options only and providing an enum value for them. It's important to be able to get consistent styling behaviour for function declarations, function implementations and function calls.

I would suggest it being a bitflag of options which can be combined so that different projects can achieve their desires styling effect easily. The reasoning behind this is that it would be quite nice to provide control of all the minor little things in splitting arguments/parameters of a function. This is just an idea:

enum pack_options { PACK_NONE = 0, // nothing to be done /**

As for the binpacking with a line argument limit that would need to be 2 extra options which would activate the binpacking behaviour (or not) when the arguments span less (or more) than a given number of lines.

llvmbot commented 9 years ago

There have been several requests around this. Re-using this bug to group them all.

My thoughts: I think it is valuable to extend the options controlling the wrapping of parameters and arguments.

The first thing we need to figure out is how to do the configuration itself. We should come up with a good long-term solution. Specifically, there are currently three boolean configuration flags: BinPackParameters, BinPackArguments and AllowAllParametersOfDeclarationOnNextLine. I think we should get rid of the last one and instead allow enum values for BinPackParameters and BinPackArguments. However, there has been one more request which is to allow bin-packing if arguments don't span more than N lines (with N e.g. being 2 or 3). Not quite sure how to fit that into the scheme yet. Any input is appreciated.

llvmbot commented 9 years ago

I see. Thank you for the reply. The documentation for BinPackArguments states:

BinPackArguments (bool) If false, a function call’s arguments will either be all on the same line or will have one line each.

So there is no way to control which of the two cases happens at the moment?

Tf that's so then where/how can I make a feature request for this? Many projects, including the ones I work on, would benefit from being able to have the option to break long function calls and function declarations with each argument on its own line.

I really like clang formatter and could even try to work on such a thing on my free time, assuming I can get familiar with the code fast and that the implementation is not too complicated.

llvmbot commented 9 years ago

clang-format generally does not consider:

SomeFunction( a, b, c, d);

to be bin-packing. There is no option to enforce the one-per-line wrapping in this case.

So, to answer your questions:

"Is this a bug or expected behavior?": Expected behavior.

"How can I get .. consistent manner?": Set AllowAllParametersOfDeclarationOnNextLine to true.

llvmbot commented 9 years ago

If I setAllowAllParametersOfDeclarationOnNextLine to true then the result I will get is all the parameters in one line for both.

The desired result I would expect is to have each parameter in its own line for both.

In short I would expect the function call to be formatted just like the function declaration.

llvmbot commented 9 years ago

Try setting AllowAllParametersOfDeclarationOnNextLine to true.