From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id cKZ5FUQdA2a9PRcAWB0awg (envelope-from ) for ; Tue, 26 Mar 2024 15:08:52 -0400 Received: by simark.ca (Postfix, from userid 112) id 557D71E0C0; Tue, 26 Mar 2024 15:08:52 -0400 (EDT) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 450781E08C for ; Tue, 26 Mar 2024 15:08:50 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 052DD3858432 for ; Tue, 26 Mar 2024 19:08:50 +0000 (GMT) Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id CA8B13858C60 for ; Tue, 26 Mar 2024 19:08:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CA8B13858C60 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=efficios.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=efficios.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CA8B13858C60 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711480092; cv=none; b=Y5dg566Tc7wWkdg4DoxdasEpgetqhtn5MtQL8Us1sXr/whADNg1SKuSUI0dyTXkCrUqScfN2U3/IokxFvZE0FSoRLt3igXGtcjDcx7M+BEUKXGjD3wOzX0gGY3Xqny93DKQAqDYbgNoiOwvPQrW9/IVX2sjxD0EZRdcAUH7rGBY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711480092; c=relaxed/simple; bh=0gj4k1iQ9oqIoFmtanjh9aymqzHawMgNReTk5NQFYyU=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=NVPe4WFTT8usb0UJmu7LjtmvoXTE2SbI539ibLGYjvbv5Z8s4A68KwCHZ/bxbOltMjvLWdmTr9tSkKdjEr2eWkpXDpBOOSPE3PG/NnRIksw4VW1ZGQcglBAhL20kVrEHypAYzrgnwFEpG9BjkZMNl1NJuxkMofeanR7LVexWcdw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from localhost.localdomain (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 289351E0C1; Tue, 26 Mar 2024 15:08:09 -0400 (EDT) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH v3 3/4] gdb, gdbserver, gdbsupport: include early header files with `-include` Date: Tue, 26 Mar 2024 15:06:45 -0400 Message-ID: <20240326190806.89541-4-simon.marchi@efficios.com> X-Mailer: git-send-email 2.44.0 In-Reply-To: <20240326190806.89541-1-simon.marchi@efficios.com> References: <20240326190806.89541-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1173.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org 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 --- 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 a5139ea75803..a9f641c06593 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 45073abca493..5180e7336cb4 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 7c360aa413ef..2b0f987125c1 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 ab35b9148a50..ee709112aaed 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) -- 2.44.0