Skip to content

Conversation

@kylekatarnls
Copy link
Contributor

@kylekatarnls kylekatarnls commented Aug 9, 2025

Follow-up #7191

Implementation for the RFC: https://wiki.php.net/rfc/clamp_v2 (see also https://wiki.php.net/rfc/clamp)

clamp(2, min: 1, max: 3) // 2
clamp(0, min: 1, max: 3) // 1
clamp(6, min: 1, max: 3) // 3
clamp(2, 1.3, 3.4) // 2
clamp(2.5, 1, 3) // 2.5
clamp(2.5, 1.3, 3.4) // 2.5
clamp(0, 1.3, 3.4) // 1.3
clamp(M_PI, -INF, INF) // 3.141592653589793
clamp(NAN, 4, 6) // NAN
clamp("a", "c", "g") // "c"
clamp("d", "c", "g") // "d"
clamp(new \DateTimeImmutable('2025-08-01'), new \DateTimeImmutable('2025-08-15'), new \DateTimeImmutable('2025-09-15'))->format('Y-m-d') // 2025-08-15
clamp(new \DateTimeImmutable('2025-08-20'), new \DateTimeImmutable('2025-08-15'), new \DateTimeImmutable('2025-09-15'))->format('Y-m-d') // 2025-08-20

clamp(4, 8, 6)
// Throws ValueError: clamp(): Argument #2 ($min) must be smaller than or equal to argument #3 ($max)

clamp(4, NAN, 6)
// Throws ValueError: clamp(): Argument #2 ($min) cannot be NAN

clamp(4, 6, NAN)
// Throws ValueError: clamp(): Argument #3 ($max) cannot be NAN

Documentation: php/doc-en#4814

@medabkari
Copy link

The RFC page status is set to Withdrawn, update it to Draft if you have access.

@kylekatarnls kylekatarnls force-pushed the feature/clamp branch 3 times, most recently from a499c4c to 408f386 Compare August 10, 2025 08:48
@kylekatarnls kylekatarnls marked this pull request as ready for review August 10, 2025 09:02
@kylekatarnls
Copy link
Contributor Author

kylekatarnls commented Aug 18, 2025

Hello @kocsismate and @bukka, any news about this proposal to reconsider clamp RFC?

@TimWolla
Copy link
Member

@kylekatarnls RFCs are discussed on the mailing list and then need to be voted on. See https://wiki.php.net/rfc/howto for an explanation of the process.

@kylekatarnls
Copy link
Contributor Author

Thanks. I'd like to participate there, but I failed to create a wiki account, the captcha stuff is asking me "To which email address do you have to send an email to now?" and re-reading the whole page, I tried internals@lists.php.net but it's not that, now, I've no idea what it's expecting from me. 😞 Life is hard for humans who failed the Turing test.

@TimWolla
Copy link
Member

I tried internals@lists.php.net but it's not that, now, I've no idea what it's expecting from me. 😞

That is accurate:

https://github.com/php/web-wiki/blob/0fbb083c530dfa960a73c8cca82193fe4004f00e/dokuwiki/lib/plugins/phpreg/action.php#L87

What error message did you receive?

@kylekatarnls
Copy link
Contributor Author

Thanks for pointing me to the source code of the wiki, my error was The user could not be created. which from source code can have multiple reasons, but mine was that I already have an account (probably created a decade ago 😄) All good now 👍

@kylekatarnls kylekatarnls force-pushed the feature/clamp branch 2 times, most recently from f973850 to b39d140 Compare November 19, 2025 07:45
@kylekatarnls kylekatarnls force-pushed the feature/clamp branch 2 times, most recently from 4606ae5 to 77b8827 Compare November 19, 2025 17:23
Z_FLF_PARAM_ZVAL(2, zmin);
Z_FLF_PARAM_ZVAL(3, zmax);

if (EXPECTED(Z_TYPE_P(zmin) == IS_DOUBLE) && UNEXPECTED(zend_isnan(Z_DVAL_P(zmin)))) {
Copy link
Member

Choose a reason for hiding this comment

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

The current code duplicates the code into the two different implementations.
It would be great to factor this out to a separate function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it all good? I used define assuming it's the most efficient way to share code without affecting runtime performances. Also is the position (just before the actual function) correct regarding conventions for the repository?

Copy link
Member

Choose a reason for hiding this comment

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

I used define assuming it's the most efficient way to share code without affecting runtime performances.

No define, please. Just use a static function for the shared logic and the compiler will figure out whether or not inlining is reasonable (or whether it can just be a tail call or whatever).

Also is the position (just before the actual function) correct regarding conventions for the repository?

Position is fine.

Copy link
Member

Choose a reason for hiding this comment

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

Fully agree to use a proper function instead of defines.
You can pass the implicitly-defined zval *return_value variable to your helper.

@kylekatarnls kylekatarnls force-pushed the feature/clamp branch 2 times, most recently from e1bcf76 to 4f07c5d Compare November 20, 2025 10:53
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.

5 participants