tc39 / proposal-record-tuple

ECMAScript proposal for the Record and Tuple value types. | Stage 2: it will change!
https://tc39.es/proposal-record-tuple/
2.5k stars 62 forks source link

Equality semantics for `-0` and `NaN` #65

Closed bakkot closed 2 years ago

bakkot commented 5 years ago

What should each of the following evaluate to?

#[+0] == #[-0];

#[+0] === #[-0];

Object.is(#[+0], #[-0]);

#[NaN] == #[NaN];

#[NaN] === #[NaN];

Object.is(#[NaN], #[NaN]);

(For context, this is non-obvious because +0 === -0 is true, Object.is(+0, -0) is false, NaN === NaN is false, and Object.is(NaN, NaN) is true.)

Personally I lean towards the -0 cases all being false and the NaN cases all being true, so that the unusual equality semantics of -0 and NaN do not propagate to the new kinds of objects being introduced by this proposal.

acusti commented 2 years ago

@papb i can’t speak for anyone else, but i can speak for myself: as a JS developer, i would be truly shocked and perplexed if creating a tuple or record with NaN as a value in it threw an error. for those who assiduously treat NaN as an error state (by convention, because the language doesn’t enforce that), this behavior would most likely be harmless. but for the majority of JS developers, who are not able to enforce such strict avoidance of NaN except when their code is in an error state, it would be a bad surprise. consider:

const savedItemsPerRow = parseInt(localStorage.getItem('itemsPerRow'), 10);
updateStore('userPreferences', #{ itemsPerRow: savedItemsPerRow });
// elsewhere (getItemsPerRow gets userPreferences from the store, then itemsPerRow from that record):
const itemsPerRow = getItemsPerRow(store) || 3;

if records allow NaN, this will work fine even if there is no itemsPerRow key set in localStorage, or if it is set but it can’t be parsed as an int, because NaN is a falsey value, so getItemsPerRow(store) || 3 will return 3 as expected. however, if creating a record with NaN as a value will throw, this code will break in production (though not in dev if the person testing has a value set in localStorage that can be parsed as an int). i show the case of a record here, but obviously it applies equally to tuples.

pygy commented 2 years ago

The surprises for NaN and -0 fundamentally lie in the IEEE-754 spec, and the additional complexity added by how JS handles them (e.g. String(-0) === '0').

How surprising these are depends on how acquainted coders are with those intricacies.

My guess would be that the median dev is not at all familiar with any of this (e.g. I just discovered that the default JSON stringifier culls the minus sign of -0, whereas JSON.parse respects the minus sign), and diking the values out of new parts of the language would be a net improvement for code reliability.

waldemarhorwat commented 2 years ago

There are many contexts in which you want to store data without any need for comparison semantics — in common programming language value categories (example: C++ concepts), storable categories are a large superset of comparable categories.

Disallowing NaNs would be far more damaging than treating records containing them as equal. It would gratuitously break the use case of using records to temporarily store some Numbers and retrieve them later without any interpretation of what Numbers they contain.

sjrd commented 2 years ago

I wish we could generalize that reasoning not to stop at Numbers, but go all the way to any Values. Unfortunately, that's broken because we can't store and retrieve uninterpreted objects. 😞

papb commented 2 years ago

Yeah, I think I'm convinced that throwing on NaN would do more harm than good. Thank you everyone for your inputs.

Maybe NaN itself should not exist as a value. Maybe 1/0 should just throw since the beginning. But it's too late for this, obviously. The reality is that NaN is a value that can be stored in variables, passed around, and all that. Disallowing it in only in records and tuples would prevent things like what @waldemarhorwat said:

It would gratuitously break the use case of using records to temporarily store some Numbers and retrieve them later without any interpretation of what Numbers they contain.

Something else that crossed my mind: it would also become a hassle for people to refactor existing code that uses objects and arrays into records and tuples. The special NaN behavior would be something else to keep in mind (apart from mutability).

I think it might be even possible for TypeScript projects to have automated refactoring to records and tuples, by statically analyzing whether or not each array/object has to be mutable. If NaN was forbidden, this would become impossible...

Maxdamantus commented 2 years ago

https://github.com/tc39/proposal-record-tuple/issues/65#issuecomment-1180494480

@rickbutton I think you missed some parentheses: the second case is -0, because it's calling .add(#[-0]) and thus it's not doing any conversion.

Is this intentional, or has this simply not been addressed yet? I'd expect that Set and Map should either use SameValueZero or normalise -0 to 0 when dealing with R/T keys (the former would do a better job at preserving sign, but the latter would be consistent with the existing normalisation):

Currently specified behaviour according to comment:

const a = -10;
const set = new Set();
set.add(#[a*0, a*0]);
set.has(#[0, 0]); // false, because it has `#[-0, -0]` instead; EDIT: nevermind; it will be true

In general, 0 and -0 should be interchangable (only distinguished by Object.is or during some division-by-zero operation; even (-0).toString() and (0).toString() do the same thing).

Edit: nevermind. It already uses SameValueZero, so it essentially uses the former approach (which might be surprising, but imo would have been the better behaviour even for non-R/T -0 and 0 values).

acutmore commented 2 years ago

@Maxdamantus Map and Set will continue to apply SameValueZero on Records & Tuples with no conversion on insertion.

const s = new Set();
s.add(#[-0]).add(#[0]);
s.size; // 1
Object.is(-0, […s].at(0).at(0)); // true
pygy commented 2 years ago

@papb re. typing IEEE Floats are effectively a Maybe number (number | NaN in a parallel world where TS would have typed them as such) type, with NaN as the Nothing type.

TS could keep track of the operations that can return NaN, make math ops generic, and have them return number | NaN unless they can statically prove that the result isn't NaN.