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

Support for php8 #2111

Closed
mruz opened this issue Aug 14, 2020 · 36 comments · Fixed by #2171
Closed

Support for php8 #2111

mruz opened this issue Aug 14, 2020 · 36 comments · Fixed by #2171
Assignees
Labels
nfr New Feature Request
Milestone

Comments

@mruz
Copy link
Contributor

mruz commented Aug 14, 2020

Add php 8.0 support. We have feature freeze and beta1 now https://wiki.php.net/todo/php80.

It's disabled in composer/build. Can we enable (with allowed failures) and see what's failing?

Would be good if it's ready on Nov 26 2020 (GA).

@Jurigag
Copy link
Contributor

Jurigag commented Aug 16, 2020

Yea i guess we will need to prepare stuff for it. I guess feel free anyone to figure out what needs to be done and use open collective funds for it.

@dreamsxin would have any time to at least do any review how lot of work this would require?

@Jurigag Jurigag added the nfr New Feature Request label Aug 16, 2020
@Jurigag Jurigag pinned this issue Aug 16, 2020
@dreamsxin
Copy link
Contributor

dreamsxin commented Aug 17, 2020

Changes:

Arg info must be defined.
Some zend api parameter type changed: zval -> zend_ object.

Optimistic estimation: 2 weeks.

@Jeckerson
Copy link
Member

Screenshot_20210125_130442

Work in progress.

@mruz
Copy link
Contributor Author

mruz commented Jan 26, 2021

I'm receiving SIGABRT on kernel/fcall https://travis-ci.org/github/ice/framework/jobs/756221697

class Di extends Arr
{
    protected static di;

    public function __construct(array data = [])
    {
        parent::__construct(data);

        let self::di = this;
    }
class Arr implements \ArrayAccess, \Countable, \IteratorAggregate
{
    protected data = [] { get };

    public function __construct(array data = [])
    {
        let this->data = data;
    }
Program received signal SIGABRT, Aborted.
0x00007ffff26a8428 in raise () from /lib/x86_64-linux-gnu/libc.so.6
Thread 1 (Thread 0x7ffff7fc7ac0 (LWP 28558)):
...
#5  0x000000000085521d in zval_get_string_func (op=<optimized out>) at /tmp/php-build/source/8.0.1/Zend/zend_operators.c:932
#6  0x0000000000863f0e in zend_get_callable_name_ex (callable=<optimized out>, callable@entry=0x7fffffff98b8, object=<optimized out>) at /tmp/php-build/source/8.0.1/Zend/zend_API.c:3299
#7  0x000000000084f704 in zend_call_function (fci=fci@entry=0x7fffffff98b0, fci_cache=fci_cache@entry=0x7fffffff9890) at /tmp/php-build/source/8.0.1/Zend/zend_execute_API.c:717
#8  0x00007fffebaf61bd in zephir_call_user_function (object_pp=object_pp@entry=0x7fffec218260, obj_ce=obj_ce@entry=0x193d8c0, type=type@entry=zephir_fcall_parent, function_name=function_name@entry=0x7fffffff9aa0, retval_ptr=retval_ptr@entry=0x0, cache_entry=cache_entry@entry=0x7fffffff9b20, cache_slot=0, param_count=1, params=0x7fffffff9b40) at /home/travis/build/ice/framework/ext/kernel/fcall.c:386
#9  0x00007fffebaf67f2 in zephir_call_class_method_aparams (return_value=return_value@entry=0x0, ce=0x193d8c0, type=type@entry=zephir_fcall_parent, object=0x7fffec218260, method_name=method_name@entry=0x7fffebc6c5fa "__construct", method_len=method_len@entry=11, cache_entry=0x7fffffff9b20, cache_slot=0, param_count=1, params=0x7fffffff9b40) at /home/travis/build/ice/framework/ext/kernel/fcall.c:524
#10 0x00007fffebb8d287 in zim_Ice_Di___construct (execute_data=0x7fffec218240, return_value=0x7fffffff9bb0) at /home/travis/build/ice/framework/ext/ice/di.zep.c:94
#11 0x00000000008cd945 in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER () at /tmp/php-build/source/8.0.1/Zend/zend_vm_execute.h:1755

@Jeckerson
Copy link
Member

@mruz Might be due static, known bug atm.

@Jeckerson
Copy link
Member

@dreamsxin Currently there are x2 problems:

php -d extension=ext/modules/stub.so vendor/bin/simple-phpunit --debug

Problem 1 - Only occur when execute whole tests

Test 'Extension\ScallTest::testShouldReturnInterpolatedMethodFromZephir' started
Test 'Extension\ScallTest::testShouldReturnInterpolatedMethodFromZephir' ended
Test 'Extension\ScallTest::testShouldEchoInterpolatedMethodFromZephir' started
Test 'Extension\ScallTest::testShouldEchoInterpolatedMethodFromZephir' ended
Test 'Extension\ScopeTest::testScope1' started
Segmentation fault (core dumped)

Problem 2

$obj12 = $test->testInstance12();
$this->assertIsObject($obj12);
$this->assertInstanceOf('Stub\Oo\OoDynamicA', $obj12);
Test 'Extension\OoTest::testAssertations' started
Illegal instruction (core dumped)

@dreamsxin dreamsxin mentioned this issue Jan 27, 2021
3 tasks
@Jeckerson
Copy link
Member

Jeckerson commented Jan 27, 2021

@dreamsxin @AlexNDRmac

Bugs

1 - Wrong constructor pick during extension (very strange bug) - Constructor is picked from latest class where is called and extended, in that case TestCase class

Extension\Oo\ExtendClassTest::testPDOStatementExtending
TypeError: PHPUnit\Framework\TestCase::__construct(): Argument #2 ($data) must be of type array, string given
/srv/tests/Extension/Oo/ExtendClassTest.php:38

2 - Static methods and its visibility - Static methods that are called not directly Class::method, but from inside another method. And if static method is protected?/private

Extension\Oo\ExtendClassTest::testShouldCallParentMethodFromStaticByUsingSelf
RuntimeException: Call to undefined method self::parentfunction()

/srv/tests/Extension/Oo/ExtendClassTest.php:76
Extension\OoTest::testAssertations
RuntimeException: Call to undefined method self::call2()

/srv/tests/Extension/OoTest.php:70
Extension\ScallTest::testScall
RuntimeException: Trying to call method testmethod1 on a non-object

/srv/tests/Extension/ScallTest.php:32
Extension\ScopeTest::testScope1
RuntimeException: Call to undefined method self::getstr()

/srv/tests/Extension/ScopeTest.php:27

3 - Float (double) strict data type detection

Extension\Oo\OoParamsStrictTest::testShouldThrowInvalidArgumentExceptionForDouble
Failed asserting that exception of type "InvalidArgumentException" matches expected exception "Error". Message was: "Parameter 'average' must be of the type double" at
/srv/tests/Extension/Oo/OoParamsStrictTest.php:73

@AlexNDRmac I temporary disabled skipped tests for CI (in case some new PRs) - be6d13e


image

@dreamsxin
Copy link
Contributor

dreamsxin commented Jan 28, 2021

@Jeckerson

#if PHP_VERSION_ID >= 70300
static zend_never_inline zend_function *phalcon_get_function(zend_class_entry *scope, zend_string *function_name)
{
	zval *func;
	zend_function *fbc;

	func = zend_hash_find(&scope->function_table, function_name);
	if (func != NULL) {
		fbc = Z_FUNC_P(func);
		return fbc;
	}

	return NULL;
}

#if PHP_VERSION_ID >= 80000
static zend_result phalcon_call_user_function(zend_function *fn, zend_class_entry *called_scope, zval *object, zval *function_name, zval *retval_ptr, uint32_t param_count, zval params[])
#else
static int phalcon_call_user_function(zend_function *fn, zend_class_entry *called_scope, zval *object, zval *function_name, zval *retval_ptr, uint32_t param_count, zval params[])
#endif
{
	zend_fcall_info fci;
	zend_fcall_info_cache fcic;

	fci.size = sizeof(fci);
	fci.object = object ? Z_OBJ_P(object) : NULL;
	ZVAL_COPY_VALUE(&fci.function_name, function_name);
	fci.retval = retval_ptr;
	fci.param_count = param_count;
	fci.params = params;
#if PHP_VERSION_ID >= 80000
	fci.named_params = NULL;
#else
	fci.no_separation = 1;
#endif
	if (fn != NULL) {
		fcic.function_handler = fn;
		fcic.object = object ? Z_OBJ_P(object) : NULL;
		fcic.called_scope = called_scope;

		return zend_call_function(&fci, &fcic);
	}
	return zend_call_function(&fci, NULL);
}
#endif

@Jeckerson Jeckerson linked a pull request Jan 28, 2021 that will close this issue
3 tasks
@Jeckerson Jeckerson removed a link to a pull request Jan 28, 2021
3 tasks
@Jeckerson
Copy link
Member

Jeckerson commented Jan 28, 2021

@dreamsxin ZVAL_COPY_VALUE(&fci.function_name, function_name); doesn't seem to have any effect.

https://github.com/phalcon/zephir/blob/ed8634978d08d43f4ec02c9f304d204c87ec652b/kernels/ZendEngine3/fcall.c#L289

I think this function itself need to be refactored.

@Jeckerson
Copy link
Member

@dreamsxin Could you also check another errors? To not focus on same error together.

@dreamsxin
Copy link
Contributor

dreamsxin commented Jan 29, 2021

@Jeckerson All self/parent use zephir_call_ce_static replace, We can use zend_function:

ZEND_HOT int phalcon_call_user_method(zend_object *obj, zend_function* fbc, int num_arg, zval *args, zval *ret) /* {{{ */ {
	uint32_t i, call_info;
	zend_execute_data *call;

	if (UNEXPECTED(fbc->common.fn_flags & (ZEND_ACC_PROTECTED|ZEND_ACC_PRIVATE))) {
		php_error_docref(NULL, E_WARNING, "cannot call %s method %s::%s()", 
				(fbc->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED)) == ZEND_ACC_PROTECTED?
				"protected" : "private", ZSTR_VAL(obj->ce->name), ZSTR_VAL(fbc->common.function_name));
		return 0;
	}

#if PHP_VERSION_ID < 70400
	call_info = ZEND_CALL_TOP_FUNCTION;
	call = zend_vm_stack_push_call_frame(call_info, fbc, num_arg, NULL, obj);
#else
	call_info = ZEND_CALL_TOP_FUNCTION | ZEND_CALL_HAS_THIS;
	call = zend_vm_stack_push_call_frame(call_info, fbc, num_arg, obj);
#endif
	call->symbol_table = NULL;

	for (i = 0; i < num_arg; i++) {
		ZVAL_COPY(ZEND_CALL_ARG(call, i+1), &args[i]);
	}

	/* At least we should in calls of Dispatchers */
	ZEND_ASSERT(EG(current_execute_data));

	if (EXPECTED(fbc->type == ZEND_USER_FUNCTION)) {
		return phalcon_do_call_user_method(call, fbc, ret);
	} else {
		ZEND_ASSERT(fbc->type == ZEND_INTERNAL_FUNCTION);
		call->prev_execute_data = EG(current_execute_data);
		EG(current_execute_data) = call;
		if (EXPECTED(zend_execute_internal == NULL)) {
			fbc->internal_function.handler(call, ret);
		} else {
			zend_execute_internal(call, ret);
		}
		EG(current_execute_data) = call->prev_execute_data;
		zend_vm_stack_free_args(call);

		zend_vm_stack_free_call_frame(call);
		if (UNEXPECTED(EG(exception))) {
			/* We should return directly to user codes */
			ZVAL_UNDEF(ret);
			return 0;
		}
		return 1;
	}
}

@Jeckerson
Copy link
Member

@dreamsxin Could you please specify exactly where?

@dreamsxin
Copy link
Contributor

dreamsxin commented Feb 1, 2021

@Jeckerson
Call to undefined method self::parentfunction()
Simple solution use ZEPHIR_CALL_FUNCTION replace:

ZEPHIR_CALL_FUNCTION(return_value, "self::parentfunction", NULL, 0);
# or
ZEPHIR_CALL_FUNCTION(return_value, "parent::parentfunction", NULL, 0);

TypeError: PHPUnit\Framework\TestCase::__construct(): Argument #2 ($data) must be of type array, string given

ZEPHIR_CALL_PARENT(NULL, stub_oo_extendpdoclass_ce, getThis(), "__construct", NULL, 0, &dsn, &username, &password, attrs);
...
// fcall.c
			case zephir_fcall_parent:
				assert(ce);
				zend_string_addref(ce->parent->name);
				ZVAL_STR(&q, ce->parent->name);
				ZEND_HASH_FILL_ADD(&q);
				break;

@Jeckerson
Copy link
Member

By changing from:

zend_string_addref(i_parent);
ZVAL_STR(&q, i_parent);

to:

zend_string_addref(ce->parent->name);
ZVAL_STR(&q, ce->parent->name);

Gives Segmentation fault

@AlexNDRmac
Copy link
Member

Thanks @dreamsxin I'll try to replace this function call

@mruz
Copy link
Contributor Author

mruz commented Feb 1, 2021

I still have issues with constructor #2111 (comment)

PHP Warning: Invalid callback parent::__construct, cannot access "parent" when current class scope has no parent
RuntimeException: Call to undefined method parent::__construct()

@Jeckerson
Copy link
Member

@mruz try with 0.13.0-beta-1

@mruz
Copy link
Contributor Author

mruz commented Mar 8, 2021

It compiled, a few of my tests failed, so I had to fix some type errors on my side.

@Jeckerson
Copy link
Member

@mruz So all worked in general?

@mruz
Copy link
Contributor Author

mruz commented Mar 8, 2021

Yes, all good 🎉
Thanks @Jeckerson!

@ufoproger
Copy link
Contributor

ufoproger commented Mar 10, 2021

Hello!

I found a bug in the zephir-php8 branch related to detecting class name entry.

For example, the following code:

// File is located at /srv/stub/bug/a/b/param.zep

namespace Stub\Bug\A\B;

class Param
{
	public value;
}

// File is located at /srv/stub/bug/myclass.zep

namespace Stub\Bug;

class MyClass
{
	public function setParam(<\Stub\Bug\A\B\Param> parameter)
	{
		return parameter;
	}
}

Command zephir compile breaks with error:

/srv/ext/stub/bug/myclass.zep.c:36:38: error: 'stub_bug_param_ce' undeclared (first use in this function); did you mean 'stub_oo_param_ce'?
   Z_PARAM_OBJECT_OF_CLASS(parameter, stub_bug_param_ce)
                                      ^~~~~~~~~~~~~~~~~

The generated file /srv/ext/stub/bug/myclass.zep.c should look like this:

 ...

 ZEPHIR_INIT_CLASS(Stub_Bug_MyClass) {
 	ZEPHIR_REGISTER_CLASS(Stub\\Bug, MyClass, stub, bug_myclass, stub_bug_myclass_method_entry, 0);
 	return SUCCESS;
 }

 PHP_METHOD(Stub_Bug_MyClass, setParam) {
 	zval *parameter, parameter_sub;
 	zval *this_ptr = getThis();
 	ZVAL_UNDEF(&parameter_sub);
 
 #if PHP_VERSION_ID >= 80000
 	bool is_null_true = 1;
 	ZEND_PARSE_PARAMETERS_START(1, 1)
-		Z_PARAM_OBJECT_OF_CLASS(parameter, stub_bug_param_ce)
+		Z_PARAM_OBJECT_OF_CLASS(parameter, stub_bug_a_b_param_ce)
 	ZEND_PARSE_PARAMETERS_END();
 #endif
 
 	zephir_fetch_params_without_memory_grow(1, 0, &parameter);
 	RETVAL_ZVAL(parameter, 1, 0);
 	return;
 }

Current solution is to import class with the use statement:

namespace Stub\Bug;

use Stub\Bug\A\B\Param;

class MyClass
{
	public function setParam(<Param> parameter)
	{
		return parameter;
	}
}

@Jeckerson
Copy link
Member

Related #2165

@Jeckerson
Copy link
Member

Added support for PHP8.0 in v0.13.0

@Jeckerson Jeckerson linked a pull request Mar 26, 2021 that will close this issue
@Jeckerson Jeckerson modified the milestones: 0.13.x, 0.13.0 Mar 29, 2021
@Jeckerson Jeckerson unpinned this issue Apr 17, 2021
@fakharak
Copy link

It appears from this thread that Zephir is officially ready for PHP 8.0 ? If i am not wrong, can you also reflect that on official documentation of Zephir ? like on installation page, where version is shown as 7.3.7

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

Successfully merging a pull request may close this issue.

7 participants