Editorial: Revert unintentional normative change for String.prototype.substr #2844
Conversation
ljharb
added
normative change
spec.html
Outdated
| 1. If _intStart_ is -∞, set _intStart_ to 0. | ||
| 1. Else if _intStart_ < 0, set _intStart_ to max(_size_ + _intStart_, 0). | ||
| 1. Else, set _intStart_ to min(_intStart_, _size_). |
There was a problem hiding this comment.
Can't we use clamping here, too?
| 1. If _intStart_ is -∞, set _intStart_ to 0. | |
| 1. Else if _intStart_ < 0, set _intStart_ to max(_size_ + _intStart_, 0). | |
| 1. Else, set _intStart_ to min(_intStart_, _size_). | |
| 1. If _intStart_ < 0, set _intStart_ to _size_ + _intStart_. | |
| 1. Set _intStart_ to the result of clamping _intStart_ between 0 and _size_. |
There was a problem hiding this comment.
That does arithmetic on an infinite value (-∞), which we've tried to avoid. If you restore the first line then the other changes are fine, though I don't think it's any clearer or more elegant, personally.
There was a problem hiding this comment.
And in addition to the -∞ issue, the current PR matches how String.prototype.slice is specified. If we should decide to use clamping, we should also update String.prototype.slice accordingly.
There was a problem hiding this comment.
I still think the clamping is more clear, but @bakkot is right, we should leave the guard in for infinity. We can update String.prototype.slice in a separate PR if you want.
There was a problem hiding this comment.
@michaelficarra If you approve this as-is I will submit a following changing both; no need to drag this PR out further.
bakkot
added
the
ready to merge
….substr (tc39#2844) `"a".substr(0, Infinity)` should return `"a"`, but tc39#2007 incorrectly changed the result to be `""`. If we change `substr`'s algorithm to be more similar to `String.prototype.substring` and `String.prototype.slice`, we can easily fix this issue: 1. `min(intStart, size)` guarantees that `intStart` is now in `[0, size]`. 2. Clamping `intLength` guarantees it's also in `[0, size]`. With these two changes, `intEnd = min(intStart + intLength, size)` is now in range `[intStart, size]`, so we no longer have to check for `intStart ≥ intEnd`, but instead can directly perform the `substring` operation.
ljharb
force-pushed
the
substr-infinity
branch
from
055690a
to
262e214
Compare
"a".substr(0, Infinity)should return"a", but #2007 incorrectly changed the result to be"".If we change
substr's algorithm to be more similar toString.prototype.substringandString.prototype.slice, we can easily fix this issue:min(intStart, size)guarantees thatintStartis now in[0, size].intLengthguarantees it's also in[0, size].With these two changes,
intEnd = min(intStart + intLength, size)is now in range[intStart, size], so we no longer have to check forintStart ≥ intEnd, but instead can directly perform thesubstringoperation.