ecma402 icon indicating copy to clipboard operation
ecma402 copied to clipboard

Spec should be consistent about calling intrinsics

Open gsathya opened this issue 7 years ago • 4 comments
trafficstars

In ResolveLocale, we have

  1. a. Let privateIndex be Call(%StringProto_indexOf%, foundLocale, « "-x-" »).

subsequently, we have

  1. b. If privateIndex = -1, then i. Let foundLocale be the concatenation of foundLocale and supportedExtension.

and

  1. c. Else, i. Let preExtension be the substring of foundLocale from position 0, inclusive, to position privateIndex, exclusive. ii. Let postExtension be the substring of foundLocale from position privateIndex to the end of the string. iii. Let foundLocale be the concatenation of preExtension, supportedExtension, and postExtension.

For calculating the index, we call out to the %StringProto_indexOf% intrinsic, but for substring calculation we don't call out to String.prototype.substring.

It'd be great to make this consistent both here and elsewhere in the spec.

I'd prefer for us to remove calls out to these builtins (like %StringProto_indexOf%) and just inline the appropriate steps as it's currently done for substring calculation.

gsathya avatar Jul 16 '18 08:07 gsathya

What is the benefit of inlining the definitions vs calling the intrinsic? Since our implementation is in JS, I will selfishly voice my support for always calling intrinsics, since that maps more closely to the implementation for us.

jackhorton avatar Jul 16 '18 20:07 jackhorton

I think it should be relatively straightforward to translate from the English text to a JS definition for your implementation. The other way is more awkward, though: you have to think through if the JS function is getting at any funny edge case in JS semantics before understanding what it does. But above all, an English explanation will be more readable, and we already have too much cruft to work through when reading this specification.

littledan avatar Jul 17 '18 00:07 littledan

We discussed this in July 2018's ECMA-402 meeting. Ultimately, there weren't strong opinions in the group; some people found the current wording easier for self-hosted implementations, but we'd also be OK with this change.

littledan avatar Jul 28 '18 15:07 littledan

Looking at the usages of builtins in the spec, it seems like we may be rewriting those portions soon: We'll be able to replace the pattern handling with @anba 's excellent DeconstructPattern from Intl.ListFormat, and we'll revisit locale parsing with the Intl.Locale proposal. When merging these specifications, I think it will make sense to minimize the use of JS functions.

littledan avatar Oct 14 '18 19:10 littledan

Closing. The spec is now using StringIndexOf and string-concatenation instead of calling intrinsics.

anba avatar Aug 14 '23 12:08 anba