# Change in type-hint representation - Thread: https://externals.io/message/97674 - Original archive: https://news-web.php.net/php.internals/97674 - Started by Dmitry Stogov on 2017-01-11T12:07:39+00:00 - Messages: 22 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #97674 — Dmitry Stogov — 2017-01-11T12:07:39+00:00 URL: https://externals.io/message/97674#97674 Hi, I propose to introduce a unified type representation (zend_type). Now it's going to be used for typing of arguments and return values. Later we should use it for properties and other things. https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 The main changes are in zend_types.h and zend_compile.h, the rest is just an adoption for new type representation. I don't think we need RFC, because this is just an internal change that doesn't change behavior. I got the idea working on typed properties together with Bob and Joe. https://github.com/php/php-src/compare/master...bwoebi:typed_ref_properties I think it would be better to introduce zend_type and then continue work on typed properties. Any comments? Thanks. Dmitry. --- ## #97675 — Nikita Nefedov — 2017-01-11T13:35:52+00:00 URL: https://externals.io/message/97674#97675 · In reply to: #97674 > Hi, > > > I propose to introduce a unified type representation (zend_type). > > Now it's going to be used for typing of arguments and return values. > > Later we should use it for properties and other things. > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 > > > The main changes are in zend_types.h and zend_compile.h, the rest is > just an adoption for new type representation. > > I don't think we need RFC, because this is just an internal change that > doesn't change behavior. > > > I got the idea working on typed properties together with Bob and Joe. > > https://github.com/php/php-src/compare/master...bwoebi:typed_ref_properties > > I think it would be better to introduce zend_type and then continue work > on typed properties. > > > Any comments? > > > Thanks. Dmitry. > > Hey Dmitry, Having worked on callable prototypes I'd say unifying PHP types in Zend is something we urgently need for PHP to continue evolving. I'm not sure if PHP have ever been compatible with less-than-32bit archs but if it was I think it should be said that this would break such compatibility though. It would be great if there were some comment in the code near zend_type declaration where you'd explain how it is used and how additional data is being added to the pointer. Is there any use of ZEND_TYPE_CE() macro? It seems to be forgotten there? If I understood this correctly, the layout of zend_type is as follows: [xxxx xxxx xxxx xxxx] xxxx xxxx xxxx xxy0 - for IS_OBJECT type hint where the `xxxx`s are a (zend_string *) pointer and `y` designates an allow_null flag [xxxx xxxx xxxx xxxx] xxxx xxxx xxxx xxy1 - for all other type hints where the `xxxx`s are a IS_CALLABLE, _IS_BOOL, IS_LONG, IS_DOUBLE, IS_STRING or IS_ARRAY Do we decide here that IS_REF modifier should belong to the concrete usages of the type (e.g. referentiality is a property of a variable and not of a type)? I'm not sure this if is a right decision or not but I feel like this question should be raised. It is usually the opposite in other languages. How would you plan to extend this further? Let's say at some point we will have callable prototypes or generic classes: we will need to encode somehow this type into zend_type: `callable(A)` or `A`. Even right now it might be useful (as you suggest with ZEND_TYPE_CE) to store a (zend_class_entry *) instead of (zend_string *) when it is known to us in the zend_type. Seems like without extending zend_type to the size of two pointers it almost isn't doable :\ Or, it could be made that zend_type, when it's not a simple type hint, would point to the `zend_type_complex` which would store a zend_class_entry pointer (or not, if it's for callable) and an array of type specifiers. But that introduces another indirection. Anyway thanks for polishing this part, we definitely need zend_type in some form. --- ## #97677 — michal@brzuchalski.com — 2017-01-11T13:51:22+00:00 URL: https://externals.io/message/97674#97677 · In reply to: #97675 2017-01-11 14:35 GMT+01:00 Nikita Nefedov : > > > Hi, > > […] > Hey Dmitry, > > Having worked on callable prototypes I'd say unifying PHP types in Zend > is something we urgently need for PHP to continue evolving. > > I'm not sure if PHP have ever been compatible with less-than-32bit > archs but if it was I think it should be said that this would break > such compatibility though. > > It would be great if there were some comment in the code near zend_type > declaration where you'd explain how it is used and how additional > data is being added to the pointer. > > Is there any use of ZEND_TYPE_CE() macro? It seems to be forgotten there? > > If I understood this correctly, the layout of zend_type is as follows: > > [xxxx xxxx xxxx xxxx] xxxx xxxx xxxx xxy0 - for IS_OBJECT type hint > where the `xxxx`s are a (zend_string *) pointer and `y` designates > an allow_null flag > > I've got prepared Object Typehint RFC https://wiki.php.net/rfc/object-typehint where IS_OBJECT is used without class name as type hint for any object kind, if this patch would be applied how can I deal with this new zend_type? As far as I undestand last 0 for IS_OBJECT and no (zend_string *) pointer would give me empty zend_string value right? So that won't bive me any chances to store IS_OBJECT without classname am I right? > [xxxx xxxx xxxx xxxx] xxxx xxxx xxxx xxy1 - for all other type hints > where the `xxxx`s are a IS_CALLABLE, _IS_BOOL, IS_LONG, IS_DOUBLE, > IS_STRING or IS_ARRAY > > Do we decide here that IS_REF modifier should belong to the concrete > usages of the type (e.g. referentiality is a property of a variable > and not of a type)? > I'm not sure this if is a right decision or not but I feel like this > question should be raised. It is usually the opposite in other languages. > > How would you plan to extend this further? Let's say at some point we > will have callable prototypes or generic classes: we will need to encode > somehow this type into zend_type: `callable(A)` or `A`. > Even right now it might be useful (as you suggest with ZEND_TYPE_CE) > to store a (zend_class_entry *) instead of (zend_string *) when > it is known to us in the zend_type. > Seems like without extending zend_type to the size of two pointers > it almost isn't doable :\ > Or, it could be made that zend_type, when it's not a simple type hint, > would point to the `zend_type_complex` which would store a > zend_class_entry pointer (or not, if it's for callable) and an array > of type specifiers. But that introduces another indirection. > > Anyway thanks for polishing this part, we definitely need zend_type in > some form. > > -- > > > > -- regards / pozdrawiam, -- Michał Brzuchalski about.me/brzuchal brzuchalski.com --- ## #97678 — Nikita Nefedov — 2017-01-11T13:57:26+00:00 URL: https://externals.io/message/97674#97678 · In reply to: #97677 On Wed, 11 Jan 2017 16:51:22 +0300, Michał Brzuchalski wrote: > > > 2017-01-11 14:35 GMT+01:00 Nikita Nefedov : > > […] > > I've got prepared Object Typehint RFC > https://wiki.php.net/rfc/object-typehint where > IS_OBJECT is used without class name as type hint for any object kind, > if this patch > would be applied how can I deal with this new zend_type? > As far as I undestand last 0 for IS_OBJECT and no (zend_string *) > pointer would give me > empty zend_string value right? So that won't bive me any chances to > store IS_OBJECT > without classname am I right? Hey Michal, no for you it's quite easy, you can just store IS_OBJECT with ZEND_TYPE_ENCODE_HINT(IS_OBJECT, is_null) But then the code that checks ZEND_TYPE_IS_CLASS will need to be adjusted to work with ZEND_TYPE_IS_HINT(type) == 1 && ZEND_TYPE_HINT(type) == IS_OBJECT --- ## #97679 — michal@brzuchalski.com — 2017-01-11T13:59:25+00:00 URL: https://externals.io/message/97674#97679 · In reply to: #97678 2017-01-11 14:57 GMT+01:00 Nikita Nefedov : > On Wed, 11 Jan 2017 16:51:22 +0300, Michał Brzuchalski < > michal@brzuchalski.com> wrote: > > > > […] > > Hey Michal, > > no for you it's quite easy, you can just store IS_OBJECT with > > ZEND_TYPE_ENCODE_HINT(IS_OBJECT, is_null) > > But then the code that checks ZEND_TYPE_IS_CLASS will need to > be adjusted to work with > > ZEND_TYPE_IS_HINT(type) == 1 && ZEND_TYPE_HINT(type) == IS_OBJECT > Thanks. -- regards / pozdrawiam, -- Michał Brzuchalski about.me/brzuchal brzuchalski.com --- ## #97680 — Niklas Keller — 2017-01-11T14:00:02+00:00 URL: https://externals.io/message/97674#97680 · In reply to: #97677 Michał Brzuchalski schrieb am Mi., 11. Jan. 2017, 14:51: > 2017-01-11 14:35 GMT+01:00 Nikita Nefedov : > > > […] > wrote: > > […] > just > > […] > I've got prepared Object Typehint RFC > https://wiki.php.net/rfc/object-typehint where > IS_OBJECT is used without class name as type hint for any object kind, if > this patch > would be applied how can I deal with this new zend_type? > As far as I undestand last 0 for IS_OBJECT and no (zend_string *) pointer > would give me > empty zend_string value right? So that won't bive me any chances to store > IS_OBJECT > without classname am I right? > Morning. As far as I understand it, 0 just means no built in type. As long as no class name is there, it's just no type at all. Regards, Niklas > [xxxx xxxx xxxx xxxx] xxxx xxxx xxxx xxy1 - for all other type hints > > […] > > > -- > regards / pozdrawiam, > -- > Michał Brzuchalski > about.me/brzuchal > brzuchalski.com --- ## #97681 — Bob Weinand — 2017-01-11T14:09:25+00:00 URL: https://externals.io/message/97674#97681 · In reply to: #97675 > Am 11.01.2017 um 14:35 schrieb Nikita Nefedov : > > > > > […] > > Hey Dmitry, > > Having worked on callable prototypes I'd say unifying PHP types in Zend > is something we urgently need for PHP to continue evolving. > > I'm not sure if PHP have ever been compatible with less-than-32bit > archs but if it was I think it should be said that this would break > such compatibility though. PHP itself uses more than 2^16 bytes of memory… And we anyway have some code assuming that sizeof(void *) is a power of two. Hence we'll anyway need at least 32 bit. > It would be great if there were some comment in the code near zend_type > declaration where you'd explain how it is used and how additional > data is being added to the pointer. That's what the ZEND_TYPE_ENCODE_*() macros are for? > Is there any use of ZEND_TYPE_CE() macro? It seems to be forgotten there? Not necessarily in this patch, but in typed_ref_property branch reference types use the CE directly. > If I understood this correctly, the layout of zend_type is as follows: > > [xxxx xxxx xxxx xxxx] xxxx xxxx xxxx xxy0 - for IS_OBJECT type hint > where the `xxxx`s are a (zend_string *) pointer and `y` designates > an allow_null flag > > [xxxx xxxx xxxx xxxx] xxxx xxxx xxxx xxy1 - for all other type hints > where the `xxxx`s are a IS_CALLABLE, _IS_BOOL, IS_LONG, IS_DOUBLE, > IS_STRING or IS_ARRAY correct. > Do we decide here that IS_REF modifier should belong to the concrete > usages of the type (e.g. referentiality is a property of a variable > and not of a type)? > I'm not sure this if is a right decision or not but I feel like this > question should be raised. It is usually the opposite in other languages. We nowhere user-facing consider references to be a separate type, just internally for the zval representation (which was is_ref in PHP 5) - and as such being a reference is a property of the variable. > How would you plan to extend this further? Let's say at some point we > will have callable prototypes or generic classes: we will need to encode > somehow this type into zend_type: `callable(A)` or `A`. > Even right now it might be useful (as you suggest with ZEND_TYPE_CE) > to store a (zend_class_entry *) instead of (zend_string *) when > it is known to us in the zend_type. > Seems like without extending zend_type to the size of two pointers > it almost isn't doable :\ > Or, it could be made that zend_type, when it's not a simple type hint, > would point to the `zend_type_complex` which would store a > zend_class_entry pointer (or not, if it's for callable) and an array > of type specifiers. But that introduces another indirection. zend_type is just a typedef. If needed in future we may define it to occupy 2 * sizeof(void *) or such, just as we'll need it. The goal of the patch is making type encoding transparent and handleable in a single place. The current way of adding a new class of types usually was adding a new element to the relevant structs or similar hacks… Bob > Anyway thanks for polishing this part, we definitely need zend_type in some form. --- ## #97682 — Joe Watkins — 2017-01-11T14:11:03+00:00 URL: https://externals.io/message/97674#97682 · In reply to: #97680 Morning Dmitry, I know bob has already requested some additions ... nothing to really say about it ... I just wanted to chime in with VERY NICE :D (shouting intentional) ... Oh and, I think an RFC is pretty pointless ... */me looks forward to master having this* Cheers Joe > Michał Brzuchalski schrieb am Mi., 11. Jan. 2017, > 14:51: > > > […] > that > > […] > work > > […] > there? > > […] > > Morning. > > As far as I understand it, 0 just means no built in type. As long as no > class name is there, it's just no type at all. > > Regards, Niklas > > > > […] > languages. > > […] > encode > > […] --- ## #97684 — Markus Fischer — 2017-01-11T14:16:23+00:00 URL: https://externals.io/message/97674#97684 · In reply to: #97674 > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 - there's a typo "/* this is a calss name */" a few times - a few times I see this condition: "if (info->type > 0x3ff) {" Shouldn't we use *some* descriptive constant for this seamingly cryptic looking 0x3ff ? The approach looks great .. are there any benchmarks? thanks, - Markus --- ## #97690 — Derick Rethans — 2017-01-11T15:43:50+00:00 URL: https://externals.io/message/97674#97690 · In reply to: #97674 > Hi, > > > I propose to introduce a unified type representation (zend_type). > > Now it's going to be used for typing of arguments and return values. > > Later we should use it for properties and other things. > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 > > > The main changes are in zend_types.h and zend_compile.h, the rest is just an adoption for new type representation. > > I don't think we need RFC, because this is just an internal change that doesn't change behavior. > > > I got the idea working on typed properties together with Bob and Joe. > > https://github.com/php/php-src/compare/master...bwoebi:typed_ref_properties > > I think it would be better to introduce zend_type and then continue work on typed properties. > > > Any comments? 208 + if (new_arg_info[i].type > 0x3ff) { I wouldn't use a magical constant there, but do a define of what 0x3ff actually is. 209 + /* this is a calss name */ That's spelled "class" (not "calss"). cheers, Derick --- ## #97692 — Dmitry Stogov — 2017-01-11T16:37:57+00:00 URL: https://externals.io/message/97674#97692 The patch was updated according to feedback: added comments, better names and encapsulation, less magic, better code reuse, keep a free bit in zend_type for future extension. https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 Thanks. Dmitry. --- ## #97693 — Levi Morrison — 2017-01-11T17:24:53+00:00 URL: https://externals.io/message/97674#97693 · In reply to: #97692 > The patch was updated according to feedback: added comments, better names and encapsulation, less magic, better code reuse, keep a free bit in zend_type for future extension. > > > > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 > > > Thanks. Dmitry. > > > > ________________________________ > From: Derick Rethans > Sent: Wednesday, January 11, 2017 6:43:50 PM > To: Dmitry Stogov > Cc: PHP internals list; Bob Weinand; Joe Watkins; Zeev Suraski; Anatol Belski (ab@php.net); Nikita Popov; Xinchen Hui > Subject: Re: [PHP-DEV] Change in type-hint representation > > > > > […] > > 208 + if (new_arg_info[i].type > 0x3ff) { > > I wouldn't use a magical constant there, but do a define of what 0x3ff > actually is. > > 209 + /* this is a calss name */ > > That's spelled "class" (not "calss"). > > cheers, > Derick As part of this effort can we refactor the IS_LONG, IS_ARRAY, IS_OBJECT, etc macros to use an enum? Maybe `zend_type_code` if you like the `code` name for it? Also we already use "kind" in the AST; should it be `ZEND_TYPE_KIND` and `zend_type_kind` instead? Overall I think this cleanup is needed. Thanks, Dmitry. --- ## #97694 — Bob Weinand — 2017-01-11T18:13:33+00:00 URL: https://externals.io/message/97674#97694 · In reply to: #97692 > Am 11.01.2017 um 17:37 schrieb Dmitry Stogov : > > The patch was updated according to feedback: added comments, better names and encapsulation, less magic, better code reuse, keep a free bit in zend_type for future extension. > > > > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 > > > Thanks. Dmitry. Looks good so far… Thought I've asked you before on IRC to somehow extract validation so that we don't have to duplicate it everywhere. Validate (and cast if necessary) given a zval, strict mode and the representation type (class present as ce? as string?) Also, perhaps a string representation of the type (reusable in reflection and in error messages) Thanks, Bob --- ## #97696 — Anatol Belski — 2017-01-11T18:41:28+00:00 URL: https://externals.io/message/97674#97696 · In reply to: #97674 Hi Dmitry, > -----Original Message----- > From: Dmitry Stogov [mailto:dmitry@zend.com] > Sent: Wednesday, January 11, 2017 1:08 PM > To: PHP internals list ; Bob Weinand > ; Joe Watkins > Cc: Zeev Suraski ; Anatol Belski (ab@php.net) ; > Nikita Popov ; Xinchen Hui > Subject: [PHP-DEV] Change in type-hint representation > > Hi, > > > I propose to introduce a unified type representation (zend_type). > > Now it's going to be used for typing of arguments and return values. > > Later we should use it for properties and other things. > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 > > > The main changes are in zend_types.h and zend_compile.h, the rest is just an > adoption for new type representation. > > I don't think we need RFC, because this is just an internal change that doesn't > change behavior. > > > I got the idea working on typed properties together with Bob and Joe. > > https://github.com/php/php- > src/compare/master...bwoebi:typed_ref_properties > > I think it would be better to introduce zend_type and then continue work on > typed properties. > I just did a test compilation and everything looks good. There's one place in ext/com_dotnet/com_handlers.c:305 that would require an adaptation. In this regard a question - would possibly a universal macro make sense, to change the "null allowed" mask ? Also, a macro to change the type, with/without changing the null allowed mask. Just as an API extension to ease the consumer code. I mean, clear, it'd be doable with just a bit more effort, but an API could be handier also for the future compatibility, should the implementation change. For the rest IMO - very elegant approach giving base to facilitate arbitrary types. I also think RFC were a waste of time. Thanks everyone involved! Regards Anatol --- ## #97698 — Nikita Nefedov — 2017-01-11T19:20:58+00:00 URL: https://externals.io/message/97674#97698 · In reply to: #97692 > > > The patch was updated according to feedback: added comments, better names and encapsulation, less magic, better code reuse, keep a free bit in zend_type for future extension. > > > > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 > > > Thanks. Dmitry. Thanks for the patch, looks good to me! --- ## #97701 — Nikita Popov — 2017-01-11T20:46:02+00:00 URL: https://externals.io/message/97674#97701 · In reply to: #97674 > Hi, > > > I propose to introduce a unified type representation (zend_type). > > Now it's going to be used for typing of arguments and return values. > > Later we should use it for properties and other things. > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 > > > The main changes are in zend_types.h and zend_compile.h, the rest is just > an adoption for new type representation. > > I don't think we need RFC, because this is just an internal change that > doesn't change behavior. > > > I got the idea working on typed properties together with Bob and Joe. > > https://github.com/php/php-src/compare/master...bwoebi: > typed_ref_properties > > I think it would be better to introduce zend_type and then continue work > on typed properties. > > > Any comments? > > > Thanks. Dmitry. > I like this. Two questions: 1. Can you please clarify what ZEND_TYPE_ENCODE_CLASS_CONST_* is for? Why does this not use the usual encoding for allow_null? Does the compiler not allow casts in initializers or something? 2. Is the use of zend_strings in arginfo of internal functions zts safe? I.e. does the function registration happen per-thread or do we reuse registered functions across threads? If that's the case we might run into issues with non-atomic RCs. Nikita --- ## #97707 — Andrea Faulds — 2017-01-11T22:30:18+00:00 URL: https://externals.io/message/97674#97707 · In reply to: #97693 Hi Levi, Levi Morrison wrote: > As part of this effort can we refactor the IS_LONG, IS_ARRAY, > IS_OBJECT, etc macros to use an enum? Maybe `zend_type_code` if you > like the `code` name for it? Also we already use "kind" in the AST; > should it be `ZEND_TYPE_KIND` and `zend_type_kind` instead? C's enum members always have the type `int`. However, typically PHP uses the smaller `zend_uchar` for representing a type code. I guess this is why we don't use an enum already. Having a typedef of zend_uchar for this purpose would be great, though. It would make some function signatures more obvious (e.g. is_numeric_string and friends return a zend_uchar for the type of the result, and it'd be clearer this is the case). Thanks. -- Andrea Faulds https://ajf.me/ --- ## #97708 — Levi Morrison — 2017-01-11T22:38:55+00:00 URL: https://externals.io/message/97674#97708 · In reply to: #97707 > Hi Levi, > > Levi Morrison wrote: > > […] > > > C's enum members always have the type `int`. However, typically PHP uses the > smaller `zend_uchar` for representing a type code. I guess this is why we > don't use an enum already. This is not true; rather they are only required to support values that can be held in `int`. Implementations are free to compact it. However it is an issue I had forgotten about, and it is certainly something to keep in mind. > Having a typedef of zend_uchar for this purpose would be great, though. It > would make some function signatures more obvious (e.g. is_numeric_string and > friends return a zend_uchar for the type of the result, and it'd be clearer > this is the case). Yes, at least *something* would be nice if `enum` is problematic. --- ## #97709 — Andrea Faulds — 2017-01-11T22:39:56+00:00 URL: https://externals.io/message/97674#97709 · In reply to: #97674 Hey Dmitry, Dmitry Stogov wrote: > I propose to introduce a unified type representation (zend_type). > > Now it's going to be used for typing of arguments and return values. > > Later we should use it for properties and other things. > > > https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1 I like this proposal, particularly how it's more compact than our existing representation. One comment: it appears that these zend_type values can encode both pointers to strings (class names) and pointers to class entries, right? How do you know whether a given zend_type value is one or the other? Should you just know from context? (It looks like ZEND_TYPE_CE() is in fact unused right now…) On a related note, look at this portion of the diff: + if (ZEND_TYPE_IS_SET(info->type)) { + if (ZEND_TYPE_IS_CLASS(info->type)) { + const char *type_name = (const char*)info->type; Is this a type confusion between zend_string* and const char*? Thanks! -- Andrea Faulds https://ajf.me/ --- ## #97716 — Dmitry Stogov — 2017-01-12T08:19:14+00:00 URL: https://externals.io/message/97674#97716 Hi Andrea, 1) ZEND_TYPE_CE() is not used yet. We may make distinct between zend_string* and zend_class_entry* based on context or using a value of reserved bit (0x2). 2) We can't initialize zend_strings* in constant structures, so we store const char*. This code converts that const char* into zend_string*. Thanks. Dmitry. --- ## #97717 — Dmitry Stogov — 2017-01-12T08:23:36+00:00 URL: https://externals.io/message/97674#97717 Hi Bob, I don't exactly understand, what do you like. Can you explain, or may be make it on top of this patch? (I think I'll commit it on Friday) Thanks. Dmitry. --- ## #97724 — Andrea Faulds — 2017-01-12T14:43:23+00:00 URL: https://externals.io/message/97674#97724 · In reply to: #97716 Hi, Dmitry Stogov wrote: > 1) ZEND_TYPE_CE() is not used yet. We may make distinct between zend_string* and zend_class_entry* based on context or using a value of reserved bit (0x2). Ah, I see. Until it is used, could it be removed? Its presence right now creates confusion. > 2) We can't initialize zend_strings* in constant structures, so we store const char*. This code converts that const char* into zend_string*. I see, thanks. -- Andrea Faulds https://ajf.me/