llvm / llvm-project

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

AlwaysBreakLambda #31499

Open llvmbot opened 7 years ago

llvmbot commented 7 years ago
Bugzilla Link 32151
Version 4.0
OS All
Reporter LLVM Bugzilla Contributor
CC @egorpugin,@JVApen,@SuperV1234

Extended Description

This is more of an enhancement than a bug, but it would be great if there was an option like: AlwaysBreakLambda that did the following:

Before:

TEST_CASE("cout")
{
    CHECK_COUT("hello\n", [] { std::cout << "hello\n"; });
}

After:

TEST_CASE("cout")
{
    CHECK_COUT("hello\n", [] { 
        std::cout << "hello\n"; 
    });
}

Currently, clang format is always outputting the "before" case if it can fit on a single line, and it just looks bad IMO. The "after" case is really what I would like to see since I am defining a function.

It also seems to do the following which is even worse IMO:

Odd Ball Case:

TEST_CASE("cout")
{
    CHECK_COUT(
        "slightly longer\n", [] { std::cout << "slightly longer\n"; });
}
llvmbot commented 6 years ago

I make a patch proposal to fix a similar problem with lambda: https://reviews.llvm.org/D44609

It's now exactly the some option you propose, but it could perhaps help. I propose a new option named "BreakBeforeLambdaBody". I wait that someone make a review on this patch.

llvmbot commented 7 years ago

Right now we use 100% automated coding style via Astyle meaning, you are not allowed to submit a patch to our repo if astyle finds a problem (we actually apply this rule for static analysis, dynamic analysis, git check, doxygen, etc....).

The astyle config is here: https://github.com/Bareflank/hypervisor/blob/master/tools/astyle/astyle.config

We are currently in the process of switching over to LLVM for most things, including clang-format. The requirement for project size states dozens, not hundreds or thousands, and has no requirement for open / closed (so the 14 you see is only our open contributors). But I do appreciate that you are willing to consider our request regardless.

As for AllowShortFunctionsOnASingleLine and AllowShortBlocksOnASingleLine, I agree but would add that another option should added to BraceWrapping. Maybe called AfterLambda. Specifically, if AllowShortBlocksOnASingleLine and AllowShortFunctionsOnASingleLine are set to none, you need to know how to treat the break. For example:

AfterLambda: 'true' void foo([] { });

AfterLambda: 'false' void foo([] { });

llvmbot commented 7 years ago

Well, 14 contributors barely counts for sufficient size. Could you provide me with a link to the coding style?

Generally, I can imagine that this is desired by other coding styles as well, so that might be fine. Alternatively, I am wondering whether this should be tied to either AllowShortFunctionsOnASingleLine or AllowShortBlocksOnASingleLine. It seems to be somewhere in between those two. What is your preference for those coding styles?

Generally, you can contribute to LLVM/Clang by posting a patch to reviews.llvm.org.

llvmbot commented 7 years ago

That's fine. This is for: www.bareflank.org

We meet the first two requirements. As for the third, I have no problem writing the patch and maintaining it. Is there a doc on how you prefer patches being submitted, RFC process etc?

llvmbot commented 7 years ago

There are certain requirements we have for adding a style option: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options