Reference type and implementation reality

[littledan]

Does anyone have an idea of a resolution on this bug in the old repository?

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

Quoting @anba

Tests added to test262 [1] have revealed long-standing deviations [2] between the specification for the Reference type and actual implementation reality. Whether the specification should be changed to reflect implementation reality [3] or alternatively implementers should try to comply with specified behaviour should be discussed.

[1] Added/Proposed PRs for test262 coverage: https://github.com/tc39/test262/pull/91 https://github.com/tc39/test262/pull/273 https://github.com/tc39/test262/pull/275

[2] Mozilla source code from 1998 already shows the spec violation which is still present in modern engines. Details can be found in https://github.com/tc39/test262/pull/273.

[3] The exact "implementation reality" still needs to be determined.

@ajklein

[bterlson]

There is no resolution, and further the deviation continues to exist. I think if we want to change this it should be a full proposal because it is a core part of the semantics and changing this has been very controversial in the past. The difficult part in my mind is that the current spec seems correct and the biggest argment in favor of fixing this problem is that it's hard for implementations to do so. Maybe that's enough?

[bakkot]

For reference, here's a couple of simple tests, with results on current (public) builds of various engines (ETA: and some older browsers). The current spec dictates that both should have hits = 1 immediately after execution; I report the value it actually attains.

var hits = 0, obj = {p: 0}, prop = {toString: function(){++hits; return 'p';}}; obj[prop]++;

• SpiderMonkey: 1
• v8: 2
• WebkitCore: 2
• ChakraCore: 2
• Nashorn: 2
• IE6: 1
• IE8: 1
• IE10: 2
• Chrome 15: 2
• Firefox 0.8: 1
• Firefox 1.5: 1

var hits = 0, obj = {p: 0}, prop = {toString: function(){++hits; return 'p';}}; obj[prop]+=1;

• SpiderMonkey: 2
• v8: 2
• WebkitCore: 2
• ChakraCore: 2
• Nashorn: 2
• IE6: 1
• IE8: 1
• IE10: 2
• Chrome 15: 2
• Firefox 0.8: 2
• Firefox 1.5: 2

Prefix and postfix increment and decrement all behave identically on a given platform, as do all of the compound assignment operators. Per the above, the only variation is that SpiderMonkey only performs ToPropertyKey once for increment/decrement. In all other cases, across this set of engines, ToPropertyKey is performed exactly twice, in defiance of spec.

ETA: I've added older versions of Chrome, Firefox, and Internet Explorer, in the interests of knowing how long-standing this is. IE6 and IE8, uniquely among the browsers I've tested, conform to the spec as it then stood and still stands.

[littledan]

@bakkot Would you be interested in writing up a proposal for a spec change here? Or do you prefer the semantics of the spec?

[bakkot]

@littledan - I much prefer the semantics in the spec. The situation in question seems similar to obj[f()]++, which surely should not call f twice.

[littledan]

@bakkot No, f() would only be called once; it's ToPropertyKey on the result which is called multiple times.

[bakkot]

@littledan, right, that's my objection to changing the spec. To the user it appears that there is a single operation being performed, which is getting the name of the property. For half of that step to be repeated seems overcomplicated and confusing. Since f obviously should not be (and is not) called more than once, nor should ToPropertyKey.

[bterlson]

@bakkot Curious, considering that implementations have had this bug for two decades now, do you want the spec to continue to say something implementers aren't motivated to fix and might not be web-compatible, or would you prefer the spec to reflect reality of what implementers do and the web (and other code) may depend on?

[bakkot]

@bterlson, I have no strong feelings either way.

I'd be surprised to learn anyone was particularly depending on the current behavior, though, both because of that one SpiderMonkey difference above and because I believe it's rare to have side-effecting toStrings. If that's the case, the best thing in my opinion would be for implementations to change to reflect the current spec; given, as you say, implementers maybe aren't going to bother, I'm largely ambivalent as to what the spec should say.

[bterlson]

@bakkot, thanks this is helpful. For what it's worth, I don't think there is any (or much) disagreement that the spec text is currently better than how implementations behave. The problem is that this is a long-standing deviation implementations have and even for an edge case like this the likelihood of the web depending on this behavior seems high. The problem is that an implementation will have to proceed carefully under the assumption that this pattern exists on the web, and that's the hard issue to work around. I'm not sure any of us are willing to go through the motions for this.

[bakkot]

@bterlson: In the interests of knowing how likely it is that anyone is depending on this... it turns out that IE6 and IE8 got this right, that is, they conform to spec. I'll try to test some other browsers when I get a chance. ETA: Done; see updated table. IE8 conforms to spec, IE10 does not. The oldest versions of Chrome and Firefox I have easy access to do not conform.

[evilpie]

Eric just fixed the += case in Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1260577 I don't think it's likely that sites depend on some specific toString behavior here, at least I have never seen it. Especially considering IE had the correct behavior here.

[allenwb]

Does anyone have an idea of a resolution on this bug in the old repository?

My recollection (I could be wrong) the last time TC39 discussed this issue at a meeting, we decided to retain the current (and legacy) spec. language. I meeting notes search (it may predate github notes) would be required to verify that.

[lars-t-hansen]

@bterlson, of historical interest: Opera 9 (March 2009) got this right (conforms to the spec), as did likely Opera versions going back to Opera 7 at least (ca 2004). Opera 12 gets it wrong. (I only tested the ++ case but IIRC this was all handled through some ToPropertyKey type operation in the bytecode for RMW ops in the older engine.)

[claudepache]

Another interesting testcase (related to tc39/test262#273):

var hits = '';
var base = {};
var prop = function () { 
    hits += 'A';
    return { toString: function () { hits += 'B'; } };
}
var expr = function () { 
    hits += 'C';
}
base[prop()] = expr();
hits;

Expected: "ABC" Actual: "ACB" (tested on current Chrome, Edge, Firefox, Opera, Safari)

Old Opera 9 had it right ("ABC"). Unable to test old IEs.

Changing the spec to match current implementations might be tricky, as it implies to split the algorithm in 12.3.2.1 in two parts: steps 1-4 produces "A" in my testcase, steps 5-8 produces "B".

[littledan]

When this was discussed at TC39, it seemed like the resolution was to stick with the current spec, pending Mozilla shipping its implementation and demonstrating that it was web-compatible. Microsoft emphasized that fixing the bug on their side was low priority. @efaust How has shipping that patch gone? Do you pass the test that @claudepache mentioned?

[hax]

@claudepache IE6/IE8 return "ABC".

[ljharb]

@evilpie @jswalden can you reply to https://github.com/tc39/ecma262/issues/467#issuecomment-214943340, perhaps?

I get the following results today with eshost -x "var hits = 0, obj = {p: 0}, prop = {toString: function(){++hits; return 'p';}}; obj[prop]++; print(hits);":

#### Chakra
2

#### JavaScriptCore
2

#### SpiderMonkey
1

#### V8
2

which suggests that "2" is in fact the web reality, and is what should be specified. @bakkot are you willing to write the PR?

[littledan]

@ljharb Good point. Additionally, I believe there are other, related cases that even SpiderMonkey gets "wrong", agreeing with those three other implementations, but I don't have those cases dug up at the moment (maybe @anba does?).

[anba]

Additionally, I believe there are other, related cases that even SpiderMonkey gets "wrong", agreeing with those three other implementations, but I don't have those cases dug up at the moment (maybe @anba does?).

You mean in addition to the one from above https://github.com/tc39/ecma262/issues/467#issuecomment-214787216?

A related one is:

var hits = '';
var base = {};
var prop = function () { 
    hits += 'A';
    return { toString: function () { hits += 'B'; } };
}
var expr = function () { 
    hits += 'C';
}

try {
    null[prop()] = expr();
} catch (e) {}

print(hits);

Spec: "A" SM/V8: "AC" JSC/CH/Graal: "ACB"

---

I think this issue is simply super-low priority for implementers, because it's too obscure to make any difference in practice and the cost benefits are too small to justify putting any efforts in it to change an implementation. But I'm also not sure if that justifies that the spec has to change, especially because it's not a one-line change what's needed here (as already mentioned above by Claude).

[jswalden]

I agree with @anba -- this is just low priority for people to fix. I don't think that alone justifies changing sensible spec semantics.

[ljharb]

It seems important that the spec match what engines actually do; if there's a way to decrease that gap (even if it's too low a priority for the resulting minority of "incorrect" implementations to ever fix), it seems worth making the change in a general sense.