Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[#79521] Check __set_state structure#5462

Closed
carusogabriel wants to merge 1 commit into
php:masterphp/php-src:masterfrom
carusogabriel:php8/set_state-staticCopy head branch name to clipboard
Closed

[#79521] Check __set_state structure#5462
carusogabriel wants to merge 1 commit into
php:masterphp/php-src:masterfrom
carusogabriel:php8/set_state-staticCopy head branch name to clipboard

Conversation

@carusogabriel
Copy link
Copy Markdown
Contributor

@carusogabriel carusogabriel commented Apr 26, 2020

Found while working on #4177. This fixes two problems:

Similar to #5441.

@carusogabriel carusogabriel changed the title Ensure declaration of __set_state modifiers Check __set_state structure Apr 26, 2020
@carusogabriel carusogabriel changed the title Check __set_state structure [#79521] Check __set_state structure Apr 26, 2020
Comment thread ext/date/tests/ExtendDateTime.phpt Outdated
@nikic
Copy link
Copy Markdown
Member

nikic commented Apr 27, 2020

Hm, I'm wondering if we should be checking max arg number on magic methods, e.g. there's https://bugs.php.net/bug.php?id=46225.

Maybe it makes sense to rewrite magic method validation in terms of validation against dummy signatures, which will then follow exact PHP signature validation semantics.

@carusogabriel
Copy link
Copy Markdown
Contributor Author

@nikic Thanks for sharing that feature request, indeed is something that we could change. I'll remove the check related to arguments and I'll come up with something regarding the dummy signatures validation

@carusogabriel carusogabriel requested a review from nikic April 27, 2020 14:30
Comment thread Zend/tests/magic_methods_set_state.phpt Outdated
@php-pulls php-pulls closed this in 34c460f Apr 28, 2020
@carusogabriel carusogabriel deleted the php8/set_state-static branch April 28, 2020 15:02
@carusogabriel carusogabriel added this to the PHP 8.0 milestone May 29, 2020
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 this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.