Skip to content

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Mar 2, 2020

Since DateTimeZone does not implement a compare_objects handler,
nor has any properties, two DateTimeZone instances always compare as
being equal, even if they designate totally different timezones. Even
worse, after calling var_dump() on these objects, the actual
comparison may yield a correct result.

We therefore introduce a compare_objects handlers, which prevents
different behavior before/after var_dump(), and which allows us to
clearly define the intended semantics.


Actually this is not something that should go into PHP-7.4 for BC reasons, but the current behavior is so wrong, that we might consider it nonetheless.

For now I implemented most basic comparison semantics, basically what happens right now after the objects have been var_dump()ed.

Since `DateTimeZone` does not implement a `compare_objects` handler,
nor has any properties, two `DateTimeZone` instances always compare as
being equal, even if they designate totally different timezones.  Even
worse, after calling `var_dump()` on these objects, the actual
comparison may yield a correct result.

We therefore introduce a `compare_objects` handlers, which prevents
different behavior before/after `var_dump()`, and which allows us to
clearly define the intended semantics.
@cmb69 cmb69 added the Bug label Mar 3, 2020
@cmb69
Copy link
Member Author

cmb69 commented Mar 3, 2020

@derickr what do you think about this?

This is not supposed to happen, so actually checking at run-time is
superfluous.
@cmb69
Copy link
Member Author

cmb69 commented Mar 27, 2020

@derickr is this good to be merged? PHP-7.4 or master?

@derickr
Copy link
Member

derickr commented Mar 27, 2020

Go for it, and IMO 7.4 is fine.

@cmb69
Copy link
Member Author

cmb69 commented Mar 30, 2020

Thanks! Applied as a2f8c78.

@cmb69 cmb69 closed this Mar 30, 2020
@cmb69 cmb69 deleted the cmb/74940 branch March 30, 2020 07:20
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.

2 participants