Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editorial: Add a few well-known intrinsic objects #1105

Closed
wants to merge 1 commit into from

Conversation

TimothyGu
Copy link
Member

These will be helpful in solving whatwg/webidl#254.

@bterlson
Copy link
Member

bterlson commented Feb 21, 2018

This list is getting so huge. How much bigger will it get? Do we need to consider another way to handle well-known intrinsics?

E.g. you could imagine an abstract operation GetIntrinsic with a parameter path where each path segment corresponds to the name of a property starting on the global? So, e.g., %ArrayProto_keys% becomes GetIntrinsic("Array.prototype.keys"). Then we can reserve %-style for "anonymous" intrinsics like AsyncFunctionPrototype. Thoughts?

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 21, 2018

There are two questions there, which I suspect are orthogonal:

  1. How do we refer to an intrinsic (in an algorithm, say): %ArrayProto_keys% vs GetIntrinsic("Array.prototype.keys")?

  2. Is there a benefit to an up-front listing of well-known intrinsics? (Would it be better to scatter their 'declarations' throughout the spec? Do they need to be declared at all?)

@TimothyGu
Copy link
Member Author

you could imagine an abstract operation GetIntrinsic with a parameter path where each path segment corresponds to the name of a property starting on the global

I believe the spec right now is mostly pretty consistent in having one "statement" on each line in algorithms, and to do that with GetIntrinsic will definitely increase a lot of lines.

Is there a benefit to an up-front listing of well-known intrinsics? (Would it be better to scatter their 'declarations' throughout the spec? Do they need to be declared at all?)

I'm fine with not listing all of them, but note that there are some intrinsics that are not <dfn>'d; see the unlinked items in Table 7.


In any case I believe this discussion isn't directly related to the PR. I'd be happy to offer opinions outside of this issue but I don't think it should block whatwg/webidl#254.

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 22, 2018

Technically, I don't think whatwg/webidl#254 is blocked on this. It isn't constrained to use the %Foo% syntax, it could just use the 'column 3' prose from the patch.

(This isn't an objection to the PR.)

@bterlson
Copy link
Member

I believe the spec right now is mostly pretty consistent in having one "statement" on each line in algorithms, and to do that with GetIntrinsic will definitely increase a lot of lines.

I don't think there is any problem with nesting a calls (it's used in quite a few places I can see, e.g. the nesting of Get inside ToBoolean here).

Is there a benefit to an up-front listing of well-known intrinsics? (Would it be better to scatter their 'declarations' throughout the spec? Do they need to be declared at all?)

If there's an active thing the spec has to do to "export" a symbol, an upfront listing makes sense. Right now it's an easy way to figure out all the well-known intrinsics that are DFN's cross the spec. But I don't think it's super useful in-and-of-itself and wouldn't mind deleting most of it if we can find a better convention.

@domenic
Copy link
Member

domenic commented Feb 22, 2018

It's important to remember that this table is actually the basis for some more formal infrastructure, namely the [[Intrinsics]] field. See in particular https://tc39.github.io/ecma262/#sec-createintrinsics.

That is, in the rest of the spec, %Foo% is actually shorthand for (the current Realm Record).[[Intrinsics]].[[%Foo%]]. So in the current spec, there is an explicit step during Realm creation where we stash away all these pre-designated intrinsics for future reference.

I'm not sure how to spec something like GetIntrinsic that works on a similar rigorous stashed-ahead-of-time foundation.

(Also, please ensure that any solution here allows for looking up the intrinsic in a specific realm, even if it defaults to the current Realm Record. We use the realm.[[Intrinsics]].[[%Foo%]] syntax in Web IDL a few times.)

@allenwb
Copy link
Member

allenwb commented Feb 22, 2018

Also, please note that the %foo% notation and the supporting definitions were added to clear up realm-related ambiguities that had existed in previous editions that used various prose formulations to refer to built-ins and what we now call "intrinsics". Any changes need to take into account both that there are no actual ambiguity and that the new notation is not likely to be read ambiguously. The nice thing about %foo% is that there are no hints WRT what it might mean so everybody who is serious about understanding the specification has to initially look it up.

@annevk
Copy link
Member

annevk commented Feb 23, 2018

Nobody mentioned it yet, and I'm not entirely sure, but isn't this also useful information for implementations to have these gathered in a single place? Which objects they need to have a safe reference for?

@littledan
Copy link
Member

@annevk I think the set of things that actually needs to be kept around in a real implementation will differ somewhat from what the spec lists (in both directions, including some additional things and leaving out some things).

It seems like these particular objects are needed for export just to call the original versions of the algorithms, rather than to actually refer to particular ES-defined objects with identity. ECMA-402 has a number of similar usages of %-identified functions.

What if we referred to these algorithms instead by factoring out the algorithm into a separate abstract algorithm to refer to directly? This would avoid the need for a table, and also avoid the need to make reference to ECMAScript semantic concepts like Call when we're really just trying to get at those algorithm steps.

@annevk
Copy link
Member

annevk commented Feb 23, 2018

That sounds like a good division. If you need to reuse an algorithm, abstract it; if you need the initial version of an object, make it an intrinsic.

(I'm wondering again how we want to keep track of downstream dependencies so future editors won't be misled into thinking they can inline some of those abstract operations or remove intrinsics.)

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 23, 2018

isn't this also useful information for implementations to have these gathered in a single place? Which objects they need to have a safe reference for?

From that point of view, I think Table 7 is bigger than it needs to be. E.g. consider %ArrayProto_entries% or %isNaN%: I don't think there's anything in the spec that would require an implementation to keep a "safe reference" (assuming I know what you mean by that) to either of them.

I'm not sure how many of these there are: they aren't something you can just grep the spec for. Maybe they're there for the benefit of other specs (as in this PR), but it seems like we need a better system, where other specs can get the benefit without requiring additions to Table 7.

@ljharb
Copy link
Member

ljharb commented Feb 23, 2018

There are a few current uses of intrinsics where an abstract operation wouldn’t suffice; such as %Promise% and other similar instances where the function reference is needed. I really like some sort of “GetIntrinsic” operation as a way to refer to “the original value of”. Implementors could grep the spec for all uses of the operation to come up with a list of all of the intrinsics they needed to “keep around”. Such an operation would of course follow the realm constraints @domenic has laid out, but I’m not sure yet how to address the concerns here.

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 23, 2018

It seems like these particular objects are needed for export just to call the original versions of the algorithms, rather than to actually refer to particular ES-defined objects with identity.

Not sure I agree. Do you have an example?

@annevk
Copy link
Member

annevk commented Feb 23, 2018

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 23, 2018

I really like some sort of “GetIntrinsic” operation as a way to refer to “the original value of”.

It's a little tricky in that not every intrinsic is "the original value of" some access path (i.e., a property of a property [etc] of the global object). In Table 7, look at the blank cells in the "Global Name" column.

Of course, we can just make up names for those (as the current spec has), but we might want to visually distinguish between made-up names and 'real' access-paths, so people don't mistake one for the other.

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 23, 2018

@annevk: Ah, thanks, I was trying to think of examples just in the ES spec (not paying enough attention when @littledan said "needed for export").

@annevk
Copy link
Member

annevk commented Feb 23, 2018

I really like some sort of “GetIntrinsic” operation as a way to refer to “the original value of”. Implementors could grep the spec for all uses of the operation to come up with a list of all of the intrinsics they needed to “keep around”.

This doesn't address that use case due to external dependencies, which are probably useful to know about when you're working on a JavaScript engine.

@allenwb
Copy link
Member

allenwb commented Feb 23, 2018

From that point of view, I think Table 7 is bigger than it needs to be. E.g. consider %ArrayProto_entries% or %isNaN%: I don't think there's anything in the spec that would require an implementation to keep a "safe reference" (assuming I know what you mean by that) to either of them.

So why were these added to Table 7? The table is only intended to contain preallocated built-in object that need to explicitly referenced by the specification.

If entries like these have been added because of a misunderstanding of the purpose of Table 7, then those entries should be removed.

If they are there because they are needed by host environments, then they should probably be tagged as such in the table. However I'm actually a bit dubious of host requesting that we add entries to this table. They may be needed by host in a few cases, but I'd like to see some specific uses.

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 23, 2018

So why were these added to Table 7?

Well, you were the editor when %isNaN% was added (ed 6 rev 35), so maybe your notes say why. As for %ArrayProto_entries%, it was added in 54a0c8c, which refers to #993, which indicates that it was (or would soon be) needed by WebIDL.

@littledan
Copy link
Member

@ljharb I agree that factoring out abstract algorithms would only handle some of the usages, but it would handle all the ones in this PR. That would allow this particular issue to make progress, while GetIntrinsic can be considered as a separate cleanup.

@annevk
Copy link
Member

annevk commented Feb 24, 2018

You can find IDL usage of %ArrayProto... at https://heycam.github.io/webidl/#es-iterable. That does suggest to me those cannot simply become an abstract operation. The same probably goes for those added in this PR?

(The Infra example I linked earlier on parsing JSON could easily become an abstract operation I think.)

@domenic
Copy link
Member

domenic commented Feb 24, 2018

In #1012 (comment) I proposed an abstract operation for JSON parsing but #1013 ended up using the intrinsic per #1013 (comment)

@littledan
Copy link
Member

OK, if we consider adding more % things cleaner than abstract algorithms, then there should be no problem landing this PR as is.

@anba
Copy link
Contributor

anba commented Mar 5, 2018

Well, you were the editor when %isNaN% was added (ed 6 rev 35), so maybe your notes say why.

@jmdyck %isNaN% (and the other function properties of the global object) were added, so they can be accessed in SetDefaultGlobalBindings.

@jmdyck
Copy link
Collaborator

jmdyck commented Mar 5, 2018

%isNaN% (and the other function properties of the global object) were added, so they can be accessed in SetDefaultGlobalBindings.

Ah, right. In fact, if those 'top-level' object property values aren't in Table 7, then CreateIntrinsics won't even "know" to create them in the first place.

@ljharb
Copy link
Member

ljharb commented Dec 21, 2018

Hopefully #1376 reduces the need for many of these additions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants