Review feedback

[rkirsling]

Main points:

1. I was quite confused by the ToString table until I saw #319; if we keep this behavior, we should add a note explaining it.

2. Shouldn't SameValueNonGeneric be called SameValueGeneric? Seems like a typo.

3. Tuple [[GetOwnProperty]] and [[Get]] are attempting to directly access [[Sequence]] on a Tuple exotic object instead of going through [[TupleData]].

4. Is it intentional that Record.fromEntries passes the adder Abstract Closure directly to AddEntriesFromIterable while Tuple.from does CreateBuiltinFunction first?

Nitpicky editorial stuff:

5. RecordPropertyDefinitionEvaluation could use a ? instead of an explicit ReturnIfAbrupt for PropertyName : AssignmentExpression.

6. In TupleSequenceAccumulation, the use of SpreadElement seems to align with Array, but there would be less redundancy if we align with Record and use TupleElement : AssignmentExpression | ... AssignmentExpression instead.

7. Kind of annoying that Tuple's Evaluation comes after TupleSequenceAccumulator instead of before it, though I see that this too is in alignment with Array instead of Record.

[acutmore]

Thanks @rkirsling this is great. We'll work through these items.

[acutmore]

1. I was quite confused by the ToString table until I saw https://github.com/tc39/proposal-record-tuple/pull/319; if we keep this behavior, we should add a note explaining it.

Good idea!

2. Shouldn't SameValueNonGeneric be called SameValueGeneric? Seems like a typo.

This AO is a rename of SamevalueNonNumeric. It was previously nonNumeric because the input is not allowed to be a Number or a BigInt. It is now nonGeneric because the input is even less generic because the parameters can now also not be records or tuples.

3. Tuple [[GetOwnProperty]] and [[Get]] are attempting to directly access [[Sequence]] on a Tuple exotic object instead of going through [[TupleData]].

Great spot!

4. Is it intentional that Record.fromEntries passes the adder Abstract Closure directly to AddEntriesFromIterable while Tuple.from does CreateBuiltinFunction first?

I don't think so, this looks like a spec bug. Thanks!

[ljharb]

I haven't complained about "NonGeneric" yet because I don't have a better suggestion, but my review will hopefully include some :-)

[rkirsling]

This AO is a rename of SamevalueNonNumeric. It was previously nonNumeric because the input is not allowed to be a Number or a BigInt. It is now nonGeneric because the input is even less generic because the parameters can now also not be records or tuples.

Yeah, that's why I was viewing it as a typo. The generic (i.e. default) path used to be describable as "non-numeric" because the non-generic (i.e. special) paths were specifically numeric, but with R&T we now need to be more abstract than that.

[acutmore]

I think I understand the different perspectives now. I am seeing the word generic as "not specific - can handle any input type". So "nonGeneric" seems correct because the AO is specific, it only handles a subset of types. I can also see "Generic" as being interpreted as "handles a related a class of things" similar to WeakSet<T extends object>, where now the AO is 'generic' with a specifier on the group it is generic on.

What do we think a better name could be? Maybe SameValueBase or SameValueCore because it doesn't call out to any other AOs? Or SameValueNonNumericandNonComposite or SVNNNC for short.

[rkirsling]

I would say SameValueDefault or SameValueInner, since it's exclusively used after ruling out all the more specific options. ( Base or Core would certainly be in that direction too; I can't seem to find a precedent in scanning through AOs...)

I think the difficulty lies in trying to name it as if it were an operation with a self-standing reason to exist; from that vantage point, I think the best you could say is SameValueTriviallyComparable.