Thanks for your feedback! > On 18 Nov 2025, at 21:49, Tom Lane <[email protected]> wrote: > > Isn't this patch basically proposing to revert 10b7218? > How has the situation changed since then to make that > a good idea?
No, it is not a revert, until we don’t have a separated ENUMFOO_NUM value with numeric value as <last> + 1, this patch offers similar, but still slightly different way to define that value.. I was hoping that when adding new values, the developer would think "why there is a ENUMFOO_LAST here" and leave it the last value of the enum, allowing ENUMFOO_NUM to stay correct after adding new values. Just as it was explicitly stated before pg18 in comment in enum definition, but at the same time I wanted to get rid of the compiler warnings. > >> Please note, that this will not lead to extra compiler warnings about >> writing switch statements on enum values, >> because LAST value in enums is always equal to some other value of the enum. > > TBH I find that argument wildly optimistic. Even if it's true > today it could stop being true next week. > Hmm, I don’t see any reason, why compilers can add warnings about missing enum value, that int representation equals to some other enum's value. It would be an correct warning if you forget to write case for value, that is equals to ENUMFOO_LAST, or it would be a bug in compiler, if it offers you to write case statements that is already covered some other case statement. > PMSIGNAL_BACKGROUND_WORKER_CHANGE, /* background worker state change */ > PMSIGNAL_START_WALRECEIVER, /* start a walreceiver */ > PMSIGNAL_ADVANCE_STATE_MACHINE, /* advance postmaster's state machine */ > PMSIGNAL_XLOG_IS_SHUTDOWN, /* ShutdownXLOG() completed */ > + > +#define NUM_PMSIGNALS (PMSIGNAL_XLOG_IS_SHUTDOWN+1) > } PMSignalReason; > - > -#define NUM_PMSIGNALS (PMSIGNAL_XLOG_IS_SHUTDOWN+1) Anyway, I like your variant of defining ENUMFOO_NUM inside enum, because it allows to achieve (or at least try to (: ) goal of making developers think smth like "why there is that thing” and not to break ENUMFOO_NUM accidentally when they are adding new ENUMFOO_VALUE in the end of the enum. > (I'm not actually advocating such a patch, because I doubt it's > worth the trouble. But perhaps others will think it worthwhile.) Still, I made a patch v2, in case someone decides that it would be useful.
v2-0001-move-ENUMFOO_NUM-definitions-inside-enums.patch
Description: Binary data
