Normative: Guard IntegerIndexedElementSet with receiver check

[shvaikalesh]

Closes #1339.
Closes #1541.

Recap

In ES6, both [[Get]] and [[Set]] TypedArray methods had the receiver checks, which were removed in #347 due to consistency with [[HasProperty]]. Removing receiver check from [[Set]] turned out to be web-incompatible (SM bug), breaking the 🚀 NASA's WorldWind web app and SDK.

I've just checked the links in Firefox 63 (release binaries): the web app and SDK examples still don't work. While there are no other sites that are broken, we can't just fix the WorldWind: it's an educational material that is also distributed offline and viewed via file:/// protocol.

The breakage was caused by TypedArray instances being in [[Prototype]] chain (presumably, for default values) of various objects like Matrix or Vector instances.

Proposed change

This PR attempts to specify the most desired behavior while providing the minimum required for web-compatibility: OrdinarySet is performed for canonical numeric strings that are valid indices only.

• Consistent with [[Get]] and [[HasProperty]] TypedArray methods regarding invalid indices: prototype traversal in stopped.
• Consistent with other [[Set]] overrides like of mapped arguments or legacy platform objects regarding altered receivers: a simple reference check is performed.
• No value coercion is performed for invalid indices if receiver is altered, which is an expected behavior because coercion result would be unused. Normative: Don't coerce value if index is invalid in TypedArray [[Set]] #2332 proposes to drop value coercion for direct [[Set]] as well.

Implementation status

• V8 follows the proposed spec except that it performs value coercion even for invalid indices.
• SM follows the proposed spec for valid indices, but creates property on receiver for invalid indices, without traversing prototype chain further.
• JSC doesn't currently invoke [[Set]] overrides during prototype chain traversal, and gets Reflect.set wrong for non-ordinary [[Set]]. This bug will fix non-index [[Set]], including canonical numeric strings that are invalid indices. Indexed [[Set]] will be adjusted in a follow-up, as it's quite different and unaware of receiver.

[shvaikalesh]

It feels more idiomatic to invoke IntegerIndexedElement{Get,Set} on Receiver rather than using SameValue, though I am yet to come up with more practical example.

EDIT: Type checking Receiver is weird and won't be useful. Instead, we should align with other [[Set]] overrides like of mapped arguments or legacy platform objects: perform a simple reference check.

[ljharb]

With this code:

let o = new Int8Array(1)
Object.setPrototypeOf(Array.prototype, o);
const a = [];  //a inherits indirectly from o
a[0] = 4;  //according to current spec. this will set o[0] to 4.
print(a[0], o[0]);

let target = [0]
let receiver = [0]
Reflect.set(target, 0, 1, receiver)
print(target, receiver)

let typedTarget = new Int8Array(1)
let typedReceiver = new Int8Array(1)
Reflect.set(typedTarget, 0, 1, typedReceiver)
print(typedTarget, typedReceiver)

i get these results:

#### Chakra
4 0
0 1
0 1

#### JavaScriptCore
4 0
0 1
1 0

#### SpiderMonkey
4 0
0 1
0 1

#### V8
4 0
0 1
0 1

#### V8 --harmony
4 0
0 1
0 1

so there's actually a variance in the latter case - only JSC returns 1, 0 where the others return 0, 1. This may need consensus after all.

[evilpie]

I am probably misreading the change, but doesn't the return in Step 2.b.ii mean we never set anything in this case?

[rkirsling]

Same question as @evilpie: Based on @ljharb's third test case, it seems like we set either O or Receiver, no?

[shvaikalesh]

Oops, my mistake: while always returning w/o setting anything would be a desired change, it is not web-compatible. Fixed to fall back to OrdinarySet instead.

[bakkot]

So if you have an integer index which is in bounds for O, but O and Receiver are different values, you'll now fall through to OrdinarySet. Is that intended?

[syg]

Honestly I don't remember what the behavior we got consensus for was, but this PR currently doesn't have interop wrt in-bounds/OOB accesses.

In-bounds falls through to OrdinarySet in SM and V8. OOB falls through to OrdinarySet in SM only. JSC currently always sets the value on the target instead of the receiver.

// test-iieo-receiver-mismatch.js
let target = new Int8Array(1);
let receiver = [0];

// In-bounds
Reflect.set(target, 0, 1, receiver);
print('after in-bounds set:', target, receiver);

// OOB
Reflect.set(target, 2, 1, receiver);
print('after OOB set:', target, receiver);

$ eshost ./test-iieo-receiver-mismatch.js
#### ch
after in-bounds set: 0 1
after OOB set: 0 1,,1

#### jsc
after in-bounds set: 1 0
after OOB set: 1 0

#### sm
after in-bounds set: 0 1
after OOB set: 0 1,,1

#### v8
after in-bounds set: 0 1
after OOB set: 0 1

#### xs
after in-bounds set: 1 0
after OOB set: 1 0

[codehag]

Hm, it looks like both chakra and SM had the same behavior. Whats the usecase? This only impacts reflect api iiuc, so tooling?

[syg]

I'll try to jog my memory for what the consensus was. Failing that we might need to relitigate this, unfortunately.

removing pending reconfirming what consensus is

[shvaikalesh]

@syg The consensus for TypedArray's [[Set]] was to invoke OrdinarySet if receiver is altered, for all indices, including OOB (please see EDIT 2): https://docs.google.com/presentation/d/1Cg7XDFmq2_Aa7h1ZNdgrmT_UCIm0w02P9Z7Krht46Kw/edit#slide=id.g75ac69d4fb_0_131.

EDIT: found meeting notes for that meeting: https://github.com/tc39/notes/blob/167155eeb708d84e1758d99c88b15670f9b81f75/meetings/2019-12/december-3.md#normative-typedarray-on-prototypes-web-reality.

EDIT 2: actually, the presentation that got consensus doesn't seem to include semantics for OOB indices. As for non-canonical keys, this PR and the presentation agree to invoke OrdinarySet for those.

That matches SpiderMonkey behaviour, while this PR matches V8's. Neither proposals have interop, but both are web-compatible, so in this PR I'm suggesting to match TypedArray's [[Get]] behaviour to suppress further prototype lookup (and not invoke getters upper in prototype chain) for invalid indices.

So formally this PR must receive committee's approval.

@codehag Not only the Reflect.set, but also code like ({ __proto__: new Int32Array(10) })[0] = 1. I can't come up with a use case here, so the goal here is to spec something idiomatic while ensuring NASA's WorldWind web app, which cares only about in-bounds indices, won't break 😄.