Editorial: remove unnecessary AddRestrictedFunctionProperties

[ljharb]

• also de-nests %ThrowTypeError%

Fixes #877. Relates to #867.

cc @anba @claudepache

[jmdyck]

With the definition of AddRestrictedFunctionProperties removed, I think the definition of %ThrowTypeError% doesn't make much sense to stay where it is.

I though it might make sense after CreateIntrinsics, but now I'm thinking that the only reason for CreateIntrinsics to explicitly set up %ThrowTypeError% is so that it's defined for the call to AddRestrictedFunctionProperties. With that gone, the "thrower" steps in CreateIntrinsics could also go. (CreateIntrinsics will still do that stuff, but implicitly, in the big step.)

At that point, the only remaining use (in the ES spec, anyhow) of %ThrowTypeError% would be in CreateUnmappedArgumentsObject. We could move the definition of %ThrowTypeError% there, but it might make more sense to put it in the "standard built-ins" part of the spec. Problem is, I don't see a great place for it there either. Maybe in 19.2 Function Objects?

[ljharb]

I believe ThrowTypeError is used in a number of places in this spec, but probably also in other specs - it’s not something we should be trying to remove.

I’m fine to move it to another place, though - suggestions?

[jmdyck]

I believe ThrowTypeError is used in a number of places in this spec,

Currently only 3:

• AddRestrictedFunctionProperties
• CreateIntrinsics
• CreateUnmappedArgumentsObject

This PR deletes the first and allows explicit use of %ThrowTypeError% to be deleted from the second.

(I thought there were more uses too, but it looks like there's never been very many.)

but probably also in other specs

Wouldn't surprise me. Which is why I said "the only remaining use (in the ES spec, anyhow)".

it’s not something we should be trying to remove.

If you think I'm trying to remove %ThrowTypeError%, you misread my comment. I'm just trying to find a better home for it and simplify CreateIntrinsics.

Speaking of which, it looks to me like (given this PR) CreateIntrinsics also wouldn't need to explicitly create %FunctionPrototype%. Or %ObjectPrototype%. (It would still have to create them, just without mentioning them explicitly.)

[ljharb]

I also suspect that those need to remain explicitly created and explicitly called out as intrinsics; either way, i think that’s out of scope for this PR, which is only trying to address #877 :-)

[jmdyck]

I also suspect that those need to remain explicitly created and explicitly called out as intrinsics

Table 7 already explicitly calls them out as intrinsics. Why do you suspect that they need to remain explicitly created? (And why doesn't that suspicion extend to things like %Function% or %Object%?)

either way, i think that’s out of scope for this PR, which is only trying to address #877 :-)

Well, I think the only reason they're explicit in CreateIntrinsics is because of the call to AddRestrictedFunctionProperties. So if that goes, it seems natural to clean up the supporting code. But I'm also okay with doing that separately.

[littledan]

I'm confused by this change. I thought we were removing the poison pill from arguments, but leaving it on Function.prototype so that strict mode functions will throw when they are accessed. It looks like this patch will remove that poison, though.

[ljharb]

See the linked issue; let’s clarify there.

[claudepache]

• Why on earth is this PR tagged ”Editorial”?
• According to my tests, this PR does not reflect what browsers currently implement, whilst the current spec (simple poison pills on Function.prototype) seems implemented by Chrome and Safari, and Firefox implements a more elaborate variation of poison pills.

[ljharb]

@claudepache please see the linked issue; it’d be great to hash it out there.

[littledan]

I think this PR should simply be closed for the reasons @claudepache states. It is a normative change that doesn't reflect what browsers do or anything we agreed on. The poison pill still makes sense for the same reasons it was initially added.

[littledan]

A PR thread is a completely appropriate place to discuss a proposed change, as @claudepache has done here. This PR doesn't do what I thought the bug was about, as it is a Normative rather than Editorial change.

[ljharb]

The issue was a detailed mention of something that could be removed; i made the PR. It’s fine to discuss things in PRs, but the OP of the issue has the opinion, i just attempted to implement it while trying to address open issues.

If the issue discussion ends up with an alternative approach, I’ll be happy to close or update this PR.