Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@efficios.com>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: [PATCH 2/3] gdb, gdbserver, gdbsupport: include config.h files with -include
Date: Mon, 18 Mar 2024 16:01:39 -0400	[thread overview]
Message-ID: <20240318200257.131199-2-simon.marchi@efficios.com> (raw)
In-Reply-To: <20240318200257.131199-1-simon.marchi@efficios.com>

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 source 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 but 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, 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 copiles, it's just a convenience thing.

This patch does the small step of moving the inclusion of the various
config.h files to that new method.  The changes are:

 - Add three files meant to be included with -include: gdb/gdb.inc.h,
   gdbserver/gdbserver.inc.h and gdbsupport/gdbsupport.inc.h.
 - Move the inclusion of the various config.h files there
 - Modify the compilation flags of all three subdirectories to add the
   appropriate -include option.

Change-Id: If3e345d00a9fc42336322f1d8286687d22134340
---
 gdb/Makefile.in             |  1 +
 gdb/defs.h                  |  7 -------
 gdb/gdb.inc.h               | 22 ++++++++++++++++++++++
 gdbserver/Makefile.in       |  3 ++-
 gdbserver/gdbserver.inc.h   | 22 ++++++++++++++++++++++
 gdbserver/server.h          |  8 --------
 gdbsupport/Makefile.am      |  1 +
 gdbsupport/Makefile.in      | 16 ++++++++++++----
 gdbsupport/common-defs.h    | 10 ----------
 gdbsupport/gdbsupport.inc.h | 35 +++++++++++++++++++++++++++++++++++
 10 files changed, 95 insertions(+), 30 deletions(-)
 create mode 100644 gdb/gdb.inc.h
 create mode 100644 gdbserver/gdbserver.inc.h
 create mode 100644 gdbsupport/gdbsupport.inc.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 95709ae395a2..3b144fa3034c 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -605,6 +605,7 @@ GDB_CFLAGS = \
 	-I. \
 	-I$(srcdir) \
 	-I$(srcdir)/config \
+	-include $(srcdir)/gdb.inc.h \
 	-DLOCALEDIR="\"$(localedir)\"" \
 	$(DEFS)
 
diff --git a/gdb/defs.h b/gdb/defs.h
index cf471bf5d662..8a9ff3aba0f7 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -25,13 +25,6 @@
 
 #include "gdbsupport/common-defs.h"
 
-#undef PACKAGE
-#undef PACKAGE_NAME
-#undef PACKAGE_VERSION
-#undef PACKAGE_STRING
-#undef PACKAGE_TARNAME
-
-#include <config.h>
 #include "bfd.h"
 
 #include <sys/types.h>
diff --git a/gdb/gdb.inc.h b/gdb/gdb.inc.h
new file mode 100644
index 000000000000..7be4c8eea939
--- /dev/null
+++ b/gdb/gdb.inc.h
@@ -0,0 +1,22 @@
+/* Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This file in included automatically via `-include` at the beginning of each
+   source file compiled in gdb/.  */
+
+#include "gdbsupport/gdbsupport.inc.h"
+#include "config.h"
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index c17a0a522df2..c92b881650a4 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -134,7 +134,8 @@ INCLUDE_CFLAGS = \
 	-I$(srcdir)/../gdb \
 	$(INCGNU) \
 	$(INCSUPPORT) \
-	$(INTL_CFLAGS)
+	$(INTL_CFLAGS) \
+	-include $(srcdir)/gdbserver.inc.h
 
 # M{H,T}_CFLAGS, if defined, has host- and target-dependent CFLAGS
 # from the config/ directory.
diff --git a/gdbserver/gdbserver.inc.h b/gdbserver/gdbserver.inc.h
new file mode 100644
index 000000000000..22ec0dd94318
--- /dev/null
+++ b/gdbserver/gdbserver.inc.h
@@ -0,0 +1,22 @@
+/* Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This file in included automatically via `-include` at the beginning of each
+   source file compiled in gdbserver/.  */
+
+#include "gdbsupport/gdbsupport.inc.h"
+#include "config.h"
diff --git a/gdbserver/server.h b/gdbserver/server.h
index 0074818c6df0..ee27d2b3f5c2 100644
--- a/gdbserver/server.h
+++ b/gdbserver/server.h
@@ -21,14 +21,6 @@
 
 #include "gdbsupport/common-defs.h"
 
-#undef PACKAGE
-#undef PACKAGE_NAME
-#undef PACKAGE_VERSION
-#undef PACKAGE_STRING
-#undef PACKAGE_TARNAME
-
-#include <config.h>
-
 static_assert (sizeof (CORE_ADDR) >= sizeof (void *));
 
 #include "gdbsupport/version.h"
diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am
index 7c360aa413ef..db1eee88059a 100644
--- a/gdbsupport/Makefile.am
+++ b/gdbsupport/Makefile.am
@@ -35,6 +35,7 @@ AM_CPPFLAGS = \
 	$(INCINTL) \
 	-I../bfd \
 	-I$(srcdir)/../bfd \
+	-include $(srcdir)/gdbsupport.inc.h \
 	@LARGEFILE_CPPFLAGS@
 
 override CXX += $(CXX_DIALECT)
diff --git a/gdbsupport/Makefile.in b/gdbsupport/Makefile.in
index 6f8dacc157f9..9b1063f31ab3 100644
--- a/gdbsupport/Makefile.in
+++ b/gdbsupport/Makefile.in
@@ -393,10 +393,18 @@ ACLOCAL_AMFLAGS = -I . -I ../config
 # from Automake, as gdbsupport uses AM_GNU_GETTEXT through
 # ZW_GNU_GETTEXT_SISTER_DIR, but doesn't have any translations (currently).
 SUBDIRS = 
-AM_CPPFLAGS = -I$(srcdir)/../include -I$(srcdir)/../gdb \
-    -I../gnulib/import -I$(srcdir)/../gnulib/import \
-    -I.. -I$(srcdir)/.. $(INCINTL) -I../bfd -I$(srcdir)/../bfd \
-    @LARGEFILE_CPPFLAGS@
+AM_CPPFLAGS = \
+	-I$(srcdir)/../include \
+	-I$(srcdir)/../gdb \
+	-I../gnulib/import \
+	-I$(srcdir)/../gnulib/import \
+	-I.. \
+	-I$(srcdir)/.. \
+	$(INCINTL) \
+	-I../bfd \
+	-I$(srcdir)/../bfd \
+	-include $(srcdir)/gdbsupport.inc.h \
+	@LARGEFILE_CPPFLAGS@
 
 AM_CXXFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS)
 noinst_LIBRARIES = libgdbsupport.a
diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
index 6120719480b8..e7eb814f3251 100644
--- a/gdbsupport/common-defs.h
+++ b/gdbsupport/common-defs.h
@@ -20,16 +20,6 @@
 #ifndef COMMON_COMMON_DEFS_H
 #define COMMON_COMMON_DEFS_H
 
-#include <gdbsupport/config.h>
-
-#undef PACKAGE_NAME
-#undef PACKAGE
-#undef PACKAGE_VERSION
-#undef PACKAGE_STRING
-#undef PACKAGE_TARNAME
-
-#include "gnulib/config.h"
-
 /* From:
     https://www.gnu.org/software/gnulib/manual/html_node/stdint_002eh.html
 
diff --git a/gdbsupport/gdbsupport.inc.h b/gdbsupport/gdbsupport.inc.h
new file mode 100644
index 000000000000..ab0999579528
--- /dev/null
+++ b/gdbsupport/gdbsupport.inc.h
@@ -0,0 +1,35 @@
+/* Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This file in included automatically via `-include` at the beginning of each
+   source file compiled in gdbsupport/.  */
+
+#include "gdbsupport/config.h"
+
+#undef PACKAGE
+#undef PACKAGE_NAME
+#undef PACKAGE_STRING
+#undef PACKAGE_TARNAME
+#undef PACKAGE_VERSION
+
+#include "gnulib/config.h"
+
+#undef PACKAGE
+#undef PACKAGE_NAME
+#undef PACKAGE_STRING
+#undef PACKAGE_TARNAME
+#undef PACKAGE_VERSION
-- 
2.44.0


  reply	other threads:[~2024-03-18 20:03 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-18 20:01 [PATCH 1/3] gdb, gdbserver, gdbsupport: reformat some Makefile variables, one entry per line Simon Marchi
2024-03-18 20:01 ` Simon Marchi [this message]
2024-03-19 11:18   ` [PATCH 2/3] gdb, gdbserver, gdbsupport: include config.h files with -include Hannes Domani
2024-03-19 12:22     ` Simon Marchi
2024-03-20 20:32   ` Pedro Alves
2024-03-21  2:11     ` Simon Marchi
2024-03-21 12:50       ` Pedro Alves
2024-03-21 13:02         ` Pedro Alves
2024-03-22 14:55         ` Simon Marchi
2024-03-22 15:08           ` Simon Marchi
2024-03-22 15:43             ` Pedro Alves
2024-03-18 20:01 ` [PATCH 3/3] gdbsupport: move more things to gdbsupport.inc.h Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240318200257.131199-2-simon.marchi@efficios.com \
    --to=simon.marchi@efficios.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox