[Bug build/30098] New: Keep trying clang-format

[Bug build/30098] New: Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

            Bug ID: 30098
           Summary: Keep trying clang-format
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: build
          Assignee: unassigned at sourceware dot org
          Reporter: tromey at sourceware dot org
  Target Milestone: ---

We periodically discuss clang-format.
I think we should have a bug to track our progress.

I'll attach my current .clang-format and add some commentary
about the problems.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #1 from Tom Tromey <tromey at sourceware dot org> ---
Created attachment 14660
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14660&action=edit
the clang-format config file

Here's my current .clang-format, updated to clang-format 14.

I updated by reading:

https://clang.llvm.org/docs/ClangFormatStyleOptions.html

and considering any parameter after 10 (the last time I updated)
and before 15 (my machine has 14).

Then to evaluate this I run it on various files and spot check them.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #2 from Tom Tromey <tromey at sourceware dot org> ---
Last time I did this I noted a few upstream bugs:

Goto label indentation is wrong
https://github.com/llvm/llvm-project/issues/24492

Case label / goto label:
https://github.com/llvm/llvm-project/issues/53717

Bin-packing is a mess:
https://github.com/llvm/llvm-project/issues/37051
https://github.com/llvm/llvm-project/issues/31255

The first two of these aren't that important probably.
They result in minor differences but kind of meh ones really.

Bin-packing remains problematic for me.
I'll try to find an example.

There isn't an upstream bug open for this, but there's
no way to say that '_' is a special function not requiring
a space, so all the source is rewritten to '_ ("blah")'

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
BTW there are a bunch of FIXMEs in the config file.
I don't really know how the penalty system works so
maybe that can be improved too.

Sometimes clang-format makes bad decisions.  One example
I'm looking at is if you have an 'if' that is a series of
clauses joined by '&&', sometimes it is clearest to put
each such clause on its own line.  However clang-format does this:

@@ -194,10 +193,8 @@ eq_bfd (const void *a, const void *b)
     = (const struct gdb_bfd_cache_search *) b;
   struct gdb_bfd_data *gdata = (struct gdb_bfd_data *) bfd_usrdata (abfd);

-  return (gdata->mtime == s->mtime
-         && gdata->size == s->size
-         && gdata->inode == s->inode
-         && gdata->device_id == s->device_id
+  return (gdata->mtime == s->mtime && gdata->size == s->size
+         && gdata->inode == s->inode && gdata->device_id == s->device_id
          && strcmp (bfd_get_filename (abfd), s->filename) == 0);
 }

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #4 from Tom Tromey <tromey at sourceware dot org> ---
Also the config file needs a list of all the attribute
macros, e.g. from ansidecl.h (and elsewhere?).
Otherwise:

@@ -1148,11 +1124,11 @@ static bfd_error_handler_type
default_bfd_error_handler;
    per-inferior basis.  */

 static void ATTRIBUTE_PRINTF (1, 0)
-gdb_bfd_error_handler (const char *fmt, va_list ap)
+  gdb_bfd_error_handler (const char *fmt, va_list ap)
 {
   va_list ap_copy;

-  va_copy(ap_copy, ap);
+  va_copy (ap_copy, ap);
   const std::string str = string_vprintf (fmt, ap_copy);
   va_end (ap_copy);

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #5 from Tom Tromey <tromey at sourceware dot org> ---
One final thought -- if we do ever run this on the repo,
it would be good to use some phony setting for --author,
so it's readily obvious in 'git annotate' that some commit
is just the reformatting.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simark at simark dot ca

--- Comment #6 from Simon Marchi <simark at simark dot ca> ---
Thanks for filing this.

Can we make some branch on sourceware with the latest attempt at doing this,
that we could update over time?  I would suggest to have just two commits over
master, one commit to add .clang-format (and maybe a script to run clang-format
correctly to do a mass reformat) and another commit that is the actual mass
reformat.

Using clang-format-15, I see some changes that are not stable in
hppa-netbsd-tdep.c, clang-format changes the code every time I run it:

diff --git a/gdb/hppa-netbsd-tdep.c b/gdb/hppa-netbsd-tdep.c
index 5468a135caf7..7b4c8c71fe2f 100644
--- a/gdb/hppa-netbsd-tdep.c
+++ b/gdb/hppa-netbsd-tdep.c
@@ -63,30 +63,29 @@ static void hppanbsd_sigtramp_cache_init (const struct
tramp_frame *,
                                          struct trad_frame_cache *,
                                          CORE_ADDR);

-static const struct tramp_frame hppanbsd_sigtramp_si4 = {
-  SIGTRAMP_FRAME,
-  4,
-  { { 0xc7d7c012, ULONGEST_MAX }, /*   bb,>=,n %arg3, 30, 1f           */
-    { 0xd6e01c1e, ULONGEST_MAX }, /*    depwi 0,31,2,%arg3             */
-    { 0x0ee81093, ULONGEST_MAX }, /*   ldw 4(%arg3), %r19              */
-    { 0x0ee01097, ULONGEST_MAX }, /*   ldw 0(%arg3), %arg3             */
-    /* 1:                                      */
-    { 0xe8404000, ULONGEST_MAX }, /*   blr %r0, %rp                    */
-    { 0xeae0c002, ULONGEST_MAX }, /*   bv,n %r0(%arg3)                 */
-    { 0x08000240, ULONGEST_MAX }, /*    nop                            */
-
-    { 0x0803025a, ULONGEST_MAX }, /*   copy %r3, %arg0                 */
-    { 0x20200801, ULONGEST_MAX }, /*   ldil -40000000, %r1             */
-    { 0xe420e008, ULONGEST_MAX }, /*   be,l 4(%sr7, %r1), %sr0, %r31   */
-    { 0x34160268, ULONGEST_MAX }, /*    ldi 134, %t1 ; SYS_setcontext  */
-
-    { 0x081c025a, ULONGEST_MAX }, /*   copy ret0, %arg0                */
-    { 0x20200801, ULONGEST_MAX }, /*   ldil -40000000, %r1             */
-    { 0xe420e008, ULONGEST_MAX }, /*   be,l 4(%sr7, %r1), %sr0, %r31   */
-    { 0x34160002, ULONGEST_MAX }, /*    ldi 1, %t1 ; SYS_exit          */
-    { TRAMP_SENTINEL_INSN, ULONGEST_MAX } },
-  hppanbsd_sigtramp_cache_init
-};
+static const struct tramp_frame hppanbsd_sigtramp_si4
+  = { SIGTRAMP_FRAME,
+      4,
+      { { 0xc7d7c012, ULONGEST_MAX }, /*       bb,>=,n %arg3, 30, 1f          
*/
+       { 0xd6e01c1e, ULONGEST_MAX }, /*         depwi 0,31,2,%arg3            
*/
+       { 0x0ee81093, ULONGEST_MAX }, /*        ldw 4(%arg3), %r19             
*/
+       { 0x0ee01097, ULONGEST_MAX }, /*        ldw 0(%arg3), %arg3            
*/
+       /* 1:                                   */
+       { 0xe8404000, ULONGEST_MAX }, /*        blr %r0, %rp                   
*/
+       { 0xeae0c002, ULONGEST_MAX }, /*        bv,n %r0(%arg3)                
*/
+       { 0x08000240, ULONGEST_MAX }, /*         nop                           
*/
+
+       { 0x0803025a, ULONGEST_MAX }, /*        copy %r3, %arg0                
*/
+       { 0x20200801, ULONGEST_MAX }, /*        ldil -40000000, %r1            
*/
+       { 0xe420e008, ULONGEST_MAX }, /*        be,l 4(%sr7, %r1), %sr0, %r31  
*/
+       { 0x34160268, ULONGEST_MAX }, /*         ldi 134, %t1 ; SYS_setcontext 
*/
+
+       { 0x081c025a, ULONGEST_MAX }, /*        copy ret0, %arg0               
*/
+       { 0x20200801, ULONGEST_MAX }, /*        ldil -40000000, %r1            
*/
+       { 0xe420e008, ULONGEST_MAX }, /*        be,l 4(%sr7, %r1), %sr0, %r31  
*/
+       { 0x34160002, ULONGEST_MAX }, /*         ldi 1, %t1 ; SYS_exit         
*/
+       { TRAMP_SENTINEL_INSN, ULONGEST_MAX } },
+      hppanbsd_sigtramp_cache_init };

Probably a bug in clang-format?  I guess that could be worked around by placing
markers around that code to prevent clang-format from touching it.

In my .clang-format, I had `UseTab: Always`, but it doesn't seem to make a
difference compared to what you have.


> Sometimes clang-format makes bad decisions.  One example
> I'm looking at is if you have an 'if' that is a series of
> clauses joined by '&&', sometimes it is clearest to put
> each such clause on its own line.  However clang-format does this:
> 
> @@ -194,10 +193,8 @@ eq_bfd (const void *a, const void *b)
>      = (const struct gdb_bfd_cache_search *) b;
>    struct gdb_bfd_data *gdata = (struct gdb_bfd_data *) bfd_usrdata (abfd);
>  
> -  return (gdata->mtime == s->mtime
> -	  && gdata->size == s->size
> -	  && gdata->inode == s->inode
> -	  && gdata->device_id == s->device_id
> +  return (gdata->mtime == s->mtime && gdata->size == s->size
> +	  && gdata->inode == s->inode && gdata->device_id == s->device_id
>  	  && strcmp (bfd_get_filename (abfd), s->filename) == 0);
>  }

I agree that the old one is more readable.  I suppose that could be a
clang-format rule, similar to BinPackParameters.  If an expression fits on one
line, leave it on one line.  Otherwise, break at binary operators.  It could
happen with other operators too, like:

 something = (aaaaaaaa + bbbbbbbb + cccccccc
              + dddddddd); 

vs

 something = (aaaaaaaa
              + bbbbbbbb
              + cccccccc
              + dddddddd);

Of course, humans will pretty much always make better decisions for things like
this.  For instance, you might want to have it organized logically like this:

 path = (base
         + "/" + subdir1
         + "/" + subdir2
         + "/" + subdir3);

But the tool will either choose:


 path = (base + "/" + subdir1 + "/"
         + subdir2 + "/" + subdir3);

of

 path = (base
         + "/"
         + subdir1
         + "/"
         + subdir2
         + "/"
         + subdir3);

If we want to use an automated tool, we have to accept the tradeoff that some
cases will be less nice, in exchange for convenience.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[jan at vrany dot io]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

Jan Vrany <jan at vrany dot io> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jan at vrany dot io

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[sam at gentoo dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

Sam James <sam at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sam at gentoo dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #7 from Tom Tromey <tromey at sourceware dot org> ---
> Probably a bug in clang-format?

Seems like it has to be, formatters ought to be idempotent.
Otherwise if we use it we will get a lot of commit churn.

> If we want to use an automated tool, we have to accept the tradeoff that some cases will be less nice, in exchange for convenience.

Yeah, I agree.  I just feel some things are too ugly.
I can probably live with most of the changes clang-format imposes,
but I am not sure that is true for all of them.

Also I found out that it doesn't format lambdas close to what we like.
It puts the opening brace at the wrong spot, so I wonder if maybe
we're missing a config item.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #8 from Tom Tromey <tromey at sourceware dot org> ---
FWIW I looked at the new settings for clang-format 15 and 16
and they didn't seem useful to gdb.
Also none of the bugs linked above seem to have been fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #9 from Tom Tromey <tromey at sourceware dot org> ---
I looked through the docs for clang-format 20 and still didn't
see any relevant changes.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

Simon Marchi <simon.marchi at polymtl dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simon.marchi at polymtl dot ca

--- Comment #10 from Simon Marchi <simon.marchi at polymtl dot ca> ---
I was thinking that we could still commit a .clang-format to our repo, as the
"current best approximation" of the config for our code style, with a comment
warning that it should not be blindly trusted.  It can be improved with time,
and it can even be useful right away.

For instance, I sometimes run git-clang-format on my patches, and then
selectively add the good changes with "git add -p".  It does find some mistakes
or some better way to format things (like removing an unnecessary line wrap). 
I also use it through VSCode to format specific code sections.  Again, the
output is not to be trusted blindly, but it's often useful.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[jan at vrany dot io]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #11 from Jan Vrany <jan at vrany dot io> ---
(In reply to Simon Marchi from comment #10)
> I was thinking that we could still commit a .clang-format to our repo

I'd welcome that, exactly for the reasons you mentioned.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #12 from Tom Tromey <tromey at sourceware dot org> ---
If we land it then we'll have contributors thinking it is real.
On the other hand we already have to review for coding style,
so nothing there will change.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #13 from Tom Tromey <tromey at sourceware dot org> ---
I'm trying it a bit more.

There doesn't seem to be a way to fix lambdas.
We'd have to use "BreakBeforeBraces: Custom", but "GNU"
mode here has special behavior that I don't see how to
reproduce.  Maybe I'm missing it.  Anyway this results
in:

-         auto open = [&] (bfd *nbfd) -> gdb_bfd_iovec_base *
-         {
+         auto open = [&] (bfd *nbfd) -> gdb_bfd_iovec_base * {
            return gdb_bfd_iovec_fileio_open (nbfd, current_inferior (),
                                              warn_if_slow);

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #14 from Tom Tromey <tromey at sourceware dot org> ---
Here's a mild example of the bin-packing problem.
Here, clang-format decides to keep the args together,
while in gdb I suppose it's more normal to keep the
assignment "together":

-         descriptor->data = bfd_mmap (abfd, 0, descriptor->size, PROT_READ,
-                                      MAP_PRIVATE, sectp->filepos,
-                                      &descriptor->map_addr,
-                                      &descriptor->map_len);
+         descriptor->data
+           = bfd_mmap (abfd, 0, descriptor->size, PROT_READ, MAP_PRIVATE,
+                       sectp->filepos, &descriptor->map_addr,
+                       &descriptor->map_len);

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #15 from Tom Tromey <tromey at sourceware dot org> ---
One nit that is kind of annoying is that it insists on putting
a space into gettext calls, so it rewrites all the code 
from _("text") to _ ("text")

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #16 from Tom Tromey <tromey at sourceware dot org> ---
I find this kind of ugly:

-    return (static_cast<dwarf2_per_cu_data *>
-           (m_addrmap->find ((CORE_ADDR) addr)));
+    return (
+      static_cast<dwarf2_per_cu_data *> (m_addrmap->find ((CORE_ADDR) addr)));

I'm not a fan of the dangling "("

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #17 from Tom Tromey <tromey at sourceware dot org> ---
Here's a case where the formatter tries to be as concise
as possible, but which IMO negatively affects readability:

 static enum debug_loc_kind
-decode_debug_loc_dwo_addresses (dwarf2_per_cu_data *per_cu,
-                               dwarf2_per_objfile *per_objfile,
-                               const gdb_byte *loc_ptr,
-                               const gdb_byte *buf_end,
-                               const gdb_byte **new_ptr,
-                               unrelocated_addr *low,
-                               unrelocated_addr *high,
-                               enum bfd_endian byte_order)
+decode_debug_loc_dwo_addresses (
+  dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile,
+  const gdb_byte *loc_ptr, const gdb_byte *buf_end, const gdb_byte **new_ptr,
+  unrelocated_addr *low, unrelocated_addr *high, enum bfd_endian byte_order)
 {

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[tankut.baris.aktemur at intel dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

Baris Aktemur <tankut.baris.aktemur at intel dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tankut.baris.aktemur@intel.
                   |                            |com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #18 from Simon Marchi <simon.marchi at polymtl dot ca> ---
(In reply to Tom Tromey from comment #16)
> I find this kind of ugly:
> 
> -    return (static_cast<dwarf2_per_cu_data *>
> -	    (m_addrmap->find ((CORE_ADDR) addr)));
> +    return (
> +      static_cast<dwarf2_per_cu_data *> (m_addrmap->find ((CORE_ADDR)
> addr)));
> 
> I'm not a fan of the dangling "("

The use of the outer parenthesis exists, I've been told, to make emacs align
the continuation after the parenthesis.

If you remove those parenthesis, it gives:

    return static_cast<dwarf2_per_cu_data *> (
      m_addrmap->find ((CORE_ADDR) addr));

vs the existing:

    return (static_cast<dwarf2_per_cu_data *>
            (m_addrmap->find ((CORE_ADDR) addr)));


I don't find the first one any more ugly than the second one.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #19 from Simon Marchi <simon.marchi at polymtl dot ca> ---
(In reply to Tom Tromey from comment #14)
> Here's a mild example of the bin-packing problem.
> Here, clang-format decides to keep the args together,
> while in gdb I suppose it's more normal to keep the
> assignment "together":
> 
> -	  descriptor->data = bfd_mmap (abfd, 0, descriptor->size, PROT_READ,
> -				       MAP_PRIVATE, sectp->filepos,
> -				       &descriptor->map_addr,
> -				       &descriptor->map_len);
> +	  descriptor->data
> +	    = bfd_mmap (abfd, 0, descriptor->size, PROT_READ, MAP_PRIVATE,
> +			sectp->filepos, &descriptor->map_addr,
> +			&descriptor->map_len);

I think that either are fine.  Sometimes there are calls that could fit with
the first style, but the result is very ugly because it's all pushed to the
right where there isn't much columns left for the arguments.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #20 from Simon Marchi <simon.marchi at polymtl dot ca> ---
(In reply to Tom Tromey from comment #17)
> Here's a case where the formatter tries to be as concise
> as possible, but which IMO negatively affects readability:
> 
>  static enum debug_loc_kind
> -decode_debug_loc_dwo_addresses (dwarf2_per_cu_data *per_cu,
> -				dwarf2_per_objfile *per_objfile,
> -				const gdb_byte *loc_ptr,
> -				const gdb_byte *buf_end,
> -				const gdb_byte **new_ptr,
> -				unrelocated_addr *low,
> -				unrelocated_addr *high,
> -				enum bfd_endian byte_order)
> +decode_debug_loc_dwo_addresses (
> +  dwarf2_per_cu_data *per_cu, dwarf2_per_objfile *per_objfile,
> +  const gdb_byte *loc_ptr, const gdb_byte *buf_end, const gdb_byte
> **new_ptr,
> +  unrelocated_addr *low, unrelocated_addr *high, enum bfd_endian byte_order)
>  {

I do like the first one in this case, because the function name is not too
long.  But if the function name is longer, the parameter list gets awkwardly
pushed to the right hand side with very few columns available.

If I use these:

PenaltyBreakBeforeFirstCallParameter: 1000
PenaltyBreakOpenParenthesis: 1000
PenaltyReturnTypeOnItsOwnLine: 1000

... it is close to the original version:

static enum debug_loc_kind
decode_debug_loc_dwo_addresses (dwarf2_per_cu_data *per_cu,
                                dwarf2_per_objfile *per_objfile,
                                const gdb_byte *loc_ptr,
                                const gdb_byte *buf_end,
                                const gdb_byte **new_ptr, unrelocated_addr
*low,
                                unrelocated_addr *high,
                                enum bfd_endian byte_order)

The only difference is that it put the "low" parameter next to the "new_ptr"
parameter, because it had enough room to do so (if the column limit was
greater, there would be more packing).

The BinPackParameters option has a new "OnePerLine" enum value in clang-format
20 (it used to be boolean), which means "Put all parameters on the current line
if they fit. Otherwise, put each one on its own line.".  It would give exactly
the existing formatting:

static enum debug_loc_kind
decode_debug_loc_dwo_addresses (dwarf2_per_cu_data *per_cu,
                                dwarf2_per_objfile *per_objfile,
                                const gdb_byte *loc_ptr,
                                const gdb_byte *buf_end,
                                const gdb_byte **new_ptr,
                                unrelocated_addr *low,
                                unrelocated_addr *high,
                                enum bfd_endian byte_order)

But it means that whenever parameters don't fit on a single line, they'll be
one per line, for instance:

-target_desc *amd64_create_target_description (uint64_t xcr0, bool is_x32,
-                                             bool is_linux, bool segments);
+target_desc *amd64_create_target_description (uint64_t xcr0,
+                                             bool is_x32,
+                                             bool is_linux,
+                                             bool segments);

I don't really care, I would be fine with either style.  I think that as
humans, we sometimes make artistic choices, sometimes we prefer one per line,
sometimes we don't (and it is subjective).  Whereas a tool can't really know
when you'd prefer one style over the other, we have to choose one.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/30098] Keep trying clang-format

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=30098

--- Comment #21 from Simon Marchi <simon.marchi at polymtl dot ca> ---
See here for a fully reformatted tree using that config (which is Tom's config
with the tweaks mentioned above):

https://github.com/simark/binutils-gdb/tree/clang-format-19

-- 
You are receiving this mail because:
You are on the CC list for the bug.