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

Declare tentative return types for ext/spl - part 1 #7115

Merged
merged 5 commits into from
Jul 13, 2021

Conversation

kocsismate
Copy link
Member

No description provided.

@kocsismate kocsismate added the Tentative returns Temporary label for tentative return follow ups label Jun 7, 2021
@nikic
Copy link
Member

nikic commented Jun 8, 2021

ext/phar/tests/phar_oo_008.phpt fails

@kocsismate
Copy link
Member Author

ext/phar/tests/phar_oo_008.phpt fails

Yeah, I haven't been able to find out how to solve the issue yet, but I'l look into it again a bit later.

@kocsismate
Copy link
Member Author

ext/phar/tests/phar_oo_008.phpt fails

I've just fixed the test, the original return value simply didn't make sense.

@kocsismate
Copy link
Member Author

@nikic Can you please review this sometime soon?

ext/spl/spl_array.stub.php Outdated Show resolved Hide resolved
ext/spl/tests/iterator_002.phpt Show resolved Hide resolved
ext/spl/tests/recursivedualiterator.inc Outdated Show resolved Hide resolved
ext/spl/spl_directory.stub.php Outdated Show resolved Hide resolved
ext/spl/spl_directory.stub.php Outdated Show resolved Hide resolved
ext/spl/spl_directory.stub.php Outdated Show resolved Hide resolved
ext/spl/spl_directory.stub.php Outdated Show resolved Hide resolved
ext/spl/spl_directory.stub.php Show resolved Hide resolved
ext/spl/spl_directory.stub.php Show resolved Hide resolved
ext/spl/spl_directory.stub.php Outdated Show resolved Hide resolved
@nikic nikic added this to the PHP 8.1 milestone Jul 12, 2021
@nikic
Copy link
Member

nikic commented Jul 12, 2021

ext/phar/tests/phar_oo_008.phpt failing again.

@kocsismate
Copy link
Member Author

ext/phar/tests/phar_oo_008.phpt failing again.

Yeah, saw that, but the fix required more braincells than what I could dedicate during a working day :D So it's fixed now.

ext/spl/tests/bug66405.phpt Outdated Show resolved Hide resolved
@kocsismate kocsismate merged commit c6357b8 into php:master Jul 13, 2021
@kocsismate kocsismate deleted the tentative-return-type-21 branch July 13, 2021 11:04
@TysonAndre
Copy link
Contributor

TysonAndre commented Jul 19, 2021

I'm seeing crashes for union types in tentative return types. It's possibly not setting the union type flag, I haven't looked further

I'm using a build that merges the patches for the readonly properties RFC, hopefully unrelated

    /** @tentative-return-type */
    public function current(): string|SplFileInfo|FilesystemIterator {}
» php -a
Interactive shell

php > $x = new ReflectionMethod('FileSystemIterator::current');
php > var_export($x->getTentativeReturnType());
php: /path/to/php-src/ext/reflection/php_reflection.c:1334: get_type_kind: Assertion `((((type).type_mask) & (1u << 18)) != 0)' failed.
[1]    149925 abort (core dumped)  php -a
» php --version
PHP 8.1.0-dev (cli) (built: Jul 19 2021 10:07:35) (NTS DEBUG)
Copyright (c) The PHP Group
Zend Engine v4.1.0-dev, Copyright (c) Zend Technologies
php: /path/to/php-src/ext/reflection/php_reflection.c:1334: get_type_kind: Assertion `((((type).type_mask) & (1u << 18)) != 0)' failed.

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff66e5859 in __GI_abort () at abort.c:79
#2  0x00007ffff66e5729 in __assert_fail_base (fmt=0x7ffff687b588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x5555569cce30 "((((type).type_mask) & (1u << 18)) != 0)", 
    file=0x5555569cc6e8 "/path/to/php-src/ext/reflection/php_reflection.c", line=1334, function=<optimized out>) at assert.c:92
#3  0x00007ffff66f6f36 in __GI___assert_fail (assertion=0x5555569cce30 "((((type).type_mask) & (1u << 18)) != 0)", file=0x5555569cc6e8 "/path/to/php-src/ext/reflection/php_reflection.c", line=1334, 
    function=0x5555569cddc8 <__PRETTY_FUNCTION__.21375> "get_type_kind") at assert.c:101
#4  0x0000555555b257c1 in get_type_kind (type=...) at /path/to/php-src/ext/reflection/php_reflection.c:1334
#5  0x0000555555b2585e in reflection_type_factory (type=..., object=0x7ffff3a13080, legacy_behavior=true) at /path/to/php-src/ext/reflection/php_reflection.c:1359
#6  0x0000555555b3041c in zim_ReflectionFunctionAbstract_getTentativeReturnType (execute_data=0x7ffff3a13100, return_value=0x7ffff3a13080) at /path/to/php-src/ext/reflection/php_reflection.c:3592
#7  0x0000555555e657a6 in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER () at /path/to/php-src/Zend/zend_vm_execute.h:1864
#8  0x0000555555ed6dee in execute_ex (ex=0x7ffff3a13020) at /path/to/php-src/Zend/zend_vm_execute.h:54507
#9  0x0000555555edc57a in zend_execute (op_array=0x7ffff3a5c140, return_value=0x7fffffffc3a0) at /path/to/php-src/Zend/zend_vm_execute.h:58834
#10 0x0000555555e10a9e in zend_eval_stringl (str=0x7ffff3a84000 "var_export($x->getTentativeReturnType());\n", str_len=42, retval_ptr=0x0, string_name=0x5555569cb46d "php shell code")
    at /path/to/php-src/Zend/zend_execute_API.c:1206
#11 0x0000555555b1e214 in readline_shell_run () at /path/to/php-src/ext/readline/readline_cli.c:690
#12 0x0000555555f97970 in do_cli (argc=2, argv=0x555556f87770) at /path/to/php-src/sapi/cli/php_cli.c:963
#13 0x0000555555f98a92 in main (argc=2, argv=0x555556f87770) at /path/to/php-src/sapi/cli/php_cli.c:1367

nikic added a commit that referenced this pull request Jul 19, 2021
@nikic
Copy link
Member

nikic commented Jul 19, 2021

@TysonAndre Thanks for the report, should be fixed by 084d49a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tentative returns Temporary label for tentative return follow ups
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants