clang-format: inconsistency in function arg layout (C++)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

clang-format: inconsistency in function arg layout (C++)

Yvan Roux via cfe-dev
Hi, I've just boiled down an interesting C++ lambda formatting trait and
would like to clarify the tool's behavior. Consider the following
snippet (please view with a fixed-width font):

void f() {
   something->One(
       [this] {
         Do1();
         Do2();
       },
       1);
   something->Two(1,
                  [this] {
                    Do1();
                    Do2();
                  },
                  1);
}

There is an inconsistency in the way lambda args are formatted,
depending on whether it is first (the "One()" call above) or not (the
"Two()" call above). Is there some internal guide that the tool uses to
decide between the two layouts? Or is it just an artifact of the
implementation?

More generally, would you entertain a patch that forces the format one
way or another? (Perhaps even with a user-defined setting?)

Thanks,
Oleg.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format: inconsistency in function arg layout (C++)

Yvan Roux via cfe-dev
OK, the functionality is a special case introduced in 2014 here:
8228889b01404d7e59270b1f97a83977531a7748.

The minimal hack is to check for the preceding comma... but that breaks
some "literal" cases... So, I need to make these selections even more
particular (to either exclude literals or only include lambdas). Does
this sound right?

On 2018-09-10 12:47, Oleg Smolsky wrote:

> Hi, I've just boiled down an interesting C++ lambda formatting trait
> and would like to clarify the tool's behavior. Consider the following
> snippet (please view with a fixed-width font):
>
> void f() {
>   something->One(
>       [this] {
>         Do1();
>         Do2();
>       },
>       1);
>   something->Two(1,
>                  [this] {
>                    Do1();
>                    Do2();
>                  },
>                  1);
> }
>
> There is an inconsistency in the way lambda args are formatted,
> depending on whether it is first (the "One()" call above) or not (the
> "Two()" call above). Is there some internal guide that the tool uses
> to decide between the two layouts? Or is it just an artifact of the
> implementation?
>
> More generally, would you entertain a patch that forces the format one
> way or another? (Perhaps even with a user-defined setting?)
>
> Thanks,
> Oleg.
>

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

0001-lib-Format-tweaked-another-case-of-lambda-formatting.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: clang-format: inconsistency in function arg layout (C++)

Yvan Roux via cfe-dev

Hi Oleg,

could you please send that patch to `reviews.llvm.org`. That makes it easier to review and discuss on the code.

Best, Jonas


Am 10.09.2018 um 22:47 schrieb Oleg Smolsky via cfe-dev:
OK, the functionality is a special case introduced in 2014 here: 8228889b01404d7e59270b1f97a83977531a7748.

The minimal hack is to check for the preceding comma... but that breaks some "literal" cases... So, I need to make these selections even more particular (to either exclude literals or only include lambdas). Does this sound right?

On 2018-09-10 12:47, Oleg Smolsky wrote:
Hi, I've just boiled down an interesting C++ lambda formatting trait and would like to clarify the tool's behavior. Consider the following snippet (please view with a fixed-width font):

void f() {
  something->One(
      [this] {
        Do1();
        Do2();
      },
      1);
  something->Two(1,
                 [this] {
                   Do1();
                   Do2();
                 },
                 1);
}

There is an inconsistency in the way lambda args are formatted, depending on whether it is first (the "One()" call above) or not (the "Two()" call above). Is there some internal guide that the tool uses to decide between the two layouts? Or is it just an artifact of the implementation?

More generally, would you entertain a patch that forces the format one way or another? (Perhaps even with a user-defined setting?)

Thanks,
Oleg.



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format: inconsistency in function arg layout (C++)

Yvan Roux via cfe-dev
The patch was a baby step in right direction, but it does not function properly. Let me try to make it work, I'll post it then.

On Tue, Sep 11, 2018 at 3:12 AM, Jonas Toth via cfe-dev <[hidden email]> wrote:

Hi Oleg,

could you please send that patch to `reviews.llvm.org`. That makes it easier to review and discuss on the code.

Best, Jonas


Am 10.09.2018 um 22:47 schrieb Oleg Smolsky via cfe-dev:
OK, the functionality is a special case introduced in 2014 here: 8228889b01404d7e59270b1f97a83977531a7748.

The minimal hack is to check for the preceding comma... but that breaks some "literal" cases... So, I need to make these selections even more particular (to either exclude literals or only include lambdas). Does this sound right?

On 2018-09-10 12:47, Oleg Smolsky wrote:
Hi, I've just boiled down an interesting C++ lambda formatting trait and would like to clarify the tool's behavior. Consider the following snippet (please view with a fixed-width font):

void f() {
  something->One(
      [this] {
        Do1();
        Do2();
      },
      1);
  something->Two(1,
                 [this] {
                   Do1();
                   Do2();
                 },
                 1);
}

There is an inconsistency in the way lambda args are formatted, depending on whether it is first (the "One()" call above) or not (the "Two()" call above). Is there some internal guide that the tool uses to decide between the two layouts? Or is it just an artifact of the implementation?

More generally, would you entertain a patch that forces the format one way or another? (Perhaps even with a user-defined setting?)

Thanks,
Oleg.



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev




--
Oleg Smolsky
Member of Technical Staff

twitter-3-16.png  linkedin-6-16.png



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clang-format: inconsistency in function arg layout (C++)

Yvan Roux via cfe-dev

Hey Jonas, here is a patch that adds the functionality I described (under BinPackArguments=false).

https://reviews.llvm.org/D52676

Could you take a look please?

Thanks!
Oleg.

On 2018-09-11 08:10, Oleg Smolsky wrote:
The patch was a baby step in right direction, but it does not function properly. Let me try to make it work, I'll post it then.

On Tue, Sep 11, 2018 at 3:12 AM, Jonas Toth via cfe-dev <[hidden email]> wrote:

Hi Oleg,

could you please send that patch to `reviews.llvm.org`. That makes it easier to review and discuss on the code.

Best, Jonas


Am 10.09.2018 um 22:47 schrieb Oleg Smolsky via cfe-dev:
OK, the functionality is a special case introduced in 2014 here: 8228889b01404d7e59270b1f97a83977531a7748.

The minimal hack is to check for the preceding comma... but that breaks some "literal" cases... So, I need to make these selections even more particular (to either exclude literals or only include lambdas). Does this sound right?

On 2018-09-10 12:47, Oleg Smolsky wrote:
Hi, I've just boiled down an interesting C++ lambda formatting trait and would like to clarify the tool's behavior. Consider the following snippet (please view with a fixed-width font):

void f() {
  something->One(
      [this] {
        Do1();
        Do2();
      },
      1);
  something->Two(1,
                 [this] {
                   Do1();
                   Do2();
                 },
                 1);
}

There is an inconsistency in the way lambda args are formatted, depending on whether it is first (the "One()" call above) or not (the "Two()" call above). Is there some internal guide that the tool uses to decide between the two layouts? Or is it just an artifact of the implementation?

More generally, would you entertain a patch that forces the format one way or another? (Perhaps even with a user-defined setting?)

Thanks,
Oleg.



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev




--
Oleg Smolsky
Member of Technical Staff

twitter-3-16.png  linkedin-6-16.png



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev