[spec] refactor spec to generate a nonobservable Map instance containing all intrinsics

[ljharb]

Calling getIntrinsic with no arguments returns a "keys" iterator for this Map.

I'm sure there's lots of spec text massaging that's needed here, but I'm hoping the overall approach can achieve stage 2. Thoughts are welcome.

Closes #11.

[erights]

What's the easiest way for me to look at this rendered?

[ljharb]

@erights https://raw.githack.com/tc39/proposal-get-intrinsic/enumerability/index.html

[zloirock]

God's functions are evil. One function - one responsibility. It's much better to add a namespace like

namespace Intrinsics {
  get(name: string): any;
  iterate(): Iterator;
}

or even

namespace Intrinsics {
  get(name: string): any;
  @@iterator(): Iterator;
}

[ljharb]

I don’t think “god function” really is an appropriate label for something with only two responsibilities. Personally i find this preferable to a namespace anyways, but i think it’s critical the getIntrinsic function be global - so two functions would mean two globals, and that’s something to avoid.

[ljharb]

@erights that "one per realm" phrasing is not in this proposal, it's in the main spec already, and I thus haven't paged it in.

Getting accessor intrinsics is an important use case, so I do think we need to specify it. For example, I need to be able to get the original Map size accessor function.

That column is added because, without it, this proposal would cause these previously unobservable intrinsics. I only included a few rows to show the insertions/changes rather than replicating the entire table.

Re baseIntrinsic, that's the spec's notation for for-each loops - baseIntrinsic is defined by that line, as the iteration variable.

Regarding cycles, I thought I'd already covered that in the Map creation code, by explicitly not traversing any constructor or prototype or __proto__ properties, but if not, I'd be happy to refine that in stage 2; that's obviously important to address.

The Map instance created here is not an intrinsic, and regardless can never be directly exposed to user code - intrinsics are only things that are listed in the table, or reachable from the items in the table in a new realm, which doesn't include this map instance.

The MapHas check is not checking it's a map - that's already stipulated, by the return type of the AO - it's just meant to be checking if the key is present - but i see i've forgotten the key :-) so i'll fix that.

I would be happy to add normative text to guarantee that no builtin things ever have % in the name, but I don't think that's actually necessary - if that ever happened, the % logic in the spec would need to change for the spec to be coherent.

isFinite is included in the proposal because it's adjacent to a change, and just like git diffs, unchanged lines adjacent to changed lines are included as context clues to help with understanding where in the original document the change occurs.

[ljharb]

Introducing two names works fine, but it would mean we have to create two globals - and two new intrinsics - rather than one. Is there an alternative you can suggest that only adds a single global function?

[erights]

Normally I would also rather introduce one new global variable name rather than two. But this is based on cognitive burden --- how much more complex does it make the language feel?

For this case, could you state why you'd prefer introducing one, rather than introducing a pair or names that are obviously closely related. What cost are you economizing?

[ljharb]

I'm worried that trying to add two globals for this proposal may cause the entire thing to get more pushback than it would for having a dual-responsibility function. Obviously if my fear is unfounded, I'd switch to two globals, and either way I'll certainly mention that as a possibility.

[erights]

To answer your question, all other ways I've immediately been able to imagine that use only one function would be even worse than what you propose. So I'll spare you ;)

[erights]

Good! With that clarified, we can take the temperature of the room, find our which design gets less pushback, and go with that.

[bathos]

Could we require hosts not to name any of these data properties with a name beginning with '%' [...]

For the web platform, this would probably be an uncontroversial requirement since it’s already syntactically enforced there in a sense: the Web IDL identifiers that get mapped to property names in the ES binding’s “initial objects” (= host intrinsics) can’t include “%”.

[erights]

For the web platform, this would probably be an uncontroversial: the Web IDL identifiers that get mapped to property names in the ES binding’s “initial objects” (= host intrinsics) already can’t include “%”

@bathos that's great news! Thanks for the clarification. It would still be nice to codify the invariant, so that all hosts know to obey that constraint.

[gibson042]

We should also think about the proper key for %Intl%.[[FallbackSymbol]].

[bathos]

ECMA-402 also doesn’t currently “declare” the %SegmentIteratorPrototype% and %SegmentsPrototype% well-knowns in the WKIO table (issue). The SplitIntrinsicPath AO relies on that table, so it would throw a TypeError for these.

[gibson042]

ECMA-402 %Intl% and its [[FallbackSymbol]] are intrinsics (the latter a "hidden intrinsic" not reachable from property access that starts with the global object) in implementations supporting that spec, and therefore must be associated with some key in this proposal, whether treated as host-added or not.

[gibson042]

ECMA-402 being basically a normative-optional part of ECMA-262, I think it would be best if the two specs cooperated here, with 402 adding to the table in 262 and 262 documenting that (like the handling of toLocaleString methods). And for https://github.com/tc39/ecma402/issues/655 , possibly "demoting" its intrinsics, e.g. %NumberFormat% to %Intl.NumberFormat%.

[ljharb]

To clarify, the % is strictly intended to remain an editorial part of the spec; it's not to be exposed as part of the API. eg, getIntrinsic('Array.prototype.concat').

[ljharb]

@gibson042 intrinsic notation doesn't allow for field/slot lookup, so it'd need to be a top-level intrinsic value in the 402 spec, like %IntlFallbackSymbol%, to be valid. It's possible 402 is already not meeting this requirement, but it can and should be fixed.

[gibson042]

@ljharb Exactly. I'm not proposing that the fallback symbol be looked up using slot notation, just pointing out that the hidden intrinsic exists and needs representation (presumably as a top-level sibling of %Intl% like you suggest).

[rbuckton]

I'm not opposed to using Map, but I'm curious why this chooses to do so. Would it not be sufficient to specify this as just constructing a List of key/value pairs, and letting implementations perform their own optimizations? Or even just referencing the table directly like we do in https://tc39.es/ecma262/#sec-runtime-semantics-unicodematchproperty-p?

[rbuckton]

Would it not be sufficient to specify this as just constructing a List of key/value pairs, and letting implementations perform their own optimizations? Or even just referencing the table directly like we do in https://tc39.es/ecma262/#sec-runtime-semantics-unicodematchproperty-p?

Sorry, I see that this was the prior implementation. That said, I am curious why it would be necessary to depend on a Map just to use its keys() as opposed to producing an iterator over the names in the table.

[ljharb]

The table isn't sufficient, because it only contains top-level intrinsics, and doesn't contain host-added intrinsics - additionally, the order in the table shouldn't dictate observable ordering. Also, using a Map is already using spec Lists internally :-)

That said, in this PR I extract the MapGet etc logic (which uses lists, in [[MapData]]) into AOs so that I can use it - I could instead just copy-paste the same logic to iterate over Lists. At that point, I'd need to either use a generator, or make a new intrinsic IntrinsicIterator or something.

[gibson042]

You could also refactor the introduced AOs to operate directly on a List of { [[Key]], [[Value]] } Records rather than a Map with [[MapData]] of that shape.

@@ … @@ GetIntrinsicsMap(
           1. Let _hostIntrinsics_ be HostAddedIntrinsics().
-          1. Let _map_ be ! OrdinaryCreateFromConstructor(%Map%, "%Map.prototype%", « [[MapData]] »).
-          1. Set _map_.[[MapData]] to the list-concatenation of _intrinsics_ and _hostIntrinsics_.
-          1. Return _map_.
+          1. Return the list-concatenation of _intrinsics_ and _hostIntrinsics_.
       </emu-alg>
     </emu-clause>
@@ … @@ <emu-clause id="sec-map-get" type="abstract operation">
         MapGet(
-          _map_: a Map,
+          _entries_: a List of { [[Key]], [[Value]] } Records,
           _key_: an ECMAScript language value,
         ): a Boolean
       </h1>
       <dl class="header">
       </dl>
       <emu-alg>
-        1. Let _entries_ be the List that is _map_.[[MapData]].
         1. For each Record { [[Key]], [[Value]] } _p_ of _entries_, do
           1. If _p_.[[Key]] is not ~empty~ and SameValueZero(_p_.[[Key]], _key_) is *true*, return _p_.[[Value]].
         1. Return *undefined*.
       </emu-alg>
     </emu-clause>

and so on.

[ljharb]

That's a great point, and I definitely will do that regardless :-)

[ljharb]

I'm going to merge this, and then put up a PR that moves to Reflect methods.

After that, I'll file an issue to track whether we should be including all intrinsics or only hidden ones.

[ljharb]

Also, I'll file an issue about using a Map internally vs iterating over a List.

[thanhlucifer]