wren-lang / wren

The Wren Programming Language. Wren is a small, fast, class-based concurrent scripting language.
http://wren.io
MIT License
6.88k stars 552 forks source link

Are 0.0 and -0.0 equal or not? #1052

Open Srinivasa314 opened 3 years ago

Srinivasa314 commented 3 years ago

https://wren.io/try/?code=G4QwTgBAHgvADAKFJAnjAtI5EAWMDeUAXAIwC+CAyigM4AuApgLYB0ADmAJYB2dAFLBgoAlFVqNWHHvxwBtFAF1hQA There seems to be two types of equality (because the bit representation of -0 and 0 differ). I am not sure whether this is documented anywhere.

mhermier commented 3 years ago

That issue should be value representation dependant. Union tagged values should not be affected.

PureFox48 commented 3 years ago

As far as I'm aware, Wren always treats 0 and -0 as being equal in value even though their underlying floating-point bit representations differ.

However, in the docs for the Object.same method it says:

For value types, this returns true if the objects have equivalent state. In other words, numbers, strings, booleans, and ranges compare by value.

In the interests of clarity, I suggest we add the following sentence to that paragraph:

"Note that the numbers 0 and -0 are treated as being equal in value even though their bit representations differ."

joshgoebel commented 3 years ago

Does this depend at all on the compile options?

PureFox48 commented 3 years ago

I don't think so though @mhermier would know better than me.

The position seems to be governed by the following lines 1407-1410 after the implementation of Object.same in wren_core.c:

// These are defined just so that 0 and -0 are equal, which is specified by
// IEEE 754 even though they have different bit representations.
PRIMITIVE(vm->numClass, "==(_)", num_eqeq);
PRIMITIVE(vm->numClass, "!=(_)", num_bangeq);

So internally they remain distinct which often shows up when you're trying to format numbers for display purposes - negative zero is a nuisance, frankly, in that particular scenario.

mhermier commented 3 years ago

The problem highly lies in https://github.com/wren-lang/wren/blob/accfa598b3cbc54fdfb33cb568055ed6994a4966/src/vm/wren_value.c#L430 and the notion of equality. One would expect that equality implies same hashing, and this is usually the case, but it is not an absolute truth in particular with ieee754 double. Some values are considered equal, but in their representation they can be fundamentally different, and some numbers are not equals despite being the same exact bits.

As @PureFox48 mentioned https://github.com/wren-lang/wren/blob/accfa598b3cbc54fdfb33cb568055ed6994a4966/src/vm/wren_value.h#L803 partially solve the problem, but the normal iee754 equality is applied when WREN_NAN_TAGGING is not defined (I would fix that).

Considering how ieee754 implementation support is important for performance, I would normalize the indexes in wren if this is really an issue. Properly fixing equality and fighting against ieee754 would be a huge performance loss.

Srinivasa314 commented 3 years ago

I think the best way will be to have two types of equality. In Javascript Object.is(-0,0)==false and Object.is(NaN,NaN) == true but -0==0 and NaN!=NaN

mhermier commented 3 years ago

@Srinivasa314 if previous messages were not clear enough, wren already has 2 types of quality: ==(_) and Object.same(_,_).

PureFox48 commented 3 years ago

I've just run this code with the results shown:

System.print(0 == -0)            // true
System.print(Object.same(0, -0)) // false

So I think if we are to document this, wording should be added to the Object's ==(other) and !=(other) operators rather than to Object.same. I'd suggest now:

"Note however that these operators are overridden in the Num class to ensure 0 and -0 are treated as being equal in value even though they have different bit representations. Object.same(0, -0) still returns false because they are different objects."

PureFox48 commented 3 years ago

As you'll see above I've added a pull request for the additional wording so this doesn't get forgotten.

PureFox48 commented 3 years ago

Following a discussion with @ruby0x1, it was decided that it would be more appropriate to document this in the Num class rather than the Object class as we're talking here about specific Num values.

I've therefore closed the original PR in favor of #1055.

Srinivasa314 commented 3 years ago

A similar situation exists for NaN. https://wren.io/try/?code=G4QwTgBADgdiMF4ByBXAtgOjjAUKSM2CAtKptjgMoCeAzgC4CmmUYAljPQBSzwCUVOkxbtOXQv0ENmGVh24B5AEYArRgGN6GWiDSMe2ADS8YfATWkj5XZWs3bd+k4YmmBQA Object.same(_,_) treats positive and negative nans differently.

PureFox48 commented 3 years ago

We looked into the NaN quagmire in some depth in #818.

As a result a Num.nan property was added in v0.4.0 (we already had a Num.isNan property) which is described in the docs as follows:

One value representing a NaN.

Provides a default NaN number suitable for the vm internal values.

@mhermier will correct me if I'm wrong but I think I'm right in saying that Object.same will only be true if the NaNs being compared happen to have the same underlying bit representation (there are several different values used).

However, the == operator will always return false even if the NaNs have the same underlying bit representation and conversely the != operator will always return true. This is what one would expect as it's standard IEEE 754 behavior.

I don't really think any further clarification is required in the docs as everyone knows NaNs are peculiar but, if anyone disagrees, I could add something to the PR.

mhermier commented 3 years ago

The result of Object.same will depends on WREN_NAN_TAGGING. When enabled bit comparison is performed, otherwise ieee754 equality is performed.

Everything else is true.