Skip to content

fake closure comparison #7223

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

fake closure comparison #7223

wants to merge 9 commits into from

Conversation

krakjoe
Copy link
Member

@krakjoe krakjoe commented Jul 8, 2021 •
edited
Loading

@krakjoe krakjoe added the Feature label Jul 8, 2021
nikic
nikic reviewed Jul 8, 2021
View reviewed changes
patrickallaert
patrickallaert reviewed Jul 8, 2021
View reviewed changes
@krakjoe krakjoe force-pushed the fake-closure-compare branch 6 times, most recently from 7c3a7b9 to 9474ae5 Compare July 8, 2021 21:04
krakjoe
krakjoe commented Jul 8, 2021
View reviewed changes
bwoebi
bwoebi reviewed Jul 8, 2021
View reviewed changes
@krakjoe krakjoe force-pushed the fake-closure-compare branch 4 times, most recently from 31f6efa to 6e32816 Compare July 8, 2021 21:24
@krakjoe krakjoe force-pushed the fake-closure-compare branch from 6e32816 to d600213 Compare July 9, 2021 07:02
@krakjoe krakjoe requested review from nikic and bwoebi July 9, 2021 07:04
nikic
nikic reviewed Jul 9, 2021
View reviewed changes
nikic
nikic reviewed Jul 9, 2021
View reviewed changes
nikic
nikic reviewed Jul 9, 2021
View reviewed changes
@nikic
Copy link
Member

nikic commented Jul 9, 2021

Implementation looks okay, but I think this will need a mail to internals.

TysonAndre
TysonAndre previously requested changes Jul 9, 2021
View reviewed changes
Copy link
Contributor

@TysonAndre TysonAndre left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think $closure == $closure should always return true for the same object.

Also, this should test the expected behavior of closures with static variables

EDIT: Never mind, PHP's == uses zend_compare_objects, which has that check already

@nikic nikic added this to the PHP 8.1 milestone Jul 12, 2021
@krakjoe
Copy link
Member Author

krakjoe commented Jul 13, 2021

Merged as 6a9daaf

@krakjoe krakjoe closed this Jul 13, 2021
@krakjoe krakjoe deleted the fake-closure-compare branch July 13, 2021 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TysonAndre TysonAndre TysonAndre left review comments

@patrickallaert patrickallaert patrickallaert approved these changes

@nikic nikic nikic approved these changes

@bwoebi bwoebi Awaiting requested review from bwoebi

Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
PHP 8.1
Development

Successfully merging this pull request may close these issues.
5 participants
@krakjoe @nikic @patrickallaert @TysonAndre @bwoebi