public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Remove duplicate or commented-out #includes
@ 2019-01-19 21:31 Tom Tromey
  2019-01-21 17:28 ` Gary Benson
  2019-01-21 17:34 ` Simon Marchi
  0 siblings, 2 replies; 7+ messages in thread
From: Tom Tromey @ 2019-01-19 21:31 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

I wrote a little script to detect duplicate or commented-out #includes
and ran it on gdb.  This patch is the result.  Tested by rebuilding.

It would be possible to sort #includes, or maybe run
include-what-you-use on gdb, but I haven't tried that.  I'd be
interested to hear if you think this would be worthwhile, thoug.

gdb/ChangeLog
2019-01-19  Tom Tromey  <tromey@bapiya>

	* ui-out.c: Fix includes.
	* tui/tui-source.c: Fix includes.
	* target.c: Fix includes.
	* remote.c: Fix includes.
	* regcache.c: Fix includes.
	* python/py-block.c: Fix includes.
	* printcmd.c: Fix includes.
	* or1k-tdep.c: Fix includes.
	* mi/mi-main.c: Fix includes.
	* m32r-tdep.c: Fix includes.
	* csky-tdep.c: Fix includes.
	* compile/compile-cplus-types.c: Fix includes.
	* cli/cli-interp.c: Fix includes.

gdb/gdbserver/ChangeLog
2019-01-19  Tom Tromey  <tromey@bapiya>

	* tracepoint.c: Fix includes.
	* remote-utils.c: Fix includes.
	* linux-x86-low.c: Fix includes.

gdb/stubs/ChangeLog
2019-01-19  Tom Tromey  <tromey@bapiya>

	* ia64vms-stub.c: Fix includes.
---
 gdb/ChangeLog                     | 16 ++++++++++++++++
 gdb/cli/cli-interp.c              |  1 -
 gdb/compile/compile-cplus-types.c |  1 -
 gdb/csky-tdep.c                   |  2 --
 gdb/gdbserver/ChangeLog           |  6 ++++++
 gdb/gdbserver/linux-x86-low.c     |  1 -
 gdb/gdbserver/remote-utils.c      |  1 -
 gdb/gdbserver/tracepoint.c        |  1 -
 gdb/m32r-tdep.c                   |  1 -
 gdb/mi/mi-main.c                  |  1 -
 gdb/or1k-tdep.c                   |  1 -
 gdb/printcmd.c                    |  1 -
 gdb/python/py-block.c             |  1 -
 gdb/regcache.c                    |  1 -
 gdb/remote.c                      |  1 -
 gdb/stubs/ChangeLog               |  4 ++++
 gdb/stubs/ia64vms-stub.c          |  1 -
 gdb/target.c                      |  1 -
 gdb/tui/tui-source.c              |  1 -
 gdb/ui-out.c                      |  1 -
 20 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index 8a3f65f088..0299f3dcdd 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -24,7 +24,6 @@
 #include "ui-out.h"
 #include "cli-out.h"
 #include "top.h"		/* for "execute_command" */
-#include "event-top.h"
 #include "infrun.h"
 #include "observable.h"
 #include "gdbthread.h"
diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index 910a874550..97c4d3c707 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -28,7 +28,6 @@
 #include "source.h"
 #include "cp-support.h"
 #include "cp-abi.h"
-#include "symtab.h"
 #include "objfiles.h"
 #include "block.h"
 #include "gdbcmd.h"
diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
index ef8f29c559..1259ccaeb1 100644
--- a/gdb/csky-tdep.c
+++ b/gdb/csky-tdep.c
@@ -52,10 +52,8 @@
 #include "dwarf2-frame.h"
 #include "user-regs.h"
 #include "valprint.h"
-#include "reggroups.h"
 #include "csky-tdep.h"
 #include "regset.h"
-#include "block.h"
 #include "opcode/csky.h"
 #include <algorithm>
 #include <vector>
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 99b0cc55ef..056d060046 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -72,7 +72,6 @@ static const char *xmltarget_amd64_linux_no_xml = "@<target>\
 
 #include <sys/reg.h>
 #include <sys/procfs.h>
-#include "nat/gdb_ptrace.h"
 #include <sys/uio.h>
 
 #ifndef PTRACE_GET_THREAD_AREA
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index ef1b0ede3a..e7a2170bbd 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -25,7 +25,6 @@
 #include "tdesc.h"
 #include "dll.h"
 #include "rsp-low.h"
-#include "gdbthread.h"
 #include "netstuff.h"
 #include "filestuff.h"
 #include <ctype.h>
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index c4af55749f..5fb8a5134b 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -7330,7 +7330,6 @@ gdb_agent_init (void)
 }
 
 #include <sys/mman.h>
-#include <fcntl.h>
 
 IP_AGENT_EXPORT_VAR char *gdb_tp_heap_buffer;
 IP_AGENT_EXPORT_VAR char *gdb_jump_pad_buffer;
diff --git a/gdb/m32r-tdep.c b/gdb/m32r-tdep.c
index 3f17a5e027..18acdb6990 100644
--- a/gdb/m32r-tdep.c
+++ b/gdb/m32r-tdep.c
@@ -35,7 +35,6 @@
 #include "regcache.h"
 #include "trad-frame.h"
 #include "dis-asm.h"
-#include "objfiles.h"
 #include "m32r-tdep.h"
 #include <algorithm>
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index dc96032b0d..7176963845 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -43,7 +43,6 @@
 #include "mi-common.h"
 #include "language.h"
 #include "valprint.h"
-#include "inferior.h"
 #include "osdata.h"
 #include "common/gdb_splay_tree.h"
 #include "tracepoint.h"
diff --git a/gdb/or1k-tdep.c b/gdb/or1k-tdep.c
index 9140ca2aab..13e130c6fd 100644
--- a/gdb/or1k-tdep.c
+++ b/gdb/or1k-tdep.c
@@ -33,7 +33,6 @@
 #include "block.h"
 #include "reggroups.h"
 #include "arch-utils.h"
-#include "frame.h"
 #include "frame-unwind.h"
 #include "frame-base.h"
 #include "dwarf2-frame.h"
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index e6fdbcf344..cd2e585235 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -50,7 +50,6 @@
 #include "format.h"
 #include "source.h"
 #include "common/byte-vector.h"
-#include "cli/cli-style.h"
 
 /* Last specified output format.  */
 
diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index c6e68a107e..90140ebc34 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -23,7 +23,6 @@
 #include "symtab.h"
 #include "python-internal.h"
 #include "objfiles.h"
-#include "symtab.h"
 
 typedef struct blpy_block_object {
   PyObject_HEAD
diff --git a/gdb/regcache.c b/gdb/regcache.c
index c51ef771be..4a68390c5f 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1419,7 +1419,6 @@ register_dump::dump (ui_file *file)
 #if GDB_SELF_TEST
 #include "selftest.h"
 #include "selftest-arch.h"
-#include "gdbthread.h"
 #include "target-float.h"
 
 namespace selftests {
diff --git a/gdb/remote.c b/gdb/remote.c
index 4e2c85a223..4b3f2907b4 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -28,7 +28,6 @@
 #include "symfile.h"
 #include "target.h"
 #include "process-stratum-target.h"
-/*#include "terminal.h" */
 #include "gdbcmd.h"
 #include "objfiles.h"
 #include "gdb-stabs.h"
diff --git a/gdb/stubs/ia64vms-stub.c b/gdb/stubs/ia64vms-stub.c
index 21119ec8ae..6e8ec4dee5 100644
--- a/gdb/stubs/ia64vms-stub.c
+++ b/gdb/stubs/ia64vms-stub.c
@@ -56,7 +56,6 @@
 #include <builtins.h>
 #include <prtdef.h>
 #include <psldef.h>
-#include <ssdef.h>
 #include <chfdef.h>
 
 #include <lib_c/imcbdef.h>
diff --git a/gdb/target.c b/gdb/target.c
index e66584f147..ad7eba3fa3 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -48,7 +48,6 @@
 #include <algorithm>
 #include "byte-vector.h"
 #include "terminal.h"
-#include <algorithm>
 #include <unordered_map>
 
 static void generic_tls_error (void) ATTRIBUTE_NORETURN;
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index ed9562a930..a7e801eba2 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -25,7 +25,6 @@
 #include "frame.h"
 #include "breakpoint.h"
 #include "source.h"
-#include "symtab.h"
 #include "objfiles.h"
 #include "filenames.h"
 #include "source-cache.h"
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 7bcc2638ae..6851fd29c6 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -28,7 +28,6 @@
 #include <vector>
 #include <memory>
 #include <string>
-#include <memory>
 
 namespace {
 
-- 
2.17.2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Remove duplicate or commented-out #includes
  2019-01-19 21:31 [PATCH] Remove duplicate or commented-out #includes Tom Tromey
@ 2019-01-21 17:28 ` Gary Benson
  2019-01-22  4:07   ` Tom Tromey
  2019-01-21 17:34 ` Simon Marchi
  1 sibling, 1 reply; 7+ messages in thread
From: Gary Benson @ 2019-01-21 17:28 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Tom Tromey wrote:
> I wrote a little script to detect duplicate or commented-out
> #includes and ran it on gdb.  This patch is the result.  Tested
> by rebuilding.

Looks good to me :)  I did something similar a few years back,
though possibly only for system headers, or for just the ones
in common-defs.h.  So obviously I approve!

> It would be possible to sort #includes, or maybe run
> include-what-you-use on gdb, but I haven't tried that.  I'd be
> interested to hear if you think this would be worthwhile, thoug.

I'd consider it worthwhile.  I'd really like sorted #includes.
Some files have random whitespace in the list too, that maybe
made sense once.

Cheers,
Gary

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Remove duplicate or commented-out #includes
  2019-01-19 21:31 [PATCH] Remove duplicate or commented-out #includes Tom Tromey
  2019-01-21 17:28 ` Gary Benson
@ 2019-01-21 17:34 ` Simon Marchi
  2019-01-21 18:28   ` Tom Tromey
  1 sibling, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2019-01-21 17:34 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2019-01-19 16:30, Tom Tromey wrote:
> I wrote a little script to detect duplicate or commented-out #includes
> and ran it on gdb.  This patch is the result.  Tested by rebuilding.

Nice, thanks, this LGTM.

> It would be possible to sort #includes, or maybe run
> include-what-you-use on gdb, but I haven't tried that.  I'd be
> interested to hear if you think this would be worthwhile, thoug.

I wasn't sure of what the advantage of using IWYU was, so I read this:

https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md

In the end, removing unnecessary includes would be nice.  If a .c file 
includes a header file unnecessarily, it will be rebuild for nothing 
when that header file is modified.  If we can avoid that, it means less 
wasted time for everybody.  Replacing includes by forward declarations 
is also nice to cut down on compilation time.  It would be really 
tedious to do those changes by hand, but if a tool can do it 
automatically, why not.

We have that policy where .h files must assume that defs.h has been 
included (providing common types) [1].  I am not sure how IWYU will cope 
with that.

[1] 
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Include_Files

> gdb/ChangeLog
> 2019-01-19  Tom Tromey  <tromey@bapiya>
> 
> 	* ui-out.c: Fix includes.
> 	* tui/tui-source.c: Fix includes.
> 	* target.c: Fix includes.
> 	* remote.c: Fix includes.
> 	* regcache.c: Fix includes.
> 	* python/py-block.c: Fix includes.
> 	* printcmd.c: Fix includes.
> 	* or1k-tdep.c: Fix includes.
> 	* mi/mi-main.c: Fix includes.
> 	* m32r-tdep.c: Fix includes.
> 	* csky-tdep.c: Fix includes.
> 	* compile/compile-cplus-types.c: Fix includes.
> 	* cli/cli-interp.c: Fix includes.
> 
> gdb/gdbserver/ChangeLog
> 2019-01-19  Tom Tromey  <tromey@bapiya>
> 
> 	* tracepoint.c: Fix includes.
> 	* remote-utils.c: Fix includes.
> 	* linux-x86-low.c: Fix includes.
> 
> gdb/stubs/ChangeLog
> 2019-01-19  Tom Tromey  <tromey@bapiya>
> 
> 	* ia64vms-stub.c: Fix includes.

Watch out for that email address in the ChangeLog entries.

Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Remove duplicate or commented-out #includes
  2019-01-21 17:34 ` Simon Marchi
@ 2019-01-21 18:28   ` Tom Tromey
  0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2019-01-21 18:28 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> 2019-01-19  Tom Tromey  <tromey@bapiya>
>> 
>> * ia64vms-stub.c: Fix includes.

Simon> Watch out for that email address in the ChangeLog entries.

Thanks, I've updated my rewriter scripts to pull the name & email from
git now.

Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Remove duplicate or commented-out #includes
  2019-01-21 17:28 ` Gary Benson
@ 2019-01-22  4:07   ` Tom Tromey
  2019-01-23  4:00     ` Tom Tromey
  2019-01-23 16:40     ` Pedro Alves
  0 siblings, 2 replies; 7+ messages in thread
From: Tom Tromey @ 2019-01-22  4:07 UTC (permalink / raw)
  To: Gary Benson; +Cc: Tom Tromey, gdb-patches

>> It would be possible to sort #includes, or maybe run
>> include-what-you-use on gdb, but I haven't tried that.  I'd be
>> interested to hear if you think this would be worthwhile, thoug.

Gary> I'd consider it worthwhile.  I'd really like sorted #includes.
Gary> Some files have random whitespace in the list too, that maybe
Gary> made sense once.

I wrote a script to sort the includes.  The output is rather voluminous,
not totally sure yet how I will submit it.

It doesn't try to do a 100% job.  For example, some files in gdb include
other files at random spots -- not all includes are at the top of the
file -- and the script doesn't try to handle this.

It did find 40 .h files that don't have include guards.  Maybe I will
try to automatically fix these.

Building revealed a few minor order dependencies in the headers.  I'll
submit this as an initial cleanup.


I chose to have the includes ordered this way:

First stanza:

1. defs.h (or server.h or common-defs.h)
2. for a .c file, the corresponding .h if it exists
   (two exceptions were needed to this rule)

Second stanza (stanzas separated by a blank line) holds system headers.

Third stanzas holds includes of headers in binutils-gdb but not part of
gdb proper.

Fourth stanza is gdb-specific headers.


It's reasonably easy to change this around though.


It would be possible to modify this same script to try removing includes
from .c files and seeing whether the file still compiles.  It would just
take a long time on this machine I have at the moment.

If anyone wants to try it out for themselves, I can send the
instructions.

Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Remove duplicate or commented-out #includes
  2019-01-22  4:07   ` Tom Tromey
@ 2019-01-23  4:00     ` Tom Tromey
  2019-01-23 16:40     ` Pedro Alves
  1 sibling, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2019-01-23  4:00 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Gary Benson, gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I wrote a script to sort the includes.  The output is rather voluminous,
Tom> not totally sure yet how I will submit it.

If anyone wants to look, it is on the branch submit/sort-includes in my
github.  The patch is pretty big... maybe I can gzip it.  Or maybe I can
submit it in pieces, since really each .c change is independent.

There's also a submit/common-includes branch, which normalizes gdb to
use #include "common/name.h", and removes the -Icommon from the
Makefiles.  I'll clean this one up and submit it soon.

Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Remove duplicate or commented-out #includes
  2019-01-22  4:07   ` Tom Tromey
  2019-01-23  4:00     ` Tom Tromey
@ 2019-01-23 16:40     ` Pedro Alves
  1 sibling, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2019-01-23 16:40 UTC (permalink / raw)
  To: Tom Tromey, Gary Benson; +Cc: gdb-patches

On 01/22/2019 04:07 AM, Tom Tromey wrote:
>>> It would be possible to sort #includes, or maybe run
>>> include-what-you-use on gdb, but I haven't tried that.  I'd be
>>> interested to hear if you think this would be worthwhile, thoug.
> 
> Gary> I'd consider it worthwhile.  I'd really like sorted #includes.
> Gary> Some files have random whitespace in the list too, that maybe
> Gary> made sense once.
> 
> I wrote a script to sort the includes.  The output is rather voluminous,
> not totally sure yet how I will submit it.
> 
> It doesn't try to do a 100% job.  For example, some files in gdb include
> other files at random spots -- not all includes are at the top of the
> file -- and the script doesn't try to handle this.
> 
> It did find 40 .h files that don't have include guards.  Maybe I will
> try to automatically fix these.
> 
> Building revealed a few minor order dependencies in the headers.  I'll
> submit this as an initial cleanup.
> 
> 
> I chose to have the includes ordered this way:
> 
> First stanza:
> 
> 1. defs.h (or server.h or common-defs.h)
> 2. for a .c file, the corresponding .h if it exists
>    (two exceptions were needed to this rule)
> 
> Second stanza (stanzas separated by a blank line) holds system headers.
> 
> Third stanzas holds includes of headers in binutils-gdb but not part of
> gdb proper.
> 
> Fourth stanza is gdb-specific headers.
> 
> 
> It's reasonably easy to change this around though.

That order seems fine to me.  Thanks a lot for doing that!  Looking forward.

Thanks,
Pedro Alves

> It would be possible to modify this same script to try removing includes
> from .c files and seeing whether the file still compiles.  It would just
> take a long time on this machine I have at the moment.
> 
> If anyone wants to try it out for themselves, I can send the
> instructions.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-23 16:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-19 21:31 [PATCH] Remove duplicate or commented-out #includes Tom Tromey
2019-01-21 17:28 ` Gary Benson
2019-01-22  4:07   ` Tom Tromey
2019-01-23  4:00     ` Tom Tromey
2019-01-23 16:40     ` Pedro Alves
2019-01-21 17:34 ` Simon Marchi
2019-01-21 18:28   ` Tom Tromey

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).