Stage 3 Review

[ljharb]

• RecordEqual: can this somehow be iterated with "for each" prose, instead of a loop with a counter?
• RecordEqual/RecordSameValue/RecordSameValueZero don't return a Completion, so it doesn't need to be called with !
• AddPropertyIntoRecordEntriesList/AddValueToTupleSequenceList should probably return ~empty~ so that they're called with Perform, to make it clear they're mutating
• lowercase "Return" when it doesn't start a step (in TupleEqual, in particular)
• why does ToPrimitive have a fast path for Records, but not also for Tuples? (i understand that the lack of a Record prototype is why it's needed for Records)
• SameValue/SameValueZero: the number/bigint "if"s are on two lines, the record/tuple ones are on one - let's make all four the same?
• SameValueNonGeneric: this name is terrible but i don't yet have a better one. It's basically checking non-numeric non-container primitives (not "non-immutable" because all primitives are immutable) - SameValueNonNumericScalarPrimitive is a bit long tho
• SameValueNonGeneric: header needs an Oxford comma in the list of exceptions for x
• CreateRecord: should this say "a new Record exotic object"? or would this be weird because it's a primitive
• IsTuple: needs a comma in step 1 after "Tuple", to disambiguate
• Record [[DefineOwnProperty]]: step 8, needs a comma before "return"
• RecordCreate: use OrdinaryObjectCreate(null, « [[RecordData]] ») instead of directly calling MakeBasicObject
• RecordHasProperty: any reason not to have this call into RecordGet, and branch on ~empty~?
• Tuple [[HasProperty]]/[[Get]]: The GetPrototypeOf() step. I think this could return null, in which case the next line seems invalid. should that be checked for?
• TupleCreate: use OrdinaryObjectCreate(%Tuple.prototype%, « [[TupleData]] » instead of directly calling MakeBasicObject
• Tuple.from step 6.d and step 11 / Tuple.of step 3 / Tuple.prototype.slice step 12 / Tuple.prototype.concat step 6 / Tuple.prototype.filter step 8 / Tuple.prototype.flat step 7 / Tuple.prototype.flatMap step 6 / etc: can these use TupleCreate?
• seems like Tuple.from in the non-usingIterator case, and Tuple.of, could use a shared AO for their repeated steps?

I also filed #347 and #348.

[acutmore]

Thanls @ljharb - much appreciated.

why does ToPrimitive have a fast path for Records, but not also for Tuples? (i understand that the lack of a Record prototype is why it's needed for Records)

Yes the lack of prototype on Record exotic objects and it being frozen requires ToPrimitive to explicitly handle them in some way otherwise it is a TypeError. Tuple exotic objects having a prototype don't have this requirement. I'll open an issue to discuss if they should align with Record instead of the existing primitives. #350

Tuple [[HasProperty]]/[[Get]]: The GetPrototypeOf() step. I think this could return null, in which case the next line seems invalid. should that be checked for?

Right now the Tuple exotic object is non-extensible and always created with %Tuple.prototype%. so GetPrototypeOf can't return null.

[ljharb]

@acutmore then why the ? if it can't throw, and why would Tuple.prototype's [[HasProperty]] be able to throw?

[acutmore]

Yep the completion handling isn’t right, likely a typo rather than intentional. I was only referring to why a null prototype isn’t handled.

[ljharb]

In that case I think it would be appropriate to add an Assert there that the value is Tuple.prototype.

[acutmore]

Adding an assertion makes total sense, I like it.