CanadaHonk / proposal-math-clamp

A TC39 proposal to add Math.clamp
https://canadahonk.github.io/proposal-math-clamp/
MIT License
36 stars 1 forks source link

Special handling for NaN #8

Open CanadaHonk opened 1 day ago

CanadaHonk commented 1 day ago

Not sure on details but it will likely have to be specially handled, as it should be rejected in modern proposals: https://github.com/tc39/how-we-work/blob/main/normative-conventions.md#number-taking-inputs-should-reject-nan

Richienb commented 1 day ago

We could replace this: https://github.com/Richienb/proposal-math-clamp/blob/fbf4b5a1173996cfc896ebdcd6c6a3e747d79e29/spec.html#L18C6-L18C54 with If _max_ is neither *undefined*, *null* nor *NaN*, then and do something similar for min

If an argument to a built-in function is expected to be a non-NaN Number

I don't think the rule applies to us because we intentionally define NaN as "no bound"

ljharb commented 1 day ago
CanadaHonk commented 1 day ago

Fixed in 513523edf172b71060b96249e90be8c7db997e9d

CanadaHonk commented 1 day ago

Reopening to allow for discussion

theScottyJam commented 11 hours ago

if the value is NaN, either throw, or return NaN (i prefer "return NaN", and i think that convention doesn't necessarily constrain us here)

I would prefer for this to throw - but that's mostly because I wish that all operations that currently return NaN would throw instead - NaN is, after all, nothing more than a way to represent an error in a calculation, and I would rather that errors throw earlier instead of causing weird bugs later.

In practice, from my understanding, it's better for operators such as division to return NaN instead of throwing an error simply because it's more performant that way (at least, that's what I've heard). But I don't know if the same argument applies to a clamping function - especially if we're considering it to throw when other arguments are NaN..

ljharb commented 5 hours ago

I think that there's definitely a potential for Math.clamp to be used in a hot path along with other math operations, which would mean that the code already has to guard against NaN, so there's no added hazard here from it.