Change in type-hint representation View as Markdown

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

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

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

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

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

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/

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

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.

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

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!

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/