Re: Reducing header interdependencies around heapam.h et al.

On 2019-Jan-14, Andres Freund wrote:

> I think the whole pointer hiding game that we play is a really really
> bad idea. We should just about *never* have typedefs that hide the fact
> that something is a pointer. But it's hard to go away from that for
> legacy reasons.
>
> The problem with your approach is that it's *eminently* reasonable to
> want to forward declare a struct in multiple places. Otherwise you end
> up in issues where you include headers like heapam.h solely for a
> typedef, which obviously doesn't make a ton of sense.

Well, my point is that in an ideal world we would have a header where
the struct is declared once in a very lean header, which doesn't include
almost anything else, so you can include it into other headers
liberally. Then the struct definitions are any another (heavy) header,
which *does* need to include lots of other stuff in order to be able to
define the structs fully, and would be #included very sparingly, only in
the few .c files that really needed it.

For example, I would split up execnodes.h so that *only* the
typedef/struct declarations are there, and *no* struct definition. Then
that header can be #included in other headers that need those to declare
functions -- no problem. Another header (say execstructs.h, whatever)
would contain the struct definition and would only be used by executor
.c files. So when a struct changes, only the executor is recompiled;
the rest of the source doesn't care, because execnodes.h (the struct
decls) hasn't changed.

This may be too disruptive. I'm not suggesting that you do things this
way, only describing my ideal world.

Your idea of "liberally forward-declaring structs in multiple places"
seems like you don't *have* the lean header at all (only the heavy one
with all the struct definitions), and instead you distribute bits and
pieces of the lean header randomly to the places that need it. It's
repetitive. It gets the job done, but it's not *clean*.

> Given the fact that including headers just for a typedef is frequent
> overkill, hiding the typedef in a separate header has basically no
> gain. I also don't quite understand why using a forward declaration with
> struct in the name is that confusing, especially when it only happens in
> the header.

Oh, that's not the confusing part -- that's just repetitive, nothing
more. What's confusing (IMO) is having two names for the same struct
(one pointer and one full struct). It'd be useful if it was used the
way I describe above. But that's the settled project style, so I don't
have any hopes that it'll ever be changed.

Anyway, I'm not objecting to your patch ... I just don't want it on
record that I'm in love with the current situation :-)

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services