Skip to content

Commit

Permalink
Fixing Nan=Nan behavior for Set and Map collection types (#14515)
Browse files Browse the repository at this point in the history
* Fixing Nan=Nan behavior for Set and Map collection types

* Fixing incorrect test asserts, updating expected length of Fsharp.core in bytes
  • Loading branch information
T-Gro authored Jan 3, 2023
1 parent f64b68b commit a9c194a
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 6 deletions.
35 changes: 35 additions & 0 deletions src/FSharp.Core/map.fs
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,41 @@ type Map<[<EqualityConditionalOn>] 'Key, [<EqualityConditionalOn; ComparisonCond
override this.GetHashCode() =
this.ComputeHashCode()

interface IStructuralEquatable with
member this.Equals(that, comparer) =
match that with
| :? Map<'Key, 'Value> as that ->
use e1 = (this :> seq<_>).GetEnumerator()
use e2 = (that :> seq<_>).GetEnumerator()

let rec loop () =
let m1 = e1.MoveNext()
let m2 = e2.MoveNext()

(m1 = m2)
&& (not m1
|| (let e1c = e1.Current
let e2c = e2.Current

(comparer.Equals(e1c.Key, e2c.Key)
&& comparer.Equals(e1c.Value, e2c.Value)
&& loop ())))

loop ()
| _ -> false

member this.GetHashCode(comparer) =
let combineHash x y =
(x <<< 1) + y + 631

let mutable res = 0

for (KeyValue (x, y)) in this do
res <- combineHash res (comparer.GetHashCode x)
res <- combineHash res (comparer.GetHashCode y)

res

interface IEnumerable<KeyValuePair<'Key, 'Value>> with
member _.GetEnumerator() =
MapTree.mkIEnumerator tree
Expand Down
1 change: 1 addition & 0 deletions src/FSharp.Core/map.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ type Map<[<EqualityConditionalOn>] 'Key, [<EqualityConditionalOn; ComparisonCond
interface ICollection<KeyValuePair<'Key, 'Value>>
interface IEnumerable<KeyValuePair<'Key, 'Value>>
interface System.IComparable
interface System.Collections.IStructuralEquatable
interface System.Collections.IEnumerable
interface IReadOnlyCollection<KeyValuePair<'Key, 'Value>>
interface IReadOnlyDictionary<'Key, 'Value>
Expand Down
28 changes: 27 additions & 1 deletion src/FSharp.Core/set.fs
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ type Set<[<EqualityConditionalOn>] 'T when 'T: comparison>(comparer: IComparer<'
member x.ToArray() =
SetTree.toArray x.Tree

member this.ComputeHashCode() =
member private this.ComputeHashCode() =
let combineHash x y =
(x <<< 1) + y + 631

Expand Down Expand Up @@ -904,6 +904,32 @@ type Set<[<EqualityConditionalOn>] 'T when 'T: comparison>(comparer: IComparer<'
member this.CompareTo(that: obj) =
SetTree.compare this.Comparer this.Tree ((that :?> Set<'T>).Tree)

interface IStructuralEquatable with
member this.Equals(that, comparer) =
match that with
| :? Set<'T> as that ->
use e1 = (this :> seq<_>).GetEnumerator()
use e2 = (that :> seq<_>).GetEnumerator()

let rec loop () =
let m1 = e1.MoveNext()
let m2 = e2.MoveNext()
(m1 = m2) && (not m1 || ((comparer.Equals(e1.Current, e2.Current)) && loop ()))

loop ()
| _ -> false

member this.GetHashCode(comparer) =
let combineHash x y =
(x <<< 1) + y + 631

let mutable res = 0

for x in this do
res <- combineHash res (comparer.GetHashCode(x))

res

interface ICollection<'T> with
member s.Add x =
ignore x
Expand Down
1 change: 1 addition & 0 deletions src/FSharp.Core/set.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ type Set<[<EqualityConditionalOn>] 'T when 'T: comparison> =
interface IEnumerable<'T>
interface System.Collections.IEnumerable
interface System.IComparable
interface System.Collections.IStructuralEquatable
interface IReadOnlyCollection<'T>
override Equals: obj -> bool

Expand Down
39 changes: 39 additions & 0 deletions tests/FSharp.Core.UnitTests/FSharp.Core/PrimTypes.fs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,45 @@ type LanguagePrimitivesModule() =

let resultNul = LanguagePrimitives.GenericEquality "ABC" null
Assert.False(resultNul)

[<Fact>]
member _.GenericEqualityForNans() =
Assert.DoesNotContain(true,
[| LanguagePrimitives.GenericEquality nan nan
LanguagePrimitives.GenericEquality [nan] [nan]
LanguagePrimitives.GenericEquality [|nan|] [|nan|]
LanguagePrimitives.GenericEquality (Set.ofList [nan]) (Set.ofList [nan])
LanguagePrimitives.GenericEquality (Map.ofList [1,nan]) (Map.ofList [1,nan])
LanguagePrimitives.GenericEquality (Map.ofList [nan,1]) (Map.ofList [nan,1])
LanguagePrimitives.GenericEquality (Map.ofList [nan,nan]) (Map.ofList [nan,nan])

LanguagePrimitives.GenericEquality nanf nanf
LanguagePrimitives.GenericEquality [nanf] [nanf]
LanguagePrimitives.GenericEquality [|nanf|] [|nanf|]
LanguagePrimitives.GenericEquality (Set.ofList [nanf]) (Set.ofList [nanf])
LanguagePrimitives.GenericEquality (Map.ofList [1,nanf]) (Map.ofList [1,nanf])
LanguagePrimitives.GenericEquality (Map.ofList [nanf,1]) (Map.ofList [nanf,1])
LanguagePrimitives.GenericEquality (Map.ofList [nanf,nanf]) (Map.ofList [nanf,nanf])|])

[<Fact>]
member _.GenericEqualityER() =
Assert.DoesNotContain(false,
[| LanguagePrimitives.GenericEqualityER nan nan
LanguagePrimitives.GenericEqualityER [nan] [nan]
LanguagePrimitives.GenericEqualityER [|nan|] [|nan|]
LanguagePrimitives.GenericEqualityER (Set.ofList [nan]) (Set.ofList [nan])
LanguagePrimitives.GenericEqualityER (Map.ofList [1,nan]) (Map.ofList [1,nan])
LanguagePrimitives.GenericEqualityER (Map.ofList [nan,1]) (Map.ofList [nan,1])
LanguagePrimitives.GenericEqualityER (Map.ofList [nan,nan]) (Map.ofList [nan,nan])

LanguagePrimitives.GenericEqualityER nanf nanf
LanguagePrimitives.GenericEqualityER [nanf] [nanf]
LanguagePrimitives.GenericEqualityER [|nanf|] [|nanf|]
LanguagePrimitives.GenericEqualityER (Set.ofList [nanf]) (Set.ofList [nanf])
LanguagePrimitives.GenericEqualityER (Map.ofList [1,nanf]) (Map.ofList [1,nanf])
LanguagePrimitives.GenericEqualityER (Map.ofList [nanf,1]) (Map.ofList [nanf,1])
LanguagePrimitives.GenericEqualityER (Map.ofList [nanf,nanf]) (Map.ofList [nanf,nanf])|])


[<Fact>]
member this.GenericGreaterOrEqual() =
Expand Down
2 changes: 1 addition & 1 deletion tests/projects/SelfContained_Trimming_Test/check.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ if (-not ($output -eq $expected))
}

# Checking that FSharp.Core binary is of expected size (needs adjustments if test is updated).
$expected_len = 245248 # In bytes
$expected_len = 246272 # In bytes
$file = Get-Item .\bin\Release\net7.0\win-x64\publish\FSharp.Core.dll
$file_len = $file.Length
if (-not ($file_len -eq $expected_len))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ module OptionParserTests =

let actual = OptionParser.getHintKinds options

Assert.AreEqual(expected, actual)
CollectionAssert.AreEquivalent(expected, actual)

[<Test>]
let ``Type hints on, parameter name hints off`` () =
Expand All @@ -36,7 +36,7 @@ module OptionParserTests =

let actual = OptionParser.getHintKinds options

Assert.AreEqual(expected, actual)
CollectionAssert.AreEquivalent(expected, actual)

[<Test>]
let ``Type hints off, parameter name hints on`` () =
Expand All @@ -50,7 +50,7 @@ module OptionParserTests =

let actual = OptionParser.getHintKinds options

Assert.AreEqual(expected, actual)
CollectionAssert.AreEquivalent(expected, actual)

[<Test>]
let ``Type hints on, parameter name hints on`` () =
Expand All @@ -64,4 +64,4 @@ module OptionParserTests =

let actual = OptionParser.getHintKinds options

Assert.AreEqual(expected, actual)
CollectionAssert.AreEquivalent(expected, actual)

0 comments on commit a9c194a

Please sign in to comment.