On 10.10.2008, at 22:58, Stanislav Malyshev wrote:
Hi!
I've updated the patch and added some tests with it.
http://sitten-polizei.de/php/getstatic.diff
Looked at the patch. There's some things I noticed there:
1. _getstatic->common.fn_flags |= ~ZEND_ACC_ALLOW_STATIC;
What was the idea here? Maybe ~ is not intended?
2. Do we really need ZEND_ASSIGN_CLASS opcode, can't we reuse
ASSIGN_OBJ with flag, as we do for fetches?
3. In zend_std_set_static_property, I'm not sure we need to do
zend_update_class_constants there. We won't be using any values from
it.
4. In zend_std_set_static_property, why you create new property_info
two times when property does not exist?
5. In zend_std_set_static_property, old value of the property is not
destroyed. Also, I'm not sure separation is handled there correctly
- previous value may be shared between variables, and just replacing
it would affect all shared variables. You may see how property
handler does assignments.
Alternatively, in the absence of the setter, you may just use the
existing assignment handler, just fetching the zval** as before and
passing it to zend_assign_to_variable. This would probably be better
- less stuff to do.
6. What would happen with foo::$bar += 1? I don't see assign-ops
handled anywhere in the patch.
7. Does zend_std_get_static_property handle the case of return by-
ref like property one does?
hmm .. i also emailed Timm a few weeks ago and got no reaction. the
question now is .. does someone else care enough to work through the
issues Stas has noted to get things in shape to be committed?
regards,
Lukas Kahwe Smith
[EMAIL PROTECTED]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
