[binutils-gdb] gdb, gdbserver, gdbsupport: include early header files with `-include`

[Simon Marchi <simark@sourceware.org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ab7daea3ad0d9a553ac0e0d37898bdda45ad2c37

commit ab7daea3ad0d9a553ac0e0d37898bdda45ad2c37
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Tue Mar 26 15:06:45 2024 -0400

    gdb, gdbserver, gdbsupport: include early header files with `-include`
    
    The motivation for this change is for analysis tools and IDEs to be
    better at analyzing header files on their own.
    
    There are some definitions and includes we want to occur at the very
    beginning of all translation units.  The way we currently do that is by
    requiring all source files (.c and .cc files) to include one of defs.h
    (for gdb), server.h (for gdbserver) of common-defs.h (for gdbsupport and
    shared source files).  These special header files define and include
    everything that needs to be included at the very beginning.  Other
    header files are written in a way that assume that these special
    "prologue" header files have already been included.
    
    My problem with that is that my editor (clangd-based) provides a very
    bad experience when editing header files.  Since clangd doesn't know
    that one of defs.h/server.h/common-defs.h was included already, a lot of
    things are flagged as errors.  For instance, CORE_ADDR is not known.
    It's possible to edit the files in this state, but a lot of the power of
    the editor is unavailable.
    
    My proposal to help with this is to include those things we always want
    to be there using the compilers' `-include` option.  Tom Tromey said
    that the current approach might exist because not all compilers used to
    have an option like this.  But I believe that it's safe to assume they
    do today.
    
    With this change, clangd picks up the -include option from the compile
    command, and is able to analyze the header file correctly, as it sees
    all that stuff included or defined by that -include option.  That works
    because when editing a header file, clangd tries to get the compilation
    flags from a source file that includes said header file.
    
    This change is a bit self-serving, because it addresses one of my
    frustrations when editing header files, but it might help others too.
    I'd be curious to know if others encounter the same kinds of problems
    when editing header files.  Also, even if the change is not necessary by
    any means, I think the solution of using -include for stuff we always
    want to be there is more elegant than the current solution.
    
    Even with this -include flag, many header files currently don't include
    what they use, but rather depend on files included before them.  This
    will still cause errors when editing them, but it should be easily
    fixable by adding the appropriate include.  There's no rush to do so, as
    long as the code still compiles, it's just a convenience thing.
    
    The changes are:
    
     - Add the appropriate `-include` option to the various Makefiles.
    
     - There is one particularity for gdbserver's Makefile: we do not want
       to include server.h when building `gdbreplay.o`, as `gdbreplay.cc`
       doesn't include it.  So we can't simply put the `-include` in
       `INTERNAL_CFLAGS`.  Add the `-include server.h` option to the
       `COMPILE` and `IPAGENT_COMPILE` variables, and added a special rule
       to compile `gdbreplay.o` with `-include gdbsupport/common-defs.h`.
    
     - Remove the `-include` option from the `check-headers` rule in
       gdb/Makefile.in, since it is already included in `INTERNAL_CFLAGS`.
    
    Change-Id: If3e345d00a9fc42336322f1d8286687d22134340
    Approved-By: Pedro Alves <pedro@palves.net>

Diff:
---
 gdb/Makefile.in        |  3 ++-
 gdbserver/Makefile.in  | 14 ++++++++++++--
 gdbsupport/Makefile.am |  1 +
 gdbsupport/Makefile.in |  1 +
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a5139ea7580..a9f641c0659 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -607,6 +607,7 @@ GDB_CFLAGS = \
 	-I. \
 	-I$(srcdir) \
 	-I$(srcdir)/config \
+	-include $(srcdir)/defs.h \
 	-DLOCALEDIR="\"$(localedir)\"" \
 	$(DEFS)
 
@@ -2050,7 +2051,7 @@ check-headers:
 	@echo Checking headers.
 	for i in $(CHECK_HEADERS) ; do \
 		$(CXX) $(CXX_DIALECT) -x c++-header -c -fsyntax-only \
-		$(INTERNAL_CFLAGS) $(CXXFLAGS) -include defs.h $(srcdir)/$$i ; \
+		$(INTERNAL_CFLAGS) $(CXXFLAGS) $(srcdir)/$$i ; \
 	done
 .PHONY: check-headers
 
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index 45073abca49..5180e7336cb 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -69,9 +69,12 @@ COMPILE.pre = $(CXX) $(CXX_DIALECT)
 COMPILE.post = -c -o $@
 POSTCOMPILE = @true
 
+INCLUDE_SERVER_H = -include $(srcdir)/server.h
+
 # CXXFLAGS is at the very end on purpose, so that user-supplied flags can
 # override internal flags.
-COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(CXXFLAGS) $(COMPILE.post)
+COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(INCLUDE_SERVER_H) \
+	$(CXXFLAGS) $(COMPILE.post)
 
 # It is also possible that you will need to add -I/usr/include/sys to the
 # CFLAGS section if your system doesn't have fcntl.h in /usr/include (which
@@ -509,7 +512,8 @@ IPAGENT_CFLAGS = \
 
 # CXXFLAGS is at the very end on purpose, so that user-supplied flags can
 # override internal flags.
-IPAGENT_COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(IPAGENT_CFLAGS) $(CXXFLAGS) $(COMPILE.post)
+IPAGENT_COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(IPAGENT_CFLAGS) \
+	$(INCLUDE_SERVER_H) $(CXXFLAGS) $(COMPILE.post)
 
 # Rules for special cases.
 
@@ -589,6 +593,12 @@ target/%.o: ../gdb/target/%.c
 %-generated.cc: ../gdb/regformats/rs6000/%.dat $(regdat_sh)
 	$(ECHO_REGDAT) $(SHELL) $(regdat_sh) $< $@
 
+# Rule for gdbreplay.o.  This is the same as COMPILE, but includes common-defs.h
+# instead of server.h.
+gdbreplay.o: gdbreplay.cc
+	$(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(CXXFLAGS) \
+		-include gdbsupport/common-defs.h $(COMPILE.post) $<
+
 #
 # Dependency tracking.
 #
diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am
index 7c360aa413e..2b0f987125c 100644
--- a/gdbsupport/Makefile.am
+++ b/gdbsupport/Makefile.am
@@ -35,6 +35,7 @@ AM_CPPFLAGS = \
 	$(INCINTL) \
 	-I../bfd \
 	-I$(srcdir)/../bfd \
+	-include $(srcdir)/common-defs.h \
 	@LARGEFILE_CPPFLAGS@
 
 override CXX += $(CXX_DIALECT)
diff --git a/gdbsupport/Makefile.in b/gdbsupport/Makefile.in
index ab35b9148a5..ee709112aae 100644
--- a/gdbsupport/Makefile.in
+++ b/gdbsupport/Makefile.in
@@ -403,6 +403,7 @@ AM_CPPFLAGS = \
 	$(INCINTL) \
 	-I../bfd \
 	-I$(srcdir)/../bfd \
+	-include $(srcdir)/common-defs.h \
 	@LARGEFILE_CPPFLAGS@
 
 AM_CXXFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS)