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
