Change in type-hint representation

Nikita

9 years ago

On Wed, 11 Jan 2017 15:07:39 +0300, Dmitry Stogov <dmitry@zend.com> wrote:

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

Michał Brzuchalski

9 years ago

2017-01-11 14:35 GMT+01:00 Nikita Nefedov <inefedor@gmail.com>:

> On Wed, 11 Jan 2017 15:07:39 +0300, Dmitry Stogov <dmitry@zend.com> wrote:
>
> 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
>
>

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<Foo>`.
> 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.
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

--
regards / pozdrawiam,
--
Michał Brzuchalski
about.me/brzuchal
brzuchalski.com

Nikita

9 years ago

On Wed, 11 Jan 2017 16:51:22 +0300, Michał Brzuchalski
<michal@brzuchalski.com> wrote:

>
>
> 2017-01-11 14:35 GMT+01:00 Nikita Nefedov <inefedor@gmail.com>:
>> -- snip --
>>
>>
>>  [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?

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

Michał Brzuchalski

9 years ago

2017-01-11 14:57 GMT+01:00 Nikita Nefedov <inefedor@gmail.com>:

> On Wed, 11 Jan 2017 16:51:22 +0300, Michał Brzuchalski <
> michal@brzuchalski.com> wrote:
>
>
>>
>> 2017-01-11 14:35 GMT+01:00 Nikita Nefedov <inefedor@gmail.com>:
>>
>>> -- snip --
>>>
>>>
>>>  [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/objec
>> t-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
>

Thanks.

--
regards / pozdrawiam,
--
Michał Brzuchalski
about.me/brzuchal
brzuchalski.com

Niklas Keller

9 years ago

Michał Brzuchalski <michal@brzuchalski.com> schrieb am Mi., 11. Jan. 2017,
14:51:

> 2017-01-11 14:35 GMT+01:00 Nikita Nefedov <inefedor@gmail.com>:
>
> > On Wed, 11 Jan 2017 15:07:39 +0300, Dmitry Stogov <dmitry@zend.com>
> wrote:
> >
> > 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
> >
> >
> 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

Joe Watkins

9 years ago

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

On Wed, Jan 11, 2017 at 2:00 PM, Niklas Keller <me@kelunik.com> wrote:

Bob Weinand

9 years ago

> Am 11.01.2017 um 14:35 schrieb Nikita Nefedov <inefedor@gmail.com>:
>
> On Wed, 11 Jan 2017 15:07:39 +0300, Dmitry Stogov <dmitry@zend.com> wrote:
>
>> 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.

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

Markus Fischer

9 years ago

On 11.01.17 13:07, Dmitry Stogov wrote:

> 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

Derick Rethans

9 years ago

On Wed, 11 Jan 2017, Dmitry Stogov wrote:

> 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

Dmitry Stogov

9 years ago

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>

<https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1>

https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1

Thanks. Dmitry.

________________________________
From: Derick Rethans <derick@php.net>
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

On Wed, 11 Jan 2017, Dmitry Stogov wrote:

> 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

Levi Morrison

9 years ago

On Wed, Jan 11, 2017 at 9:37 AM, Dmitry Stogov <dmitry@zend.com> wrote:

> 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>
>
>
> <https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1>
>
> https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1
>
>
> Thanks. Dmitry.
>
>
>
> ________________________________
> From: Derick Rethans <derick@php.net>
> 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
>
> On Wed, 11 Jan 2017, Dmitry Stogov wrote:
>
>> 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

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.

Andrew Faulds

9 years ago

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/

Levi Morrison

9 years ago

On Wed, Jan 11, 2017 at 3:30 PM, Andrea Faulds <ajf@ajf.me> wrote:

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

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.

Bob Weinand

9 years ago

> Am 11.01.2017 um 17:37 schrieb Dmitry Stogov <dmitry@zend.com>:
>
> 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>
>
>
> <https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1>
>
> 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

Dmitry Stogov

9 years ago

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.

________________________________
From: Bob Weinand <bobwei9@hotmail.com>
Sent: Wednesday, January 11, 2017 9:13:33 PM
To: Dmitry Stogov
Cc: Derick Rethans; PHP internals list; Joe Watkins; Zeev Suraski; Anatol Belski (ab@php.net); Nikita Popov; Xinchen Hui
Subject: Re: [PHP-DEV] Change in type-hint representation

> Am 11.01.2017 um 17:37 schrieb Dmitry Stogov <dmitry@zend.com>:
>
> 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>
>
>
> <https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1>
>
> 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

Nikita

9 years ago

> On 11 Jan 2017, at 19:37, Dmitry Stogov <dmitry@zend.com> wrote:
>
> 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>
>
>
> <https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1>
>
> https://gist.github.com/dstogov/1b25079856afccf0d69f77d499cb0ab1
>
>
> Thanks. Dmitry.

Thanks for the patch, looks good to me!

Anatoliy Belsky

9 years ago

Hi Dmitry,

> -----Original Message-----
> From: Dmitry Stogov [mailto:dmitry@zend.com]
> Sent: Wednesday, January 11, 2017 1:08 PM
> To: PHP internals list <internals@lists.php.net>; Bob Weinand
> <bobwei9@hotmail.com>; Joe Watkins <pthreads@pthreads.org>
> Cc: Zeev Suraski <zeev@zend.com>; Anatol Belski (ab@php.net) <ab@php.net>;
> Nikita Popov <nikic@php.net>; Xinchen Hui <xinchen.hui@roguewave.com>
> 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

Nikita Popov

9 years ago

On Wed, Jan 11, 2017 at 1:07 PM, Dmitry Stogov <dmitry@zend.com> wrote:

> 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

Andrew Faulds

9 years ago

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/

Dmitry Stogov

9 years ago

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.

________________________________
From: Andrea Faulds <ajf@ajf.me>
Sent: Thursday, January 12, 2017 1:39:56 AM
To: internals@lists.php.net
Subject: [PHP-DEV] Re: Change in type-hint representation

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/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Andrew Faulds

9 years ago

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/