tc39 / proposal-regexp-match-indices

ECMAScript RegExp Match Indices
https://arai-a.github.io/ecma262-compare/?pr=1713
BSD 3-Clause "New" or "Revised" License
64 stars 13 forks source link

Consider not specifying caching in GetIndices #29

Closed schuay closed 5 years ago

schuay commented 5 years ago

GetIndices currently specifies caching mechanisms (see f.[[MatchIndicesArray]]). This seems like an implementation detail to me and probably should not be part of the spec. The spec should just be phrased in such a way that caching becomes possible (in this case, by making relevant steps in GetIndices non-observable).

cc @joshualitt @mathiasbynens

rbuckton commented 5 years ago

This kind of behavior is often specified in this fashion. Since [[MatchIndicesArray]] is not user observable itself, implementations are likely free to implement how they choose as long as the user-observable portions of the spec are correct.

schuay commented 5 years ago

What is the advantage of writing the spec this way vs. omitting the caching logic? Out of curiosity, do you have an example of similar spots in the spec?

rbuckton commented 5 years ago

Promise reject and resolve functions work in this fashion (such that calling them the first time does something, but the second time does something different). CreateMappedArgumentsObject defines getter/setter wrappers. Aside from that, there is little other precedent in the spec for this kind of behavior, which is why I opted to follow what similar mechanisms were already present.

schuay commented 5 years ago

So I don't have a very strong opinion. Caching is something implementers would do anyway. My point is, if the spec can omit caching logic (yet still make caching possible for implementers) I think that would be preferable.

rbuckton commented 5 years ago
rbuckton commented 5 years ago

I would compare this to the [[MapData]] slot on Map. The spec algorithms for Map are extremely inefficient and I doubt most implementers have implemented Map (or Set) that way, however the algorithms indicate the minimum requirements to meet the required observable semantics.

Also, the RegExp algorithms today are already rife with somewhat ambiguous spec text and I'd like to avoid adding to that.

schuay commented 5 years ago

The way I read [[MapData]] is that it's required for correctness (the comment says The existing [[MapData]] List is preserved because there may be existing Map Iterator objects that are suspended midway through iterating over that List.)

I agree on avoiding ambiguity, but not sure I see how removing caching logic is related :)