Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.2 deprecation warning: callables and self #2405

Closed
niden opened this issue Jul 24, 2023 · 13 comments · Fixed by #2424
Closed

PHP 8.2 deprecation warning: callables and self #2405

niden opened this issue Jul 24, 2023 · 13 comments · Fixed by #2424
Assignees
Labels

Comments

@niden
Copy link
Contributor

niden commented Jul 24, 2023

Related issue:

phalcon/cphalcon#16263

For PHP 8.2 when calling a static method we get a warning about using self in callables.

Example Phalcon\Tag

Tag::linkTo(....) will call internally let urlService = self::getUrlService() and that issues the warning.

@betrixed
Copy link

betrixed commented Aug 17, 2023

Encountered same issue. Made a small test extension to highlight the C code generation.
Consider this bug demo code scall.zep ---

 namespace Sc;

class scall {
	public static function func1(string s) -> string 
       {
	return s . "_";
       }
   /* calling self::  or static::   or scall:: produce the same C code */
	public static function func2(string s) -> string
 	{
            
		return scall::func1(s);
 	}
 }

The C code generated will contain the C-macro
ZEPHIR_RETURN_CALL_SELF("func1", NULL, 0, &s);

If instead , func1 is defined in another class, defined in swork.zep, and func2 is changed to call that instead
return swork::func1(s); such that --

class swork {
	public static function func1(string s) -> string 
	{
		return s . "_";
	}
}

The C code generated will contain the C-macro
ZEPHIR_RETURN_CALL_CE_STATIC(sc_swork_ce, "func1", &_0, 0, &s);

Need a little test.php script to call up the "calling self" deprecation message ---
Deprecation warnings need to be enabled --

With self::, scall:: or static:: invocation and ZEPHIR_RETURN_CALL_SELF macro,
the deprecation message occurs.
Calling another class static method, with generated ZEPHIR_RETURN_CALL_CE_STATIC macro,
no deprecation occurs.

This ZEPHIR_RETURN_CALL_SELF macro seems to be involved in generation of bogus looking optimisations involving caching of zend information. Scanning the involved C code gave me an immediate rejection headache. I would be happier if it just trusted the known zend class entry, and used that in all possible cases.

<?php
namespace Sc;

ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

echo scall::func2("Zephir is great") . PHP_EOL;

@betrixed
Copy link

Checking a bit further, if a static function call, calls itself recursively it also generates the same error message, this time through this macro - ZEPHIR_RETURN_CALL_STATIC("func1", NULL, 0, &_2$$4); Again the deprecation warning message can be avoided by substitution with ZEPHIR_RETURN_CALL_CE_STATIC

@rudiservo
Copy link

I think the issue is the zend api funtion "zend_is_callable_check_class" that throws the deprecation notice.

That is called by "zend_is_callable_at_frame" and "zend_is_callable_check_func"

"zephir_call_user_func_array_noex" calls "zend_id_callable_at_frame" but this is not the issue.

The issue is in ZEPHIR_CALL_SELF

So tracing it is
ZEPHIR_CALL_SELF -> zephir_call_class_method_aparams -> zephir_call_user_function -> zend_is_callable_at_frame passes fci.object;

So ZEPHIR_CALL_SELF and ZEPHIR_RETURN_CALL_SELF calls zephir_call_class_method_aparams in the 4th passed parameter has getThis().

I guess getThis returns self.

@betrixed gave the idea or ZEPHIR_RETURN_CALL_CE_STATIC, but i.e in Tag, zephir has to translate this
ZEPHIR_CALL_SELF(&_1$$7, "geturlservice", NULL, 0);
into

ZEPHIR_CALL_CE_STATIC(&_1$$7, phalcon_tag_ce, "geturlservice", &_0, 0);

Or, I am guessing that maybe something like this in the 2nd parameter could fix it?
(getThis() ? Z_OBJCE_P(getThis()) : NULL)

either that or we change self with Object::class?

@magroski
Copy link

Is this fixed in Phalcon 5.5?

@Jeckerson
Copy link
Member

Is this fixed in Phalcon 5.5?

Still on it.

@niden
Copy link
Contributor Author

niden commented Feb 7, 2024

Deprecation warnings removed in PHP 8.2+

@s-ohnishi
Copy link

Isn't it resolved only about callables, but not about self?

@Jeckerson
Copy link
Member

@s-ohnishi all 3: self, parent and static.

@fichtner
Copy link

fichtner commented Feb 8, 2024

That’s great news. Is a stable release planned addressing this?

@s-ohnishi
Copy link

Good news.
I can't wait for the release!

@ilker-capli
Copy link

Nice to see good results for this issue

@niden
Copy link
Contributor Author

niden commented Feb 8, 2024

@fichtner Releasing today

@magroski
Copy link

magroski commented Feb 8, 2024

excellent news

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants