Everything

php

Constants and static methods for internal classes

php.internals

Dmitry Stogov

21 years ago

Hi,

You can look into proposal patch that implements support for constants and
static methods for internal classes. The original patch was written by
Michael and improved by me.

The patch is maden for "easiest" usage and it doesn't support internal
array/constants properties (they weren't supported before).
Unless something thinks array/constants are badly needed (we think they
probably aren't) we prefer to keep it simple like this.

Support for array and constant properties will require additional
complicated API.

Thanks. Dmitry.

Michael Wallner

21 years ago

Hi Dmitry Stogov, you wrote:

> You can look into proposal patch that implements support for constants and
> static methods for internal classes. The original patch was written by
> Michael and improved by me.

Is it my (mailers) fault or didn't the patch make it through?

Thanks,

--
Michael - < mike(@)php.net >

Dmitry Stogov

21 years ago

Michael Wallner

21 years ago

Hi Dmitry Stogov, you wrote:

> The patch is maden for "easiest" usage and it doesn't support internal
> array/constants properties (they weren't supported before).

So why burden the weigh of allocation, initialization, destruction
and freeing on the extension developer?  This will force every extension
writer to implement a kind of my dup_zval() function, wouldn't it?
And as there's not much reference devs will spam the list on how
to use static properties right.  I think we could avoid this by
stretching the minimalistic approach a bit and provide at least
a zend_init_static_properties().

Thanks a lot,

--
Michael - < mike(@)php.net >

Michael Wallner

21 years ago

I wrote:

> So why burden the weigh of allocation, initialization, destruction
> and freeing on the extension developer?  This will force every extension
> writer to implement a kind of my dup_zval() function, wouldn't it?
> And as there's not much reference devs will spam the list on how
> to use static properties right.  I think we could avoid this by
> stretching the minimalistic approach a bit and provide at least
> a zend_init_static_properties().

One note more though :-/
If we leave out the zend_update_static_property() API,
extension developers need to reinvent the wheel again
and again.

Regards,

--
Michael - < mike(@)php.net >

Dmitry Stogov

21 years ago

Hi,

> -----Original Message-----
> From: Michael Wallner [mailto:mike@php.net]
> Sent: Tuesday, August 30, 2005 1:56 PM
> To: internals@lists.php.net; Dmitry Stogov
> Cc: internals@lists.php.net; Andi Gutmans
> Subject: [PHP-DEV] Re: Constants and static methods for
> internal classes
>
>
> Hi Dmitry Stogov, you wrote:
>
> > The patch is maden for "easiest" usage and it doesn't
> support internal
> > array/constants properties (they weren't supported before).
>
> So why burden the weigh of allocation, initialization,
> destruction and freeing on the extension developer?  This
> will force every extension writer to implement a kind of my
> dup_zval() function, wouldn't it?

Static members works with this patch (except array and constant properties)
without any additional requirements for extension. See test extension in
first email. Developer should just call zend_declare_property_...() or
zend_declare_class_constant_...() in MINIT (no any special allocation,
initialization, destruction and freeing are required).

To support arrays and constants API should be extended with

zend_declare_property_constant()
zend_declare_class_constant_constant()

zend_declare_property_constant_array()
zend_declare_class_constant_constant_array()

init_constant_array()
constant_array_add_assoc_...()
constant_array_index_insert_...()
constant_array_next_inex_insert_...()

Do we need this?

> And as there's not much
> reference devs will spam the list on how to use static
> properties right.  I think we could avoid this by stretching
> the minimalistic approach a bit and provide at least a
> zend_init_static_properties().

We already have zend_declare_property_...() those are good enough to declare
both static and non-static properties.
Why we need new functions?

Dmitry.

Michael Wallner

21 years ago

Hi Dmitry Stogov, you wrote:

> Static members works with this patch (except array and constant properties)
> without any additional requirements for extension. See test extension in
> first email. Developer should just call zend_declare_property_...() or
> zend_declare_class_constant_...() in MINIT (no any special allocation,
> initialization, destruction and freeing are required).

Sorry, I missed that the default properties will be copied to
the static_members table in zend_update_class_constants().

> To support arrays and constants API should be extended with

[...]

> Do we need this?

I don't think so, and never requested that functionality.

> We already have zend_declare_property_...() those are good enough to declare
> both static and non-static properties.

Sorry, see first comment.

> Why we need new functions?

In case of zend_update_static_property(), because one would need
to replicate the body of that function at each writing access?

Regards,

--
Michael - < mike(@)php.net >

Michael Wallner

21 years ago

I wrote:

> In case of zend_update_static_property(), because one would need
> to replicate the body of that function at each writing access?

Please note that the code I'm currently using has changed from the original patch:

int zend_update_static_property(zend_class_entry *scope, char *name, size_t name_len, zval *value TSRMLS_DC)
{
int retval;
zval **property = NULL;
zend_class_entry *old_scope = EG(scope);

EG(scope) = scope;

if (!(property = zend_std_get_static_property(scope, name, name_len, 0 TSRMLS_CC))) {
retval = FAILURE;
} else if (*property == value) {
retval = SUCCESS;
} else {
value->refcount++;
if (PZVAL_IS_REF(*property)) {
zval_dtor(*property);
(*property)->type = value->type;
(*property)->value = value->value;

if (value->refcount) {
zval_copy_ctor(*property);
}
} else {
REPLACE_ZVAL_VALUE(property, value, 1);
}
retval = SUCCESS;
}
zval_ptr_dtor(&value);
EG(scope) = old_scope;

return retval;
}

Regards,

--
Michael - < mike(@)php.net >

Dmitry Stogov

21 years ago

Hi Michael,

As I understand the whole patch is OK for you.

This function make sense and I can add it then I'll commit the patch.
I am not sure about refcount/reference magic and I haven't time to look into
it right now.
I'll check it before committing.

Thanks. Dmitry.

Michael Wallner

21 years ago

Hi Dmitry Stogov, you wrote:

> As I understand the whole patch is OK for you.

Yep, I still need to test it because your patch is against HEAD
and I need to port it to PHP_5_1 because my extension is not
unicode aware.  My main concern is that it works (of course)
and that it's not to hard for the dev - i.e. me :) - to use it.
May I suggest to add the usual wrappers like for zend_update_property()?

Thanks a lot,

--
Michael - < mike(@)php.net >

Dmitry Stogov

21 years ago

OK. Let me know if you will find any problems with patch.

I am not sure that we will aplly this patch to 5.1.
Andi? Zeev?

zend_update_static_property_...() make sense and I'll add it.

Thanks. Dmitry.

Michael Wallner

21 years ago

Hi Dmitry Stogov, you wrote:

> OK. Let me know if you will find any problems with patch.

There's a typo in zend_update_class_constants():
ALLOC_HASHTABLE() refers to class_type instead of class_type->static_members

There's also a problem which I didn't track down yet:
static_members are destroyed after the request but not reinitialized

Regards,

--
Michael - < mike(@)php.net >

Dmitry Stogov

21 years ago

Hi,

> -----Original Message-----
> From: Michael Wallner [mailto:mike@php.net]
> Sent: Tuesday, August 30, 2005 7:34 PM
> To: internals@lists.php.net
> Cc: Dmitry Stogov
> Subject: Re: [PHP-DEV] Re: Constants and static methods for
> internal classes
>
>
> Hi Dmitry Stogov, you wrote:
>
> > OK. Let me know if you will find any problems with patch.
>
> There's a typo in zend_update_class_constants():
> 	ALLOC_HASHTABLE() refers to class_type instead of
> class_type->static_members

Seems you are wrong.
I see ALLOC_HASHTABLE(class_type->static_members); in the patch.

> There's also a problem which I didn't track down yet:
> 	static_members are destroyed after the request but not
> reinitialized

The are reinicilized on next access through the same
zend_update_class_constants() from default values.

Dmitry.

Marcus Börger

21 years ago

Hello Dmitry, Andi, Zeev,

i'd love to see this in 5.1. so that i could drop all defines in SPL
and make them class constants, thus cleaning up global namespace a bit.

best regards
marcus

Tuesday, August 30, 2005, 3:37:34 PM, you wrote:

Zeev Suraski

21 years ago

I think the patch is fine.  We should probably have RC2 for PHP 5.1 anyway,
so I don't see a problem putting it into the 5.1 tree (it does fix a bug
and it does break binary compatibility).

Zeev

At 16:37 30/08/2005, Dmitry Stogov wrote:

Andi Gutmans

21 years ago

Okay so if we're going RC2, then I think we can also MFH instanceof
(which is a tiny patch).

Andi

At 11:51 AM 8/30/2005, Zeev Suraski wrote:

Dmitry Stogov

21 years ago

The patch had a serious bug that corrupts memory on static property update.
New patch fixes the bug and adds utility functions.

Thanks. Dmitry.

Dmitry Stogov

21 years ago

> I think we could avoid this by stretching
> the minimalistic approach a bit and provide at least a
> zend_init_static_properties().

We don't need zend_init_static_properties() at all.
The initialization performs automatic before any access to static member in
zend_update_class_constants().

Dmitry.