On 2026-07-04 14:29, Tom Lane wrote:
Fundamentally, what this statement wants to do is arithmetic on these constants, so how is it not treating them as integers at bottom?
Yes, the cast treats the enum (which is an integer) as an int (which is also an integer, though of a different integer type). And if the cast did only that it would be fine. But in C, casts can do more than change the type of a value: they can also change the the value itself (e.g., by yielding the low order bits of a pointer), which is definitely not wanted here. I'd rather not impose an extra burden on readers to verify that the cast is OK because the identifier is not a pointer and is not floating point and has a value in int range.
Of course sometimes a cast is needed for other reasons, and then C code should use a cast despite casts' hazards and hassles; but here a cast is not needed.
I'm not really in a position to test whether MSVC would be happy with this.
Perhaps someone else can try the two patches I mentioned earlier.Another possibility is that builders using MSVC could add /wd4146 and /wd5287 to the compiler flags. That should pacify MSVC without needing to change tzcode.
In the meantime there's no rush to apply those two patches, as zic's behavior is fine; this is merely about having compilers generate fewer bogus warnings. I do take the point, though, that the motivation for using unary "+" instead of a cast is not immediately obvious to all, so I installed the attached patch which I hope helps make this clearer.
From 4d341738164df7bab6cc41d38859205eb9e27a12 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Sat, 4 Jul 2026 18:27:49 -0700 Subject: [PROPOSED] =?UTF-8?q?Document=20why=20zic.c=20uses=20unary=20?= =?UTF-8?q?=E2=80=98+=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * private.h (INT_PROMOTE): New macro. * zic.c (MAX_FIELDS): Use it. --- private.h | 5 +++++ zic.c | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/private.h b/private.h index b0faaeb9..7918082d 100644 --- a/private.h +++ b/private.h @@ -954,6 +954,11 @@ ATTRIBUTE_POSIX2TIME time_t time2posix_z(timezone_t, time_t); #define TYPE_SIGNED(type) (((type) -1) < 0) #define TWOS_COMPLEMENT(type) (TYPE_SIGNED (type) && (! ~ (type) -1)) +/* Yield the value of the arithmetic expression X after integer promotion. + This is safer than a cast, which in general would accept even + pointers and which might trap or yield a value not equal to X. */ +#define INT_PROMOTE(x) (+(x)) + /* Minimum and maximum of two values. Use lower case to avoid naming clashes with standard include files. */ #define max(a, b) ((a) > (b) ? (a) : (b)) diff --git a/zic.c b/zic.c index af2bb80a..a2c2dec8 100644 --- a/zic.c +++ b/zic.c @@ -412,10 +412,10 @@ enum { }; /* The maximum number of fields on any of the above lines. - (The "+"s pacify gcc -Wenum-compare.) */ + (This uses INT_PROMOTE to pacify gcc -Wenum-compare.) */ enum { - MAX_FIELDS = max(max(+RULE_FIELDS, +LINK_FIELDS), - max(+LEAP_FIELDS, +EXPIRES_FIELDS)) + MAX_FIELDS = max(max(INT_PROMOTE(RULE_FIELDS), INT_PROMOTE(LINK_FIELDS)), + max(INT_PROMOTE(LEAP_FIELDS), INT_PROMOTE(EXPIRES_FIELDS))) }; /* -- 2.53.0
