Skip to content

[intl] Preserve PHP-side state when cloning formatters - #23652

Open
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/aph-intl-datefmt-clone-meta-2ey-84
Open

[intl] Preserve PHP-side state when cloning formatters#23652
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/aph-intl-datefmt-clone-meta-2ey-84

Conversation

@iliaal

@iliaal iliaal commented Sep 10, 2026

Copy link
Copy Markdown
Member

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.

@LamentXU123

LamentXU123 commented Sep 11, 2026

Copy link
Copy Markdown
Member

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.

@iliaal
iliaal force-pushed the fix/aph-intl-datefmt-clone-meta-2ey-84 branch from c0bc482 to 30a6344 Compare September 11, 2026 10:49

@LamentXU123 LamentXU123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Otherwise looks good.

Comment thread NEWS
- Intl:
. Fixed cloning IntlDateFormatter and MessageFormatter losing PHP-side state
such as dateType, timeType, calendar and the message pattern. (Ilia Alshanetsky)

@LamentXU123 LamentXU123 Sep 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: no need for new empty lines.

@iliaal

iliaal commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

Ran it on unpatched 8.4: getPattern() goes red, but $original === $cloned is already true there, so only the pattern half discriminates and the existing case covers that. Changing the default timezone between the two format() calls does discriminate, so I added that instead: without the tz_set copy the clone re-adopts the current zone and diverges from the original.

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.
@iliaal
iliaal force-pushed the fix/aph-intl-datefmt-clone-meta-2ey-84 branch from 30a6344 to 1128af3 Compare September 11, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants