Re: The defs.h / common-defs.h / server.h rule

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2024-02-20 09:52, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> We currently have a rule [1] saying that all .c/.cc file must include
> Simon> one of defs.h, common-defs.h or server.h, depending on where they are,
> Simon> and that no header should include them.
> 
> Simon> I was wondering about why that rule exists, and if it is still relevant
> Simon> today.
> 
> Every source file has to include config.h first.  In other GNU-ish
> projects this is done in a straightforward way, but gdb (like gcc) used
> a kind of prologue header file instead.

Ok, so through defs.h, every source file in gdb/ for instance ends up
including:

 - gdbsupport/config.h
 - gnulib/config.h
 - gdb/config.h

Plus, in gdbsupport/common-defs.h (or transitively included) are some
definitions that we want all source files to see all the time.  For
instance, the stuff in poison.h, or our override of ATTRIBUTE_PRINTF.

I suppose the other classic way to do this is with an -include flag,
passed to the compiler?  Actually that might be a better way to solve my
problem.  In order to correctly analyze some headers in a standalone
way, it is indeed necessary for clangd to include the various config.h
files first (and perhaps other stuff from defs.h & co).  If we used
-include to include whatever we want all code to see all the time, then
clangd would see the -include flag in the compilation flags and include
those "prologue" headers when analyzing any other header.

I gave it a try just for fun, it seems to work nicely:

  - include config.h files with -include
    https://review.lttng.org/c/binutils-gdb/+/11857/1

  - move more things to gdbsupport.inc.h
    https://review.lttng.org/c/binutils-gdb/+/11858/2

I like this solution because (a) it helps my problem and (b) it makes
defs.h / common-defs.h / server.h no longer special.  All things that we
want all source files to see all the time are included by that -include
flag, so enforced by the build system, and for the rest, source files /
header files include what they need (e.g. if they don't need defs.h,
they don't have to include it).

defs.h & co pull in a ton of stuff that is not needed everywhere, so I
think it would be nice to gradually remove some includes from them, and
include things where they are really needed.  This could help build
times and avoid recompiling files unnecessarily.

I'm not sure my argument for such a change is very strong, but I would
be happy to see a change like that.

> Simon> Could we switch to having headers include what they use?
> 
> Do you mean just adding defs.h to every header, or something else?

That's what I thought at first (either defs.h, or whatever include files
they use).  But it's not possible to include defs.h / common-defs.h /
server.h in the header files that defs.h / common-defs.h / server.h
themselves include.  So for those include files, I would be stuck.

> Simon> Before someome says "it's your tools' fault, your tools are bad": I
> Simon> know.
> 
> In addition to that, I wonder how to enforce this rule.
> For example I'm not likely to start using clangd.
> Or if clangd is changed, will the rule change?

I don't think what I propose is really clangd specific.  I think it's
just to design headers that can be analyzed in a self-contained way, to
the extent possible, which I think is just good software design.

Do people use other editors / IDEs that give accurate compilation errors
in real time like clangd does?  How do they do for header files that
assume they've been included in some context?

Simon