Please do open PR(s) for the other branches ...

@jhdxr @krakjoe seems like php_error_docref is still in PHP-7.1+ branches
https://github.com/php/php-src/blob/PHP-7.1/main/php.h#L329

I haven't read the patch in detail, only done triage ... if other patches are needed, they need to exist before merge can happen is all I'm saying at this point ...

@andrewnester I guess the php_error_docref is still there just because of the BC for other extensions, like those in PECL. the code inside this repo has been replace since this commit: 771e5cc

@krakjoe ok, I'll ping you after I've provided all the necessary patches.

@jhdxr not sure that php_error_docref is left just because of BC.
zend_throw_error and php_error_docref have different purposes as far as I remember - one is to use it inside Zend code, the second - in extensions.
I also tried to search for internals discussion about deprecation of php_error_docref - haven't found anything

zend_throw_error throws an Error exception. You're probably thinking about zend_error, which is kinda similar to php_error_docref (but well, without the "docref" part). In any case, none of those are deprecated.

@krakjoe I'm all set.
this one for 7.0
#2685 for 7.1
#2686 for 7.2 and master.

@derickr Are any properties supposed to be exposed on DateTimeZone or is this just the usual get_properties side effect?

If DateTimeZone is not intended to expose properties, then the proper way to fix the comparison issue is by implementing a comparison handler.

@nikic I took implementing compare_objects into consideration, but the problem is how to define the rules of greater than and less than for DateTimeZone.

In fact, there are lots of issues on bugs.php.net of date extension which are caused by the same reason, get_properties updates the inner property, and leads to inconsistent behaviors.

I took implementing compare_objects into consideration, but the problem is how to define the rules of greater than and less than for DateTimeZone.

This patch doesn't solve that though, it just ignores it. Now it will use the default object property comparison semantics, which most likely will not result in any sensible behavior.

The "standard" way of handling non-trichotomous comparisons in PHP to is always return "greater" for non-equal comparisons. Together with the fact that the "$a > $b" comparison is defined as "$b < $a", this means that all non-equality comparisons return false. This is not perfect, but the only possibility we really have right now.

In fact, there are lots of issues on bugs.php.net of date extension which are caused by the same reason, get_properties updates the inner property, and leads to inconsistent behaviors.

Indeed. This is a long standing issue caused by the lack of specific object handlers for serialization (and in the case of DateTime objects, unfortunately also var_exporting).

The "standard" way of handling non-trichotomous comparisons in PHP to is always return "greater" for non-equal comparisons.

I suggest to add this to https://wiki.php.net/internals/engine/objects#compare_objects. I cannot find an implementation meeting all the requirements, so I just use the default handler. And yes, I know it doesn't make sense for DateTimeZone.

Indeed. This is a long standing issue caused by the lack of specific object handlers for serialization (and in the case of DateTime objects, unfortunately also var_exporting).

This is a point people usually used to blame php. IMHO, we should put our priority on standards. If we had a standard of the internal namespace, the arguments that sodium should use namespace or not wouldn't happen. In addtion, it also bring the possibilty to standlize and clean up the existed functions. There are many things we can do without BC break.

Back to the issue here, I can use compare_objects to fix #74940, and use get_debug_info to fix #68406. But I think it doesn't fix the root cause of these problem. An method named getXXX are supposed to do, and only do retrive. Maybe I should send an email to internal see other people's idea on these object handlers, and then someone may start an RFC to standlize these things.

@nikic These properties should only be exposed through the debug handler. As they are read only and meant for debugging only.

@jhdxr Greater than should probably done on:

1. timezone_type

And then depending on type:

1. UTC offset
2. UTC offset (plus an extra hour if the DST flag is set)
3. Time Zone Identifier

@derickr ok. I'll create a new pr to fix 68406 by implementing get_debug_info, since the properties are not supoosed to be exposed. However, I don't think the rules of greater than you provided make sense. Shouldn't we put offset at the first priority? or maybe you think there is no meaningful compare rule, and we just need a consistent rule?

anyway, I'm closing #2685 and #2686, and leave this one open for discussion.

#2766 stil isn't merged.

And just spent an hour finding out this is also a problem in DateInterval and DatePeriod:

DateInterval: https://3v4l.org/3FaWi
DatePeriod: https://3v4l.org/LBOMs

And already forgot about this issue...

abandoned