Remove duplicate or commented-out #includes
Commit Message
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
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
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
>>>>> "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
>> 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" == 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
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.
@@ -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"
@@ -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"
@@ -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>
@@ -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
@@ -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>
@@ -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;
@@ -35,7 +35,6 @@
#include "regcache.h"
#include "trad-frame.h"
#include "dis-asm.h"
-#include "objfiles.h"
#include "m32r-tdep.h"
#include <algorithm>
@@ -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"
@@ -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"
@@ -50,7 +50,6 @@
#include "format.h"
#include "source.h"
#include "common/byte-vector.h"
-#include "cli/cli-style.h"
/* Last specified output format. */
@@ -23,7 +23,6 @@
#include "symtab.h"
#include "python-internal.h"
#include "objfiles.h"
-#include "symtab.h"
typedef struct blpy_block_object {
PyObject_HEAD
@@ -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 {
@@ -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"
@@ -56,7 +56,6 @@
#include <builtins.h>
#include <prtdef.h>
#include <psldef.h>
-#include <ssdef.h>
#include <chfdef.h>
#include <lib_c/imcbdef.h>
@@ -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;
@@ -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"
@@ -28,7 +28,6 @@
#include <vector>
#include <memory>
#include <string>
-#include <memory>
namespace {