lambda coding style

lambda coding style

[Jason Merrill]

What formatting style do we want for non-trivial lambdas in GCC sources? 
  I'm thinking the most consistent choice would be

auto l = [&] (parms) // space between ] (
   {                  // brace on new line, indented two spaces
     return stuff;
   };

By default, recent emacs lines up the { with the previous line, like an 
in-class function definition; I talked it into the above indentation with

(defun lambda-offset (elem)
   (if (assq 'inline-open c-syntactic-context) '+ 0))
(add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))

I think we probably want the same formatting for lambdas in function 
argument lists, e.g.

algorithm ([] (parms)
   {
     return foo;
   });

Any other opinions?

Jason

I am causing valgrind errors...

[Robert Dubner]

This message is a bit of a Hail Mary play.  I am not asking anybody to spend 
any real effort on this question; I am, instead, hoping that somebody will 
say, "Oh, sure; you can fix that by doing so-and-so..." or "That happens 
when the GENERIC tree is ..."

Jim Lowden and I are working on a COBOL front end for GCC.  We are, at this 
point, pretty far along.  He's been focusing on the parsing; I've been doing 
the code generation.

Our front end is based on GCC-13; I just merged in "40e16fda0f4 - 
(gnu-gcc/releases/gcc-13) Daily bump." a few hours ago.

The minimalist background is this:  I start the process of building a 
function by calling
build_varargs_function_type_array().

I feed the returned type_decl to build_fn_decl(), which returns a 
function_decl.

That function_decl becomes the root of a tree that gets everything else 
tacked on.

When I am done creating the function, I pass that function_decl to

    cgraph_node::finalize_function (function_decl, true);

This whole process works; we've been producing executables this way for a 
couple of years.  But I freely admit that I don't know if I am performing 
all necessary magical incantations properly.

The COBOL program I am compiling here does absolutely nothing; it just 
returns.  But our COBOL executables have significant overhead; it’s the 
nature of COBOL.  There are a bunch of variables that get created, whether 
they are used or not, and there is boilerplate code on both entry and exit 
to COBOL functions.

Here's the thing:  when I run valgrind on the compilation -- not on the 
executable, but on the compiler with the COBOL front end -- I am getting a 
bunch of errors that look like variations of this:

==1232157== Command: /home/bob/repos/gcc-cobol/build/gcc/cobol1 
playpen.cbl -quiet -dumpbase 
playpen.cbl -mtune=generic -march=x86-64 -ggdb -O0 -o playpen.s -cmain
==1232157==
==1232157== Conditional jump or move depends on uninitialised value(s)
==1232157==    at 0xABA0CB: sparseset_bit_p (sparseset.h:146)
==1232157==    by 0xABA0CB: mark_pseudo_regno_live(int) (ira-lives.cc:326)
==1232157==    by 0xABBDC0: process_bb_node_lives(ira_loop_tree_node*) 
(ira-lives.cc:1434)
==1232157==    by 0xA9E8D5: ira_traverse_loop_tree(bool, 
ira_loop_tree_node*, void (*)(ira_loop_tree_node*), void 
(*)(ira_loop_tree_node*)) (ira-build.cc:1807)
==1232157==    by 0xABC6F3: ira_create_allocno_live_ranges() 
(ira-lives.cc:1734)
==1232157==    by 0xAA038C: ira_build() (ira-build.cc:3483)
==1232157==    by 0xA971BA: ira (ira.cc:5783)
==1232157==    by 0xA971BA: (anonymous 
namespace)::pass_ira::execute(function*) (ira.cc:6106)
==1232157==    by 0xB95A71: execute_one_pass(opt_pass*) (passes.cc:2651)
==1232157==    by 0xB9632F: execute_pass_list_1(opt_pass*) (passes.cc:2760)
==1232157==    by 0xB96341: execute_pass_list_1(opt_pass*) (passes.cc:2761)
==1232157==    by 0xB9636C: execute_pass_list(function*, opt_pass*) 
(passes.cc:2771)
==1232157==    by 0x846BA7: expand (cgraphunit.cc:1841)
==1232157==    by 0x846BA7: cgraph_node::expand() (cgraphunit.cc:1794)
==1232157==    by 0x847E80: output_in_order (cgraphunit.cc:2191)
==1232157==    by 0x847E80: symbol_table::compile() [clone .part.0] 
(cgraphunit.cc:2395)

Please note that the compiler is generating a good executable from the 
function_decl I am passing to cgraph_ node::finalize_function().  But I 
don't like leaving unexplained errors behind me.  They tend to sneak up 
behind, and they often have teeth.

So, here's my question to the giant brains with eidetic memories who 
populate this list:  Can anybody give me a hint as to what I might be doing 
wrong, either with the tree of GENERIC tags or with the way I am asking the 
GCC back end to compile it, to give valgrind that particular bit of 
indigestion?

Thank you so much for even reading this, and thanks for any hint you might 
have.

Bob Dubner

Re: I am causing valgrind errors...

[Jakub Jelinek]

On Wed, Jan 10, 2024 at 02:47:08PM -0600, Robert Dubner wrote:
> Here's the thing:  when I run valgrind on the compilation -- not on the 
> executable, but on the compiler with the COBOL front end -- I am getting a 
> bunch of errors that look like variations of this:
> 
> ==1232157== Command: /home/bob/repos/gcc-cobol/build/gcc/cobol1 
> playpen.cbl -quiet -dumpbase 
> playpen.cbl -mtune=generic -march=x86-64 -ggdb -O0 -o playpen.s -cmain
> ==1232157==
> ==1232157== Conditional jump or move depends on uninitialised value(s)
> ==1232157==    at 0xABA0CB: sparseset_bit_p (sparseset.h:146)
> ==1232157==    by 0xABA0CB: mark_pseudo_regno_live(int) (ira-lives.cc:326)
> ==1232157==    by 0xABBDC0: process_bb_node_lives(ira_loop_tree_node*) 

That is normal and not a bug.
If you want to avoid that, you need to configure the compiler with
--enable-valgrind-annotations
(and have valgrind development installed before you configure/build).

	Jakub

Re: lambda coding style

[Marek Polacek]

On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
> What formatting style do we want for non-trivial lambdas in GCC sources?
> I'm thinking the most consistent choice would be
> 
> auto l = [&] (parms) // space between ] (
>   {                  // brace on new line, indented two spaces
>     return stuff;
>   };

Sure, why not.  Consistency is what matters.  Thus far we seem
to have been very inconsistent.  ;)
 
> By default, recent emacs lines up the { with the previous line, like an
> in-class function definition; I talked it into the above indentation with
> 
> (defun lambda-offset (elem)
>   (if (assq 'inline-open c-syntactic-context) '+ 0))
> (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
> 
> I think we probably want the same formatting for lambdas in function
> argument lists, e.g.
> 
> algorithm ([] (parms)
>   {
>     return foo;
>   });

And what about lambdas in conditions:

if (foo ()
    && [&] (params) mutable
       {
	 return 42;
       } ())

should the { go just below [?

Marek

RE: I am causing valgrind errors...

[Robert Dubner]

In the words of Nick Danger: "You just saved me a lot of investigative
work."

Thank you so much.

Bob D.

-----Original Message-----
From: Jakub Jelinek <jakub@redhat.com> 
Sent: Wednesday, January 10, 2024 15:51
To: Robert Dubner <rdubner@symas.com>
Cc: 'GCC Mailing List' <gcc@gcc.gnu.org>
Subject: Re: I am causing valgrind errors...

On Wed, Jan 10, 2024 at 02:47:08PM -0600, Robert Dubner wrote:
> Here's the thing:  when I run valgrind on the compilation -- not on 
> the executable, but on the compiler with the COBOL front end -- I am 
> getting a bunch of errors that look like variations of this:
> 
> ==1232157== Command: /home/bob/repos/gcc-cobol/build/gcc/cobol1
> playpen.cbl -quiet -dumpbase
> playpen.cbl -mtune=generic -march=x86-64 -ggdb -O0 -o playpen.s -cmain 
> ==1232157== ==1232157== Conditional jump or move depends on 
> uninitialised value(s)
> ==1232157==    at 0xABA0CB: sparseset_bit_p (sparseset.h:146)
> ==1232157==    by 0xABA0CB: mark_pseudo_regno_live(int)
(ira-lives.cc:326)
> ==1232157==    by 0xABBDC0: process_bb_node_lives(ira_loop_tree_node*) 

That is normal and not a bug.
If you want to avoid that, you need to configure the compiler with
--enable-valgrind-annotations (and have valgrind development installed
before you configure/build).

	Jakub

Re: lambda coding style

[Jason Merrill]

On 1/10/24 15:59, Marek Polacek wrote:
> On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
>> What formatting style do we want for non-trivial lambdas in GCC sources?
>> I'm thinking the most consistent choice would be
>>
>> auto l = [&] (parms) // space between ] (
>>    {                  // brace on new line, indented two spaces
>>      return stuff;
>>    };
> 
> Sure, why not.  Consistency is what matters.  Thus far we seem
> to have been very inconsistent.  ;)
>   
>> By default, recent emacs lines up the { with the previous line, like an
>> in-class function definition; I talked it into the above indentation with
>>
>> (defun lambda-offset (elem)
>>    (if (assq 'inline-open c-syntactic-context) '+ 0))
>> (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
>>
>> I think we probably want the same formatting for lambdas in function
>> argument lists, e.g.
>>
>> algorithm ([] (parms)
>>    {
>>      return foo;
>>    });
> 
> And what about lambdas in conditions:
> 
> if (foo ()
>      && [&] (params) mutable
>         {
> 	 return 42;
>         } ())
> 
> should the { go just below [?

I think we don't want the { to go below the [ in general; that was the 
old emacs default behavior, and it produced lambda bodies with excessive 
indentation.

With my adjustment above, emacs indents the { two spaces from the &&, 
which seems a bit arbitrary but reasonable.

Jason

Re: lambda coding style

[Marek Polacek]

On Wed, Jan 10, 2024 at 04:24:42PM -0500, Jason Merrill wrote:
> On 1/10/24 15:59, Marek Polacek wrote:
> > On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
> > > What formatting style do we want for non-trivial lambdas in GCC sources?
> > > I'm thinking the most consistent choice would be
> > > 
> > > auto l = [&] (parms) // space between ] (
> > >    {                  // brace on new line, indented two spaces
> > >      return stuff;
> > >    };
> > 
> > Sure, why not.  Consistency is what matters.  Thus far we seem
> > to have been very inconsistent.  ;)
> > > By default, recent emacs lines up the { with the previous line, like an
> > > in-class function definition; I talked it into the above indentation with
> > > 
> > > (defun lambda-offset (elem)
> > >    (if (assq 'inline-open c-syntactic-context) '+ 0))
> > > (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
> > > 
> > > I think we probably want the same formatting for lambdas in function
> > > argument lists, e.g.
> > > 
> > > algorithm ([] (parms)
> > >    {
> > >      return foo;
> > >    });
> > 
> > And what about lambdas in conditions:
> > 
> > if (foo ()
> >      && [&] (params) mutable
> >         {
> > 	 return 42;
> >         } ())
> > 
> > should the { go just below [?
> 
> I think we don't want the { to go below the [ in general; that was the old
> emacs default behavior, and it produced lambda bodies with excessive
> indentation.
> 
> With my adjustment above, emacs indents the { two spaces from the &&, which
> seems a bit arbitrary but reasonable.

Fair enough, I think that's better.

I suppose we should add a note wrt lambdas to
https://gcc.gnu.org/codingconventions.html#Cxx_Conventions

Marek

Re: lambda coding style

[Jason Merrill]

On 1/10/24 16:41, Marek Polacek wrote:
> On Wed, Jan 10, 2024 at 04:24:42PM -0500, Jason Merrill wrote:
>> On 1/10/24 15:59, Marek Polacek wrote:
>>> On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
>>>> What formatting style do we want for non-trivial lambdas in GCC sources?
>>>> I'm thinking the most consistent choice would be
>>>>
>>>> auto l = [&] (parms) // space between ] (
>>>>     {                  // brace on new line, indented two spaces
>>>>       return stuff;
>>>>     };
>>>
>>> Sure, why not.  Consistency is what matters.  Thus far we seem
>>> to have been very inconsistent.  ;)
>>>> By default, recent emacs lines up the { with the previous line, like an
>>>> in-class function definition; I talked it into the above indentation with
>>>>
>>>> (defun lambda-offset (elem)
>>>>     (if (assq 'inline-open c-syntactic-context) '+ 0))
>>>> (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
>>>>
>>>> I think we probably want the same formatting for lambdas in function
>>>> argument lists, e.g.
>>>>
>>>> algorithm ([] (parms)
>>>>     {
>>>>       return foo;
>>>>     });
>>>
>>> And what about lambdas in conditions:
>>>
>>> if (foo ()
>>>       && [&] (params) mutable
>>>          {
>>> 	 return 42;
>>>          } ())
>>>
>>> should the { go just below [?
>>
>> I think we don't want the { to go below the [ in general; that was the old
>> emacs default behavior, and it produced lambda bodies with excessive
>> indentation.
>>
>> With my adjustment above, emacs indents the { two spaces from the &&, which
>> seems a bit arbitrary but reasonable.
> 
> Fair enough, I think that's better.
> 
> I suppose we should add a note wrt lambdas to
> https://gcc.gnu.org/codingconventions.html#Cxx_Conventions

Indeed, that's my goal.

Looking through the various lambdas already in the compiler, I see a 
good number already follow my suggested indentation, but a large 
majority omit the space between ] and (, so maybe we don't want it?

Jason

Re: lambda coding style

[waffl3x]

On Wednesday, January 10th, 2024 at 7:34 PM, Jason Merrill via Gcc <gcc@gcc.gnu.org> wrote:


> 
> 
> On 1/10/24 16:41, Marek Polacek wrote:
> 
> > On Wed, Jan 10, 2024 at 04:24:42PM -0500, Jason Merrill wrote:
> > 
> > > On 1/10/24 15:59, Marek Polacek wrote:
> > > 
> > > > On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
> > > > 
> > > > > What formatting style do we want for non-trivial lambdas in GCC sources?
> > > > > I'm thinking the most consistent choice would be
> > > > > 
> > > > > auto l = [&] (parms) // space between ] (
> > > > > { // brace on new line, indented two spaces
> > > > > return stuff;
> > > > > };
> > > > 
> > > > Sure, why not. Consistency is what matters. Thus far we seem
> > > > to have been very inconsistent. ;)
> > > > 
> > > > > By default, recent emacs lines up the { with the previous line, like an
> > > > > in-class function definition; I talked it into the above indentation with
> > > > > 
> > > > > (defun lambda-offset (elem)
> > > > > (if (assq 'inline-open c-syntactic-context) '+ 0))
> > > > > (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
> > > > > 
> > > > > I think we probably want the same formatting for lambdas in function
> > > > > argument lists, e.g.
> > > > > 
> > > > > algorithm ([] (parms)
> > > > > {
> > > > > return foo;
> > > > > });
> > > > 
> > > > And what about lambdas in conditions:
> > > > 
> > > > if (foo ()
> > > > && [&] (params) mutable
> > > > {
> > > > return 42;
> > > > } ())
> > > > 
> > > > should the { go just below [?
> > > 
> > > I think we don't want the { to go below the [ in general; that was the old
> > > emacs default behavior, and it produced lambda bodies with excessive
> > > indentation.
> > > 
> > > With my adjustment above, emacs indents the { two spaces from the &&, which
> > > seems a bit arbitrary but reasonable.
> > 
> > Fair enough, I think that's better.
> > 
> > I suppose we should add a note wrt lambdas to
> > https://gcc.gnu.org/codingconventions.html#Cxx_Conventions
> 
> 
> Indeed, that's my goal.
> 
> Looking through the various lambdas already in the compiler, I see a
> good number already follow my suggested indentation, but a large
> majority omit the space between ] and (, so maybe we don't want it?
> 
> Jason

I'm of the opinion that you should just be naming the lambdas once you
want to use them in complex logic. It gets really messy really quick,
if not immediately. It's not as bad when used in algorithms since they
generally are the last parameter, and it's an established practice to
do so, but it's still not super wonderful when they get long.

The other thing I want to raise is whether we want to allow different
formatting for IILE's, having the opening brace on a new line makes
those use cases look a lot messier imo. IILE's stand out quite a bit
already so the rationale for the current bracing is a little weaker for
lambdas imo, especially IILE's. But for a lambda with parameters I can
see the value of formatting it more strictly.

With that said I understand the value of consistency, so perhaps
consistency is the better way to go.

Alex

Re: lambda coding style

[Tom Tromey]

>>>>> "Jason" == Jason Merrill via Gcc <gcc@gcc.gnu.org> writes:

Jason> I think we probably want the same formatting for lambdas in function
Jason> argument lists, e.g.

Jason> algorithm ([] (parms)
Jason>   {
Jason>     return foo;
Jason>   });

Jason> Any other opinions?

FWIW gdb did pretty much this same thing.  Our rules are documented
here:

https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Indentation_of_lambdas_as_parameters

There's a special case in here where a call takes a single lambda as the
last argument -- gdb indents that in a more block-like way.

Tom

Re: lambda coding style

[Martin Jambor]

Hi,

On Wed, Jan 10 2024, Jason Merrill via Gcc wrote:
> What formatting style do we want for non-trivial lambdas in GCC sources? 
>   I'm thinking the most consistent choice would be
>
> auto l = [&] (parms) // space between ] (
>    {                  // brace on new line, indented two spaces
>      return stuff;
>    };
>
> By default, recent emacs lines up the { with the previous line, like an 
> in-class function definition; I talked it into the above indentation with
>
> (defun lambda-offset (elem)
>    (if (assq 'inline-open c-syntactic-context) '+ 0))
> (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))

Is this really add-to-hook and not add-hook?

Thanks!

Martin

>
> I think we probably want the same formatting for lambdas in function 
> argument lists, e.g.
>
> algorithm ([] (parms)
>    {
>      return foo;
>    });
>
> Any other opinions?
>
> Jason

Re: lambda coding style

[Jason Merrill]

On 1/11/24 12:48, Martin Jambor wrote:
> On Wed, Jan 10 2024, Jason Merrill via Gcc wrote:
>> What formatting style do we want for non-trivial lambdas in GCC sources?
>>    I'm thinking the most consistent choice would be
>>
>> auto l = [&] (parms) // space between ] (
>>     {                  // brace on new line, indented two spaces
>>       return stuff;
>>     };
>>
>> By default, recent emacs lines up the { with the previous line, like an
>> in-class function definition; I talked it into the above indentation with
>>
>> (defun lambda-offset (elem)
>>     (if (assq 'inline-open c-syntactic-context) '+ 0))
>> (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
> 
> Is this really add-to-hook and not add-hook?

Oops! add-to-hook is a custom variant I wrote back in like 1992; for 
add-hook you'll want

(add-hook 'c++-mode-hook
           '(lambda () (c-set-offset 'inlambda 'lambda-offset)))

Jason

Re: lambda coding style

[Martin Jambor]

Hi,

On Thu, Jan 11 2024, Jason Merrill wrote:
> On 1/11/24 12:48, Martin Jambor wrote:
>> On Wed, Jan 10 2024, Jason Merrill via Gcc wrote:
>>> What formatting style do we want for non-trivial lambdas in GCC sources?
>>>    I'm thinking the most consistent choice would be
>>>
>>> auto l = [&] (parms) // space between ] (
>>>     {                  // brace on new line, indented two spaces
>>>       return stuff;
>>>     };
>>>
>>> By default, recent emacs lines up the { with the previous line, like an
>>> in-class function definition; I talked it into the above indentation with
>>>
>>> (defun lambda-offset (elem)
>>>     (if (assq 'inline-open c-syntactic-context) '+ 0))
>>> (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
>> 
>> Is this really add-to-hook and not add-hook?
>
> Oops! add-to-hook is a custom variant I wrote back in like 1992; for 
> add-hook you'll want
>
> (add-hook 'c++-mode-hook
>            '(lambda () (c-set-offset 'inlambda 'lambda-offset)))
>

I see, thanks!

Martin

Re: lambda coding style

[Hans-Peter Nilsson]

On Wed, 10 Jan 2024, Jason Merrill via Gcc wrote:
> On 1/10/24 15:59, Marek Polacek wrote:
> > On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
> > > What formatting style do we want for non-trivial lambdas in GCC sources?
> > > I'm thinking the most consistent choice would be
> > > 
> > > auto l = [&] (parms) // space between ] (
> > >    {                  // brace on new line, indented two spaces
> > >      return stuff;
> > >    };
> > 
> > Sure, why not.  Consistency is what matters.  Thus far we seem
> > to have been very inconsistent.  ;)
> >   
> > > By default, recent emacs lines up the { with the previous line, like an
> > > in-class function definition; I talked it into the above indentation with
> > > 
> > > (defun lambda-offset (elem)
> > >    (if (assq 'inline-open c-syntactic-context) '+ 0))
> > > (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
> > > 
> > > I think we probably want the same formatting for lambdas in function
> > > argument lists, e.g.
> > > 
> > > algorithm ([] (parms)
> > >    {
> > >      return foo;
> > >    });
> > 
> > And what about lambdas in conditions:
> > 
> > if (foo ()
> >      && [&] (params) mutable
> >         {
> > 	 return 42;
> >         } ())
> > 
> > should the { go just below [?

Also, what about trailing-type and mutable (above) when needing 
a line-break?

(FTR: in technical terms, trailing-type is known as the 
pointy-arrow-declaring-return-type thing :) the optional "-> 
type" between "(parms)" and "{ body }")

I suggest the obvious (to me): line up stuff after (params) with 
the opening brace for body, when needing a line-break before 
that item, and line-break *before* "->" .

brgds, H-P