diff mbox

Remove duplicate or commented-out #includes

Message ID 20190119213007.23712-1-tom@tromey.com
State New
Headers show

Commit Message

Tom Tromey Jan. 19, 2019, 9:30 p.m. UTC
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(-)

Comments

Gary Benson Jan. 21, 2019, 5:28 p.m. UTC | #1
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
Simon Marchi Jan. 21, 2019, 5:33 p.m. UTC | #2
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
Tom Tromey Jan. 21, 2019, 6:27 p.m. UTC | #3
>>>>> "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
Tom Tromey Jan. 22, 2019, 4:07 a.m. UTC | #4
>> 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
Tom Tromey Jan. 23, 2019, 4 a.m. UTC | #5
>>>>> "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
Pedro Alves Jan. 23, 2019, 4:40 p.m. UTC | #6
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.
diff mbox

Patch

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 {