Zend/zend_variables: use C99 designated initializers

[MaxKellermann]

No description provided.

[devnexen]

well, if we are to use this C99 feature, would assignments such as
[IS_UNDEF...IS_DOUBLE] = work ?
otherwise, while the change itself is correct, as is I feel it does not real bring something to the table. What do you think ?

[nielsdos]

Isn't that range thing a GNU extension?

[devnexen]

that s possible, maybe MSVC does not support it (would not surprise me).

[devnexen]

I ll let @Girgias decide if it s worth merging.

[MaxKellermann]

Isn't that range thing a GNU extension?

It is. https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
"To initialize a range of elements to the same value, write ‘[first ... last] = value’. This is a GNU extension."

It's tempting, but in this case it's not worth deviating from the standard.

[MaxKellermann]

I feel it does not real bring something to the table

It makes the code more robust; for example, it obsoletes this comment:

php-src/Zend/zend_types.h

Line 539 in f4313a8

/* Regular data types: Must be in sync with zend_variables.c. */

Robustness is king.

This way, we can later turn these IS_ macros into an enum with automatic values. Fewer hard-coded values, obvious correctness, less clutter.

[devnexen]

Ok if your plan later on is to make them enums then it makes more sense.

[MaxKellermann]

Ok if your plan later on is to make them enums then it makes more sense.

Already submitted, see PR #10657 specifically this commit dfd98be but I didn't yet remove the hard-coded values because this PR is pending. I can do that after this PR is merged.

[MaxKellermann]

Note that we can't make this a "named" enum just yet because PHP stores type codes in zend_uchar (or uint8_t after PR #10621), but C allows specifying an underlying enum type only with C23 (see https://en.cppreference.com/w/c/language/enum); C++ has this feature since C++11 already.

[devnexen]

Note that we can't make this a "named" enum just yet because PHP stores type codes in zend_uchar (or uint8_t after PR #10621), but C allows specifying an underlying enum type only with C23 (see https://en.cppreference.com/w/c/language/enum); C++ has this feature since C++11 already.

Interesting ; I knew for C++, not for C... finally I would say.

[Girgias]

Refering to the comment in the definition of zend_type:

typedef struct {
	/* Not using a union here, because there's no good way to initialize them
	 * in a way that is supported in both C and C++ (designated initializers
	 * are only supported since C++20). */
	void *ptr;
	uint32_t type_mask;
	/* TODO: We could use the extra 32-bit of padding on 64-bit systems. */
} zend_type;

both C and C++ (designated initializers are only supported since C++20)

Wouldn't this change cause issues for extensions that are written in C++?

[Girgias]

Refering to the comment in the definition of zend_type:

typedef struct {
	/* Not using a union here, because there's no good way to initialize them
	 * in a way that is supported in both C and C++ (designated initializers
	 * are only supported since C++20). */
	void *ptr;
	uint32_t type_mask;
	/* TODO: We could use the extra 32-bit of padding on 64-bit systems. */
} zend_type;

both C and C++ (designated initializers are only supported since C++20)

Wouldn't this change cause issues for extensions that are written in C++?

Oh wait, didn't look at the code prior to posting the comment, because this is affecting a static const array in a .c file this doesn't interface with any other files so the C++ concern is non-existent. Looks sensible to me in this case (and I now need to read up on designated initializers)

[MaxKellermann]

Even if this were in a header - I don't think C++ code would be a problem - older GCC versions that don't know C++20 yet do already support designated initializers as GNU extension (as part of C99 support). I suppose the same is true for MSVC (but I don't know).

What's rather interesting about this code comment is that it was added by ac4e0f0 more than 3 years ago, long before PHP switched to C99. In C89, designated initializers did not exist. If ever PHP used them before switching to C99, PHP was already using GNU extensions and beyond the C specification.

[Girgias]

more than 3 years ago, long before PHP switched to C99

PHP switched to C99 with PHP 8.0, which is at that time, so I don't understand how you got to that statement, I committed 2d0b0d6 and 67f8557 around the same time.

[MaxKellermann]

PHP switched to C99 with PHP 8.0

8.0 wasn't released until a year after that, that's what "long before" meant. But apparently the decision to go C99 was indeed around that time (a year before), so the comment is not that interesting, after all!

C11 would be interesting, though, because it has atomics.

[devnexen]

C11 would be interesting, though, because it has atomics.

Definitively is, we can review this sometime in PHP 9.x time.