Editorial: Note how edge case in GetSubstitution can arise #2603
Conversation
spec.html
Outdated
| @@ -33899,6 +33899,7 @@ <h1> | |||
| 1. Let _matchLength_ be the number of code units in _matched_. | |||
| 1. Let _tailPos_ be _position_ + _matchLength_. | |||
| 1. Let _refReplacement_ be the substring of _str_ from min(_tailPos_, _stringLength_). | |||
| 1. NOTE: _tailPos_ can exceed _stringLength_ if and only if this abstract operation was invoked by a call to the intrinsic @@replace method of %RegExp.prototype% on an object whose *"exec"* property is not the intrinsic %RegExp.prototype.exec%. | |||
There was a problem hiding this comment.
"if an only if" suggests necessary and sufficient, but presumably the condition isn't sufficient. I.e. it alone doesn't guarantee that _tailPos_ will exceed _stringLength_. So I think delete "if and".
Also, I think it's the necessary condition only if you assume that intrinsics have their initial settings. E.g., if String.p.replace is something other than %String.p.replace%, then aren't all bets off on how it might invoke GetSubstitution? So maybe it would be better to phrase the note as one way that the weirdness can happen. (Or maybe, the only way it can happen using spec-defined algorithms?)
There was a problem hiding this comment.
Nothing has access to GetSubstitution except the builtin methods.
There was a problem hiding this comment.
An implementation can supply a non-standard function; can't its semantics be defined using abstract operations such as GetSubstitution?
There was a problem hiding this comment.
I suppose that's true, but I don't think this spec needs to consider that scenario, ever. AOs aren't an API.
There was a problem hiding this comment.
Good point about if, will fix.
Re: other specs calling this algorithm, I agree with @ljharb - we have no control over how other specs invoke our AOs, but we don't usually let that stop us from writing down useful invariants which hold within this document. (And, as a practical matter, I really don't think any other spec would want to call GetSubstitution.)
There was a problem hiding this comment.
AOs aren't an API.
True, though I don't think there's anything stopping an implementation (or another specification) from treating them as such.
bakkot
added
the
ready to merge
ljharb
force-pushed
the
note-exec-weirdness
branch
from
7cf6a91 to
ba8ebd2
Compare
This adds the note I suggested here.
It's a bit wordy because the edge case can only happen in very specific circumstances. I'm very open to suggestions for improved wording here.