Re: [PATCH 2/3] gdb, gdbserver, gdbsupport: include config.h files with -include

[Pedro Alves <pedro@palves.net>]

On 2024-03-18 20:01, Simon Marchi wrote:
> 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.  

I think you meant "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 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

This change is a bit ______? (fill in missing word).


> 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.

copiles -> compiles


Note there is a make rule to check whether gdb headers are standalone.  "make check-headers".
Unfortunately, nobody ever runs that ( me included, after adding it a decade ago :-P ).
Ideally, we'd fix all it flags and run it (or something like it) once in a while in a CI.
(And same for gdbserver/gdbsupport, of course.)


> 
> 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.

I'm surprised by the actual patch.  Why isn't this including defs.h/common-defs.h?  There are
surely things defined in those files that need to be defined in headers too.  Why create
this divergence?  I'd think that we would include defs.h/common-defs.h in headers too, and
then work on moving things out of defs.h/common-defs.h over time.

Pedro Alves