Formatting/indentation of lambdas (Re: [PATCH 2/3] gdb/varobj: Fix use after free in varobj)

[Pedro Alves <pedro@palves.net>]

On 2022-06-17 17:09, Andrew Burgess via Gdb-patches wrote:

>>  /* Update the internal variables and value history when OBJFILE is
>>     discarded; we must copy the types out of the objfile.  New global types
>>     will be created for every convenience variable which currently points to
>> @@ -2617,6 +2633,11 @@ preserve_values (struct objfile *objfile)
>>    for (var = internalvars; var; var = var->next)
>>      preserve_one_internalvar (var, objfile, copied_types.get ());
>>  
>> +  /* For the remaining varobj, check that none has type owned by OBJFILE.  */
>> +  all_root_varobjs ([&copied_types, objfile](struct varobj *varobj)
>> +		    { preserve_one_varobj (varobj, objfile,
>> +					   copied_types.get ()); });
>> +
> 
> I think the formatting here is a little off.  Looking through other
> examples in GDB I think the most common layout would be:
> 
>   all_root_varobjs ([&copied_types, objfile] (struct varobj *varobj)
>                     {
> 		      preserve_one_varobj (varobj, objfile,
> 					   copied_types.get ());
> 		    });

For for-each-like functions that take a lambda, I like to indent the body
of lambda as-if you really had a for loop.  Like:

  all_root_varobjs ([&copied_types, objfile] (struct varobj *varobj)
    {
      preserve_one_varobj (varobj, objfile, copied_types.get ());
    });

which looks similar to what you'd have if you had a real for, like:

  for (varobj *varobj: root_varobjs ())
    {
      preserve_one_varobj (varobj, objfile, copied_types.get ());
    }


I'll give you at least a couple existing examples.  E.g., in gdb/linux-nat.c:

  /* No use iterating unless we're resuming other threads.  */
  if (scope_ptid != lp->ptid)
    iterate_over_lwps (scope_ptid, [=] (struct lwp_info *info)
      {
        return linux_nat_resume_callback (info, lp);
      });

and in gdbserver, any for_each_thread call, like:

  for_each_thread ([&] (thread_info *thread)
    {
      handle_qxfer_threads_worker (thread, buffer);
    });

I've noticed that LLVM also uses this style.  It is nicely described here:

 https://llvm.org/docs/CodingStandards.html#format-lambdas-like-blocks-of-code

For the case where we have multiple lambdas in a single function call, or when the
lambda isn't the last argument, I agree with LLVM, adjusted for GNU style.  As in, indent
like other parameters. An example for this is expand_symtabs_matching, which takes
several gdb::function_view arguments.  Here's an example call:

  /* Look through the partial symtabs for all symbols which begin by
     matching SYM_TEXT.  Expand all CUs that you find to the list.  */
  expand_symtabs_matching (NULL,
			   lookup_name,
			   NULL,
			   [&] (compunit_symtab *symtab) /* expansion notify */
			     {
			       add_symtab_completions (symtab,
						       tracker, mode, lookup_name,
						       sym_text, word, code);
			       return true;
			     },
			   SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
			   ALL_DOMAIN);

I suggest we follow these conventions, and document it in the internals manual,
so we have a url we can point to the next time this comes up (it's not the first time).