Re: [VOTE] Deprecate and remove calls from incompatible context (with patch)
| From: | Alan Knowles | Date: | Tue, 05 Feb 2013 23:55:06 +0000 |
| Subject: | Re: [VOTE] Deprecate and remove calls from incompatible context (with patch) | ||
| References: | 1 2 3 4 5 6 7 8 9 10 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-65684@lists.php.net to get a copy of this message | ||
Attached should be a patch to illustrate the exception. (obviously needs
some tests modified, or created)
This removes the warning in the single situation where the calling scope
and target scope share the same top level parent. eg. Traits without the
baggage...
I think this is a reasonable change, It removes a warning which is only
partly true. In this small exception, It enables the language to behave
more like a scripting language, where yes, we do frequently have the
possibility of incompatible types, but only emits hints and notices
when they are really helpful and accurate.
Hopefully this means I can finally turn E_STRICT warnings on.
Regards
Alan
On Thursday, January 31, 2013 07:41 PM, Ferenc Kovacs wrote:
>> The fact that this use of PHP is documented in the manual as a feature
>>
>> www.php.net/manual/en/**language.oop5.basic.php<http://www.php.net/manual/en/language.oop5.basic.php
)oúMÄíþ¬
>> i>
>>
>> And mentions that it will elicit a E_STRICT error - does not indicate that
>> it would be DEPRECATED, I'm assuming that has been documented for years,
>> and only recently (a year or two) has the E_STRICT comment been added.
>> There is also no real Justification for the E_STRICT message = see
>> suggestion at end..
>>
> I stand corrected, I said that (AFAIR) is an undocumented feature, but as
> you pointed out, it is documented since (at least) 2004:
>
> http://svn.php.net/viewvc/phpdoc/en/trunk/language/oop5/basic.xml?view=markup&pathrev=166424
> (there is a typo here, so it is called a psudo variable)
> And an example was added in 2005:
> http://svn.php.net/viewvc?view=revision&revision=178495 by
> sean (funny
> commit comment: 'document seemingly-odd $this behaviour').
> The E_STRICTs were added to the examples in 2009:
> http://svn.php.net/viewvc?view=revision&revision=288217
>
> This doesn't really change my opinion, but it means that there could be
> more people using this feature than I/we assumed.
> Given the fact that this already spits E_STRICT(and E_STRICT is part of
> E_ALL since 5.4) and both E_STRICT and E_DEPRECATED is disable in
> our php.ini-production, I would say that changing this from E_STRICT to
> E_DEPRECATED has no direct impact to the userland.
> But we can use this change to sample the people using this feature and
> based on the feedback we can decide whether we want to keep this or remove
> it for the next version.
> So my opinion is that we should stick to the vote and deprecate this
> feature but update the RFC and remove the part removing it in the next
> version.
>
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 27b2bd1..c6fcdcb 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -2591,12 +2591,20 @@ ZEND_VM_HANDLER(113, ZEND_INIT_STATIC_METHOD_CALL, CONST|VAR, CONST|TMP|VAR|UNUS !instanceof_function(Z_OBJCE_P(EG(This)), ce TSRMLS_CC)) { /* We are calling method of the other (incompatible) class, but passing $this. This is done for compatibility with php-4. */ - if (call->fbc->common.fn_flags & ZEND_ACC_ALLOW_STATIC) { - zend_error(E_STRICT, "Non-static method %s::%s() should not be called statically, assuming $this from incompatible context", call->fbc->common.scope->name, call->fbc->common.function_name); - } else { + + zend_class_entry *top_ce = ce; + + if (!(call->fbc->common.fn_flags & ZEND_ACC_ALLOW_STATIC)) { /* An internal function assumes $this is present and won't check that. So PHP would crash by allowing the call. */ zend_error_noreturn(E_ERROR, "Non-static method %s::%s() cannot be called statically, assuming $this from incompatible context", call->fbc->common.scope->name, call->fbc->common.function_name); } + + while (top_ce->parent) top_ce = top_ce->parent; + + /* if calling scope and target scope share the same parent, do not show warning */ + if (!instanceof_function(Z_OBJCE_P(EG(This)), top_ce TSRMLS_CC)) + zend_error(E_DEPRECATED, "Non-static method %s::%s() should not be called statically, assuming $this from incompatible context", call->fbc->common.scope->name, call->fbc->common.function_name); + } } if ((call->object = EG(This))) { Z_ADDREF_P(call->object);
diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 27b2bd1..c6fcdcb 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -2591,12 +2591,20 @@ ZEND_VM_HANDLER(113, ZEND_INIT_STATIC_METHOD_CALL, CONST|VAR, CONST|TMP|VAR|UNUS !instanceof_function(Z_OBJCE_P(EG(This)), ce TSRMLS_CC)) { /* We are calling method of the other (incompatible) class, but passing $this. This is done for compatibility with php-4. */ - if (call->fbc->common.fn_flags & ZEND_ACC_ALLOW_STATIC) { - zend_error(E_STRICT, "Non-static method %s::%s() should not be called statically, assuming $this from incompatible context", call->fbc->common.scope->name, call->fbc->common.function_name); - } else { + + zend_class_entry *top_ce = ce; + + if (!(call->fbc->common.fn_flags & ZEND_ACC_ALLOW_STATIC)) { /* An internal function assumes $this is present and won't check that. So PHP would crash by allowing the call. */ zend_error_noreturn(E_ERROR, "Non-static method %s::%s() cannot be called statically, assuming $this from incompatible context", call->fbc->common.scope->name, call->fbc->common.function_name); } + + while (top_ce->parent) top_ce = top_ce->parent; + + /* if calling scope and target scope share the same parent, do not show warning */ + if (!instanceof_function(Z_OBJCE_P(EG(This)), top_ce TSRMLS_CC)) + zend_error(E_DEPRECATED, "Non-static method %s::%s() should not be called statically, assuming $this from incompatible context", call->fbc->common.scope->name, call->fbc->common.function_name); + } } if ((call->object = EG(This))) { Z_ADDREF_P(call->object);