Skip to content

Add support for tentative return types of internal methods - #6971

Closed
kocsismate wants to merge 3 commits into
php:masterfrom
kocsismate:tentative-return-type-1
Closed

Add support for tentative return types of internal methods#6971
kocsismate wants to merge 3 commits into
php:masterfrom
kocsismate:tentative-return-type-1

Conversation

@kocsismate

@kocsismate kocsismate commented May 11, 2021

Copy link
Copy Markdown
Member

kocsismate
kocsismate commented May 11, 2021
View reviewed changes
Comment thread ext/date/php_date.stub.php
@kocsismate kocsismate added the RFC label May 11, 2021
nikic
nikic reviewed May 11, 2021
View reviewed changes
Comment thread Zend/tests/type_declarations/variance/internal_parent/internal_parent.phpt Outdated
Comment thread Zend/zend_attributes.c Outdated
Comment thread Zend/zend_compile.c Outdated
Comment thread Zend/zend_compile.c Outdated
Comment thread Zend/zend_compile.c Outdated
Comment thread Zend/zend_execute.c Outdated
Comment thread Zend/zend_inheritance.c Outdated
Comment thread ext/opcache/ZendAccelerator.c Outdated
Comment thread ext/opcache/ZendAccelerator.c Outdated
Comment thread Zend/Optimizer/zend_optimizer.c Outdated
@nikic

nikic commented May 11, 2021

Copy link
Copy Markdown
Member

Something you might want to consider is moving the tentative flag into ZEND_ACC_TENTATIVE_RETURN_TYPE -- at least I find the asymmetry between the two flags a bit odd.

@kocsismate
kocsismate force-pushed the tentative-return-type-1 branch from 99139e8 to b96379f Compare May 13, 2021 16:14
@kocsismate

kocsismate commented May 13, 2021

Copy link
Copy Markdown
Member Author

Something you might want to consider is moving the tentative flag into ZEND_ACC_TENTATIVE_RETURN_TYPE -- at least I find the asymmetry between the two flags a bit odd.

I've just searched for & ZEND_ACC_HAS_RETURN_TYPE, and the vast majority of these usages don't need special handling of tentative types, so I'm a bit hesitant to add the ZEND_ACC_TENTATIVE_RETURN_TYPE flag instead of ZEND_ACC_HAS_RETURN_TYPE. Do you think it would be actually useful to have this? If so, I'll add it!

Or I just misunderstood you?

nikic
nikic reviewed May 14, 2021
View reviewed changes

@nikic nikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking pretty good.

Comment thread Zend/Optimizer/zend_func_info.c Outdated
Comment thread Zend/Optimizer/zend_func_info.c Outdated
Comment thread Zend/zend_API.h Outdated
Comment thread ext/reflection/php_reflection.c Outdated
kocsismate added a commit that referenced this pull request May 14, 2021
@kocsismate
Add support for tentative return types of internal methods
532c60c 
RFC: https://wiki.php.net/rfc/internal_method_return_types

Closses GH-6971
@kocsismate

kocsismate commented May 14, 2021

Copy link
Copy Markdown
Member Author

Closed via 532c60c

@kocsismate kocsismate closed this May 14, 2021
@kocsismate
kocsismate deleted the tentative-return-type-1 branch May 14, 2021 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@nikic nikic nikic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

RFC

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kocsismate @nikic