Add __defineGetter__ & friends to Annex B

[claudepache]

Based on
https://gist.github.com/anba/9140946#jsc-sourcejavascriptcoreruntimeobjectprototypecpp

Fixes https://bugs.ecmascript.org/show_bug.cgi?id=2716

[ljharb]

What happens if you include __defineGetter__ in an object literal, or as a string key?

Should, like __proto__, the cases where this behavior applies be limited to when it's statically typed out?

[claudepache]

@ljharb No extension is added to object literals: __defineGetter__ may be used as a regular key, just like hasOwnProperty or valueOf.

[evilpie]

I am not completely sure we can ignore Step 2 in the v8 and SM implementation. That should at least be discussed.
Thanks for getting this specified!

[claudepache]

I am not completely sure we can ignore Step 2 in the v8 and SM implementation.

That is manifestly a leftover of sloppy-mode function call semantics without real purpose.

Since JSC doesn’t do that, there is a good chance that it could be omitted. But the only way to know is to try.

[bterlson]

I don't think it's worth cleaning this up if there is any risk of breakage. Moving from throwing to using ES5 null/undefined handling seems less likely to cause problems.

[domenic]

So to be clear about the the missing step 2. It applies only when you do bare __defineGetter(...)__ calls in strict mode, right? (In sloppy mode, this is already the global object.)

Given that Safari, which IIRC was the original place these were introduced, does not cater to this case, it seems likely to me we can omit it. The alternative is believing that there are web pages out there that (a) use this legacy API; (b) use it in bare form in order to modify the global object; (c) have turned on strict mode while using the legacy API (despite strict mode only being introduced in ES5 alongside Object.defineProperty); and (d) work in Firefox or Chrome, but not Safari, and are fine with this.

[bterlson]

@domenic I agree that it is quite unlikely to break anyone (and as such am fine cleaning this up if others really want to), but there is still a risk of breakage taking this change however small and I don't see the benefit of cleaning up a legacy API if there's any risk to it. A lot of code that exists doesn't even touch Safari (intranets, web controls, etc.) and strict mode is not nearly as prevalent as we'd hope.

[evilpie]

Pretty sure SpiderMonkey introduced this functions first in 2000: jrmuizel/mozilla-cvs-history@5fd2a54

[domenic]

and strict mode is not nearly as prevalent as we'd hope.

This actually argues for my point, since it's only in strict mode that the presence or absence of step 2 makes a difference.

[allenwb]

@domenic >So to be clear about the the missing step 2. It applies only when you do bare defineGetter(...) calls in strict mode, right? (In sloppy mode, this is already the global object.)

No, things specified as built-in functions are not necessarily implemented using ECMAScript so the strict/sloppy mode concept does not apply to them. All built-ins that (for legacy reasons) need to treat a undefined/null this argument as if it was a reference to the global object need to be explicitly specified to do that.

[domenic]

@allenwb I don't quite understand your point. I was saying that sloppy-mode user code that calls __defineGetter(...)__ will behave the same way with or without step 2.

[claudepache]

@domenic The mode of the caller has no influence on whether the thisArgument received by the callee is wrapped or not.

[domenic]

Ahhh, I feel dumb. Sorry for my last few comments which are clearly wrong. I will go add <del>s to them.

[littledan]

I like the idea of leaving in the "sloppy mode"-style semantics for functions like __defineGetter__. There seems to be a web majority opinion, and this isn't a completely horrible wart, in that it's just a couple lines of spec text where the impact is limited to how those functions work. I'll be surprised if the proposed change is completely web-compatible, even if not all browsers are identical today.

[allenwb]

@littledan
Annex B features aren't about "majority opinion". Rather it is about determining the minimal addition needed to support interoperable legacy code within browsers. If some partial semantic of a feature is not supported by all legacy browsers, that part of the semantics is not part of the minimal required semantics.

In particular, it's my understanding that much to the recent demand for support this long obsolete feature (for example by IE and Edge) was driven by the need to be interoperable with Safari on iOS devices. If JSC/Safari does not have the implicit global object semantics then its not needed for that interoperability.

Implicit reference to the global object is a capability leak. We shouldn't specify that if it isn't required for legacy interop.

[bterlson]

@allenwb the dunder-* demand is quite old at this point, and what you recall is true for proto (don't recall it being the case for __d* methods). We found examples of __d* methods on the normal web as well.

Anyway I don't think the normal design guidelines apply strongly here. The real question for me is how much we want to assure ourselves that the spec is web compatible? We're not going to go off and do a bunch of data collection on this because it's just not worth it. So we can assume that there won't be problems because the subset of code Safari has access to works fine (probably true, and probably none of us will fix this low-pri bug soon anyway), or we can go with the (mostly) semantic superset and be more sure that the spec reflects reality. I'd rather do the latter and not worry about this at all, but as I said before, I don't care much.

[littledan]

Re @allenwb 's capability leak concern: It would be possible for systems like SES to replace __defineGetter__ with a version which doesn't leak the global object. Without an SES-like system which does a bit of work to lock things down, capability leaks are fairly endemic to the system, and this doesn't really seem like the straw that breaks the camel's back.

@evilpie seems to show that the feature actually originated with Mozilla. It doesn't really matter how it originated at this point; it's supported across browsers. The safest (web-compat-wise) way to standardize something would be to allow the union of semantics allowed by browsers, rather than the intersection. In that case, there actually is a union (rather than a contradiction) and that's the "sloppy mode" semantics.

[claudepache]

I've noticed that a very old (pre-ES5) version of Safari (4.0.4) did the substitution null/undefined → global object. Maybe that "feature" was accidentally removed at some point of time? (I am unable to test old but more recent versions of WebKit on my machine.)

Another point: I think it would be the only builtin in ECMA-262 that uses the global object as implicit this-value?

[evilpie]

So I was thinking about adding telemetry for this to Firefox. (i.e. how often do we hit null/undefined vs everything else). However it might take a while before we get any relevant data, especially stable is still 3 months away or so.

I am generally in favor of using the "legacy" null/undefined semantics, especially to get something specified in the short term. However I do appreciate that it would be nice to remove this and I will add telemetry for it in any case. Does anything speak against starting out with those semantics and changing that special case later?

Something else that I haven't seen discussed yet, which I assume everyone just agrees with:
v8 doesn't do Step 5.iii.b in __lookupGetter__. That is the Return undefined when a non-accessor descriptor is found. This means it would look past normal properties on the proto chain, searching for the getter/setter. I feel like that behavior is not very useful, especially because it doesn't really model how [[Get]] would behave when invoked for that property. I think SM and JSC have the right behavior here.

[bterlson]

Good point @evilpie, we aligned our semantics with v8 assuming they had good reason for their semantics. If not, I am happy to standardize the return undefined behavior (although I'll note again that I don't particularly care how useful this API is).

[erights]

Another point: I think it would be the only builtin in ECMA-262 that uses the global object as implicit this-value?

This was fixed as of ES5. For all builtins that operator on their this-argument, in ES3 they would start by coercing it to an object by the rules

null or undefined --> global
primitive -> wrapper

In ES5 we changed all of these coercions to use ToObject, turning the first case into an error:

Object.prototype.valueOf.call(null);
TypeError: Cannot convert undefined or null to object(…)

typeof Object.prototype.valueOf.call(3);
"object"

The only problem we ran into was Object.prototype.toString.call(null). An old version of jQuery relied on this not throwing but didn't care what string it returned. So we special cased this to return either the string "[object Null]" or ["object Undefined]" depending. Even this was a change from legacy behavior of printing the global object (often "[object Window]"). So we have successfully prevented all the builtins from leaking the global object, long ago. We will not regress that.

[evilpie]

I am going to land the Firefox telemetry soon. In the meantime I scrapped the JS on the homepage of the top 10k websites. As far as I can tell none of these obviously use a bareword __define[GS]etter__: https://gist.github.com/evilpie/084b0ac48d22ab7d99fb
This does make me fairly optimistic. (I manually checked those were the __define isn't prefixed by a dot or quotation mark)

[evilpie]

http://mzl.la/1S7ZxIq This is data from Nightly users for only one or two days. The results however seems pretty clear: 3 calls with undefined/null and 244.28M with everything else.

[evilpie]

Based on this data, I am fine with merging this change as is. Unless something unforeseen happens we will change the behavior in Firefox to match the spec soon.

[bterlson]

@evilpie Thanks so much for the data! I too would be fine taking this as-is given this data. Thoughts @littledan?

[littledan]

Yeah, the data sounds not bad to me, and especially given the ES5 history lesson. I'm still not positive it's representative; I think we'll learn more when we try to ship this beyond nightly and wait for complaints, and when this telemetry gets more time to ship. The change would be pretty nice, if it's web-compatible, as it would let V8 put more intrinsics in strict mode. So I'm fine with provisionally merging it, as long as we're still on the look-out for data to the contrary.

[bterlson]

@littledan totally agree. In general my philosophy is that the entire spec (and especially annex B) is provisional on web-reality :)

[zloirock]

@bterlson question: will it be in the annex B ES2016 or in the next version of the spec?

[littledan]

@zloirock This was added only to ES2017; it didn't make the cut for ES2016.

[zloirock]

@littledan ok, thanks.