Re: *_LAST in enums to define NUM* macross

Roman Khapov <rkhapov(at)yandex-team(dot)ru> writes:
> Based on commit 10b7218, I extended this pattern by adding *_LAST elements and updating NUM-like macros in other
> modified enums within this patch (see attachment).

Isn't this patch basically proposing to revert 10b7218?
How has the situation changed since then to make that
a good idea?

> 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.

I don't really like the FOOENUM_LAST style at all. It's ugly, it
makes a dubious assumption about compiler behavior, and it doesn't
look even one bit safer than the NUM_FOOENUM style. Either way,
when adding a new enum value there is another place that you have
to remember to update.

Admittedly, with FOOENUM_LAST the "other place" is one line closer
than with a separate NUM_FOOENUM macro, and we have seen repeatedly
that people don't read code more than two lines away from what they
are patching :-(. But we could narrow that gap without making any
new assumptions, like this:

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)

(I'm not actually advocating such a patch, because I doubt it's
worth the trouble. But perhaps others will think it worthwhile.)

regards, tom lane