Hi, On 2019-07-31 16:55:31 -0400, Tom Lane wrote: > Alvaro Herrera <alvhe...@2ndquadrant.com> writes: > > On 2019-Jul-31, Andres Freund wrote: > >> * I think a lot of the interlinking stems from the bad idea to use > >> typedef's everywhere. In contrast to structs they cannot be forward > >> declared portably in our version of C. We should use a lot more struct > >> forward declarations, and just not use the typedef. > > > I don't know about that ... I think the problem is that we both declare > > the typedef *and* define the struct in the same place. If we were to > > split those things to separate files, the required rebuilds would be > > much less, I think, because changing a struct would no longer require > > recompiles of files that merely pass those structs around (that's very > > common for Node-derived structs). Forward-declaring structs in > > unrelated header files just because they need them, feels a bit like > > cheating to me. > > Yeah. I seem to recall a proposal that nodes.h should contain > > typedef struct Foo Foo; > > for every node type Foo, and then the other headers would just > fill in the structs, and we could get rid of a lot of ad-hoc > forward struct declarations and other hackery.
That to me just seems objectively worse. Now adding a new struct as a minor implementation detail of some subsystem doesn't just require recompiling the relevant files, but just about all of pg. And just about every header would acquire a nodes.h include - there's still a lot of them that today don't. I don't understand why you guys consider forward declaring structs ugly. It's what just about every other C project does. The only reason it's sometimes problematic in postgres is that we "hide the pointer" within some typedefs, making it not as obvious which type we're referring to (because the struct usage will be struct FooData*, instead of just Foo). But we also have confusion due to that in a lot of other places, so I don't really buy that this is a significant issue. Right now we really have weird dependencies between largely independent subsystem. Some are partially because functions aren't always in the right file, but it's also not often clear what the right one would be. E.g. snapmgr.h depending on relcache.h (for TransactionIdLimitedForOldSnapshots() having a Relation parameter), on resowner.h (for RegisterSnapshotOnOwner()) is imo not good. For one they lead to a number of .c files that actually use functionality from resowner.h to not have the necessary includes. There's a lot of things like that. .oO(the fmgr.h include in snapmgr.h has been unnecessary since 352a24a1f9) We could of course be super rigorous and have an accompanying foo_structs.h or something for every foo.h. But that seems to add no actual advantages, and makes things more complicated. The only reason the explicit forward declaration is needed in the common case of a 'struct foo*' parameter is that C has weird visibility rules about the scope of forward declarations in paramters. If you instead first have e.g. a function *return* type of that struct type, the explicit forward declaration isn't even needed - it's visible afterwards. But for parameters it's basically a *different* struct, that cannot be referenced again. Note that in C++ the visibility routines are more consistent, and you don't need an explicit forward declaration in either case (I'd also be OK with requiring it in both cases, it's just weird to only need them in one). Greetings, Andres Freund
