search

jow- / ucode

JavaScript-like language with optional templating
ISC License
87 stars 24 forks source link

make `"" in [0]` false again #193

Closed eric-j-ason closed 3 months ago

eric-j-ason commented 3 months ago

Before this change: 0 in [""] is true

After this change: 0 in [""] is false

Hitherto, the in operator has used non-strict equality when testing for array membership. Since the non-strict equality 0 == "" is true, this means that 0 in [""] and "" in [0] are surprisingly true. The present pull request switches to strict equality, guided by how it's done in uc_vm_insn_equality.

Honestly, I think that strict equality (as in ===) makes sense in almost all situations and should be what's used under the hood everywhere. I wouldn't mind abolishing non-strict equality altogether.

jow- commented 3 months ago

Good catch, it definitely is supposed to perform strict equality tests. Was it ever correct though? Asking because you wrote "... again" in the PR title.

eric-j-ason commented 3 months ago

Was it ever correct though? Asking because you wrote "... again" in the PR title.

No, I just wanted to make a parody of the well-known saying!

I guess you could say that it was false in our head, then observed to be true in the code, and now rectified to be false again. :)

jow- commented 3 months ago

Understood. I am doing testcases right now and stumbled over another issue, I think that NaN in [ NaN ] should yield true as well. This requires some special casing though.

jow- commented 3 months ago

This also raises another question, how to deal with in in the object case. Should it coerce the test operand to a string or only ever yield true for string values:

"1" in { "1": true } // => true
1 in { "1": true }  // => false?
eric-j-ason commented 3 months ago

Understood. I am doing testcases right now

Great. I was going to, but as in didn't have any yet, I didn't know where to put them or how to make them.

I think that NaN in [ NaN ] should yield true as well. This requires some special casing though.

Right, as NaN is not equal even to itself, I gather. I had a look at IEEE 754, to confirm, and it says "Every NaN shall compare unordered with everything, including itself." I guess that means that it shouldn't be equal to itself, but I'm not completely sure, haha.

This also raises another question, how to deal with in in the object case. Should it coerce the test operand to a string or only ever yield true for string values:

"1" in { "1": true } // => true
1 in { "1": true }  // => false?

Good question. Thinking about it now, I think I come down on not coercing, as coercing leaves the programmer at the whim of whatever internal method is used to convert things to strings. The programmer might not guess, for example, that 1.0 in {"1.0": true} is false. Better, then, do to let the programmer do explicit conversions, if non-strings should be treated as keys.

I tend to generally lean towards being strict with types, thought that perhaps goes against the philosophy behind ucode and JavaScript.

jow- commented 3 months ago

Well after noticing that [] in { "[ ]": 1 } == true I was also convinced that we shouldn't do implicit string conversions when testing object keys. Please follow up in the referenced PR, I am closing this one as some further changes are needed.