On Wed, 25 Oct 2023 at 13:52, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Looking at for_each_ptr() I think it may be cleaner to follow
> palloc_object()'s precedent and make it foreach_object() instead (I have
> no love for the extra underscore, but I won't object to it either). And
> like foreach_node, have it receive a type name to add a cast to.
>
> I'd imagine something like
>
> SubscriptionRelState *rstate;
>
> foreach_object(SubscriptionRelState *, rstate, table_states_not_ready)
> {
Could you clarify why you think it may be cleaner? I don't see much
benefit to passing the type in there if all we use it for is adding a
cast. It seems like extra things to type for little benefit.
palloc_object uses the passed in type to not only do the cast, but
also to determine the size of the the allocation.
If foreach_object would allow us to remove the declaration further up
in the function I do see a benefit though.
I attached a new patchset which includes a 3rd patch that does this
(the other patches are equivalent to v4). I quite like that it moves
the type declaration to the loop itself, limiting its scope. But I'm
not fully convinced it's worth the hackiness of introducing a second
for loop that does a single iteration, just to be able to declare a
variable of a different type though. But I don't know another way of
achieving this. If this hack/trick is deemed acceptable, we can do the
same for the other newly introduced macros. The type would not even
need to be specified for oid/xid/int because it's already known to be
Oid/TransactionId/int respectively.