[intl] Preserve PHP-side state when cloning formatters - #23652
Conversation
|
I'd suggest to add this test <?php
$m = new MessageFormatter('en_US', '{0,number} | {1,time,short} | {2,date,medium}');
$dt = new DateTimeImmutable('2026-01-02 03:04:05', new DateTimeZone('UTC'));
$m->format([1, $dt, $dt]);
$mc = clone $m;
$original = $m->format([1, $dt, $dt]);
$cloned = $mc->format([1, $dt, $dt]);
var_dump($mc->getPattern());
var_dump($original === $cloned);
?>Otherwise this looks good to me. |
c0bc482 to
30a6344
Compare
| - Intl: | ||
| . Fixed cloning IntlDateFormatter and MessageFormatter losing PHP-side state | ||
| such as dateType, timeType, calendar and the message pattern. (Ilia Alshanetsky) | ||
|
|
There was a problem hiding this comment.
nit: no need for new empty lines.
|
Ran it on unpatched 8.4: |
IntlDateFormatter_object_clone() left date_type, time_type, calendar and requested_locale at their constructor defaults and MessageFormatter_object_ clone() dropped orig_format/orig_format_len/tz_set, so a cloned formatter reported wrong types/calendar/pattern and lost the requested locale used by datefmt_set_calendar(). The clone handlers now copy these fields alongside the ICU handle; msgformat_data.arg_types is deliberately not copied since it is a lazily rebuilt cache derived from the cloned ICU formatter. Sibling audit: NumberFormatter, IntlCalendar, SpoofChecker and Transliterator carry no other PHP-side scalar state in their clone paths.
30a6344 to
1128af3
Compare
|
note I see your agent tends now to do [< extension >] rather than ext/< extension > for your pr titles. Might be preferable to conform to general style. |
Dang I actually like the |
Cloning IntlDateFormatter left dateType, timeType, calendar, and the requested locale at constructor defaults, and cloning MessageFormatter dropped its pattern. The clone handlers now copy those PHP-side fields with the ICU handle.