zend_compile: Bundle function type constants into an zend_function_type enum#21208
zend_compile: Bundle function type constants into an zend_function_type enum#21208TimWolla wants to merge 2 commits intophp:masterfrom
zend_function_type enum#21208Conversation
…ype` enum This clarifies the relationship between these constants and improves type safety a little.
TimWolla
force-pushed
the
zend-function-type
branch
from
a6c8db0 to
2fcddda
Compare
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
Looking forward to being able to use C23_ENUM more often! :)
| /* Define a fixed-size enum as enum C23_ENUM(name, size) { }. */ | ||
| #if __STDC_VERSION__ >= 202311L || defined(__cplusplus) | ||
| # define C23_ENUM(name, size) \ | ||
| name: size; \ |
There was a problem hiding this comment.
I don't comprehend this. Shouldn't this line be this:
| name: size; \ | |
| enum name: size; \ |
There was a problem hiding this comment.
Ah no, I get it now. You put enum at the use-site of C23_ENUM... I think it would be more logical to embed enum in this macro
There was a problem hiding this comment.
The intended usage is enum C23_ENUM(name, size), thus the leading enum keyword is provided “outside of the macro”. I wanted to keep it outside of the macro to make it easier to visually scan the file.
There was a problem hiding this comment.
I think putting it outside adds redundancy and confusion.
There was a problem hiding this comment.
I would have also preferred to have it in the macro, but don't care enough either way.
| enum name: size | ||
| #else | ||
| # define C23_ENUM(name, size) \ | ||
| name; \ |
There was a problem hiding this comment.
Likewise, is enum prefix missing here or what am I missing?
4 participants
This clarifies the relationship between these constants and improves type safety a little.