Skip to content

Time\Duration - #23073

Draft
TimWolla wants to merge 5 commits into
php:masterfrom
TimWolla:time-duration
Draft

Time\Duration#23073
TimWolla wants to merge 5 commits into
php:masterfrom
TimWolla:time-duration

Conversation

@TimWolla

@TimWolla TimWolla commented Aug 5, 2026

Copy link
Copy Markdown
Member

Comment thread ext/date/config0.m4 Outdated

PHP_NEW_EXTENSION([date],
[php_date.c],
[php_date.c php_date_time.c php_date_time_duration.c],

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.

I was hoping to get rid of the php_date prefix for all of the new stuff. We don't really need it, and it's inconsistent with all other extensions (something I got wrong in 2004). My intention was to move it all to just the class name (with namespace), such as:

  • date_time_immutable.c (For DateTimeImmutable)
  • time_duration.c (For Time/Duration)

It also helps auto-complete in the file browser opening files.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've renamed php_date_time_duration.c to just time_duration.c and php_date_time.[ch] to php_time.[ch].

Just time.[ch] didn't work, because I assume something is erroneously including the system-wise time.h header with "" instead of <> or something. At least I got weird compiler errors I didn't want to investigate.

Comment thread ext/date/time_duration.c

static void throw_out_of_range_exception(void)
{
zend_throw_exception(php_date_ce_time_timeexception, "The maximum representable range is 9_223_372_035 seconds (roughly 292 years)", 0);

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.

There is the timelib_get_error_message() function that returns a string for each of the TIMELIB_ERROR codes. We can tweak these message, in case you don't think they're good enough for PHP.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I needed a custom implementation anyway for 32-bit PHP and to handle the overflows in the fromMinutes() and fromHours() constructors, so I would leave this as a special case.

Comment thread ext/date/php_date_time_duration.c Outdated
Comment thread ext/date/time_duration.c Outdated
Comment thread ext/date/php_date_time_duration.c Outdated
Comment thread ext/date/time_duration.c Outdated
Comment thread ext/date/tests/time/duration/fromNanoseconds_64.phpt Outdated
Comment thread ext/date/php_date_time_duration.c Outdated
Comment thread ext/date/time_duration.c
Comment thread ext/date/time_duration.c Outdated
object_init_ex(return_value, php_date_ce_time_duration);

php_date_time_duration *original = Z_DATE_TIME_DURATION_P(ZEND_THIS);
php_date_time_duration *additional = php_date_time_duration_from_obj(duration);

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.

Suggested change
php_date_time_duration *additional = php_date_time_duration_from_obj(duration);
php_date_time_duration *addend = php_date_time_duration_from_obj(duration);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was (re)using the timelib naming here, which has:

int timelib_duration_add_static(
	timelib_duration       *new_duration,
	const timelib_duration *original,
	const timelib_duration *additional
)

otherwise I also get a useless parameter name hint in VS Code:

image

Comment thread ext/date/php_date_time_duration.c Outdated

php_date_time_duration *original = Z_DATE_TIME_DURATION_P(ZEND_THIS);
php_date_time_duration *additional = php_date_time_duration_from_obj(duration);
php_date_time_duration *new = Z_DATE_TIME_DURATION_P(return_value);

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.

When either original or additional has recount==1, it may be possible to reuse it?

Comment thread ext/date/time_duration.c Outdated
ZEND_STATIC_ASSERT(MICROS_IN_NANOS * MICROS_IN_SEC == NANOS_IN_SEC, "");
ZEND_STATIC_ASSERT(MILLIS_IN_NANOS * MILLIS_IN_SEC == NANOS_IN_SEC, "");

static void sync_properties(php_date_time_duration *object)

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.

Could we avoid this with hooks?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Before implementing this I had a quick chat with @iluuu1994 about this:

  • The class is specified to be readonly in the RFC and this needs to be specified in the stub to be visible to Reflection.
  • Hooks are semantically not legal on readonly properties. Marking the property as @virtual in the stub would be visible to reflection.
  • Also marking a property as @virtual would require me to manually implement read_property(), get_properties(), write_property(), and get_property_ptr_ptr() to obtain the correct behavior.

Since there is no such thing as a “internal only hook” for a property as far as I’m aware and given the upcoming freeze, I opted to materialize this into real properties for the initial version. This can (and probably should) be optimized with PHP 8.7.

@TimWolla
TimWolla force-pushed the time-duration branch 8 times, most recently from ebb8107 to 5f203d8 Compare August 5, 2026 20:09
@TimWolla TimWolla added the RFC label Aug 5, 2026
@TimWolla
TimWolla requested review from arnaud-lb and derickr August 5, 2026 20:16
@TimWolla
TimWolla force-pushed the time-duration branch 2 times, most recently from 89e45ab to 6390166 Compare August 5, 2026 20:43
Comment thread ext/date/tests/time/duration/methods/add.phpt Outdated
Comment thread ext/date/tests/time/duration/methods/div.phpt Outdated
Comment thread ext/date/tests/time/duration/methods/mul.phpt Outdated
Comment thread ext/date/php_time.c
Comment thread ext/date/php_time.c Outdated
Comment thread ext/date/php_time.h Outdated
Comment thread ext/date/php_time.h Outdated
@TimWolla
TimWolla force-pushed the time-duration branch 2 times, most recently from 074da03 to 1f0ff1f Compare August 6, 2026 07:38
TimWolla and others added 3 commits August 6, 2026 10:23
…om*()`

This is useful for patterns like the following:

    for (;;) {
        $watchers = $poll->wait(Time\Duration::fromSeconds(1));
        // …
    }

which is repeatedly creating identical duration objects for every loop
iteration.
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