public inbox for gdb-prs@sourceware.org help / color / mirror / Atom feed
* [Bug build/30098] New: Keep trying clang-format @ 2023-02-08 19:57 tromey at sourceware dot org 2023-02-08 20:00 ` [Bug build/30098] " tromey at sourceware dot org ` (9 more replies) 0 siblings, 10 replies; 11+ messages in thread From: tromey at sourceware dot org @ 2023-02-08 19:57 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org @ 2023-02-08 20:00 ` tromey at sourceware dot org 2023-02-08 20:05 ` tromey at sourceware dot org ` (8 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: tromey at sourceware dot org @ 2023-02-08 20:00 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org 2023-02-08 20:00 ` [Bug build/30098] " tromey at sourceware dot org @ 2023-02-08 20:05 ` tromey at sourceware dot org 2023-02-08 20:07 ` tromey at sourceware dot org ` (7 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: tromey at sourceware dot org @ 2023-02-08 20:05 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org 2023-02-08 20:00 ` [Bug build/30098] " tromey at sourceware dot org 2023-02-08 20:05 ` tromey at sourceware dot org @ 2023-02-08 20:07 ` tromey at sourceware dot org 2023-02-08 20:09 ` tromey at sourceware dot org ` (6 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: tromey at sourceware dot org @ 2023-02-08 20:07 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org ` (2 preceding siblings ...) 2023-02-08 20:07 ` tromey at sourceware dot org @ 2023-02-08 20:09 ` tromey at sourceware dot org 2023-02-08 20:10 ` tromey at sourceware dot org ` (5 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: tromey at sourceware dot org @ 2023-02-08 20:09 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org ` (3 preceding siblings ...) 2023-02-08 20:09 ` tromey at sourceware dot org @ 2023-02-08 20:10 ` tromey at sourceware dot org 2023-02-08 20:32 ` simark at simark dot ca ` (4 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: tromey at sourceware dot org @ 2023-02-08 20:10 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org ` (4 preceding siblings ...) 2023-02-08 20:10 ` tromey at sourceware dot org @ 2023-02-08 20:32 ` simark at simark dot ca 2023-02-08 21:31 ` jan at vrany dot io ` (3 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: simark at simark dot ca @ 2023-02-08 20:32 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org ` (5 preceding siblings ...) 2023-02-08 20:32 ` simark at simark dot ca @ 2023-02-08 21:31 ` jan at vrany dot io 2023-02-08 22:27 ` sam at gentoo dot org ` (2 subsequent siblings) 9 siblings, 0 replies; 11+ messages in thread From: jan at vrany dot io @ 2023-02-08 21:31 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org ` (6 preceding siblings ...) 2023-02-08 21:31 ` jan at vrany dot io @ 2023-02-08 22:27 ` sam at gentoo dot org 2023-02-08 23:57 ` tromey at sourceware dot org 2023-08-03 16:53 ` tromey at sourceware dot org 9 siblings, 0 replies; 11+ messages in thread From: sam at gentoo dot org @ 2023-02-08 22:27 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org ` (7 preceding siblings ...) 2023-02-08 22:27 ` sam at gentoo dot org @ 2023-02-08 23:57 ` tromey at sourceware dot org 2023-08-03 16:53 ` tromey at sourceware dot org 9 siblings, 0 replies; 11+ messages in thread From: tromey at sourceware dot org @ 2023-02-08 23:57 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug build/30098] Keep trying clang-format 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org ` (8 preceding siblings ...) 2023-02-08 23:57 ` tromey at sourceware dot org @ 2023-08-03 16:53 ` tromey at sourceware dot org 9 siblings, 0 replies; 11+ messages in thread From: tromey at sourceware dot org @ 2023-08-03 16:53 UTC (permalink / raw) To: gdb-prs 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-08-03 16:53 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-08 19:57 [Bug build/30098] New: Keep trying clang-format tromey at sourceware dot org 2023-02-08 20:00 ` [Bug build/30098] " tromey at sourceware dot org 2023-02-08 20:05 ` tromey at sourceware dot org 2023-02-08 20:07 ` tromey at sourceware dot org 2023-02-08 20:09 ` tromey at sourceware dot org 2023-02-08 20:10 ` tromey at sourceware dot org 2023-02-08 20:32 ` simark at simark dot ca 2023-02-08 21:31 ` jan at vrany dot io 2023-02-08 22:27 ` sam at gentoo dot org 2023-02-08 23:57 ` tromey at sourceware dot org 2023-08-03 16:53 ` tromey at sourceware dot org
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).