Skip to content

Conversation

@kylekatarnls
Copy link

@kylekatarnls kylekatarnls commented Nov 19, 2025

RFC: https://wiki.php.net/rfc/clamp_v2
Original implementation: php/php-src#19434
Documentation: php/doc-en#4814

@nicolas-grekas
Copy link
Member

That would be for PHP 8.6, isn't it?

@kylekatarnls
Copy link
Author

That would be for PHP 8.6, isn't it?

Correct, moving it.

@kylekatarnls kylekatarnls force-pushed the feature/clamp branch 2 times, most recently from 5bbf49c to d88b259 Compare November 19, 2025 09:53
@kylekatarnls kylekatarnls changed the title [8.5] Add clamp function [8.6] Add clamp function Nov 19, 2025
@kylekatarnls kylekatarnls force-pushed the feature/clamp branch 3 times, most recently from bead94d to c9033e5 Compare November 19, 2025 10:45
@kylekatarnls kylekatarnls marked this pull request as ready for review November 19, 2025 18:05
@kylekatarnls kylekatarnls reopened this Dec 7, 2025
@kylekatarnls
Copy link
Author

kylekatarnls commented Dec 19, 2025

php/php-src#19434 is merged (will land in PHP 8.6)

Comment on lines +22 to +31
* @template V
* @template L
* @template H
*
* @param V $value
* @param L $min
* @param H $max
*
* @return V|L|H
*/
Copy link
Member

Choose a reason for hiding this comment

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

What's the logic behind L and H? Maybe this could be more explanatory with TVal, TMin, TMax.

Copy link
Author

Choose a reason for hiding this comment

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

L = Low, H = High.

It was because min and max both start with an M.

But I'm fine using full words such as Value, Minimum, Maximum.

Comment on lines +34 to +40
if (\is_float($min) && \is_nan($min)) {
self::throwValueErrorIfAvailable('clamp(): Argument #2 ($min) cannot be NAN');
}

if (\is_float($max) && is_nan($max)) {
self::throwValueErrorIfAvailable('clamp(): Argument #3 ($max) cannot be NAN');
}
Copy link
Member

Choose a reason for hiding this comment

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

It was merged using must in the wording, it should be updated here as well. See https://github.com/php/php-src/blob/f80338cb926a6dbe8007ad62776bdb4f8d6dd047/ext/standard/math.c#L393-L403

Co-authored-by: Alexandre Daubois <2144837+alexandre-daubois@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants