Skip to content

Commit

Permalink
Map: optimize comparer: limit types to optimize for IComparable
Browse files Browse the repository at this point in the history
Arrays and structural comparison may cause problems

Maybe should just copy the snippet from dotnet#9348
  • Loading branch information
buybackoff committed Jan 10, 2021
1 parent 2a21940 commit e9e22b4
Showing 1 changed file with 22 additions and 13 deletions.
35 changes: 22 additions & 13 deletions src/fsharp/FSharp.Core/map.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
namespace Microsoft.FSharp.Collections

open System
open System.Collections
open System.Collections.Generic
open System.Diagnostics
open System.Numerics
Expand Down Expand Up @@ -39,22 +40,29 @@ module MapTree =
static member private CompareCG<'U when 'U :> IComparable<'U>>(l:'U, r:'U):int = l.CompareTo(r)

// A call to IComparable.CompareTo
// static member private CompareC<'U when 'U :> IComparable>(l:'U, r:'U):int = l.CompareTo(r)
static member private CompareC<'U when 'U :> IComparable>(l:'U, r:'U):int = l.CompareTo(r)

static member val CompareToDlg : Func<'T,'T,int> =
let dlg =
try
// See #816, IComparable<'T> actually does not satisfy comparison constraint, but it should be preferred
if typeof<IComparable<'T>>.IsAssignableFrom(typeof<'T>) then
let m =
typeof<CompareHelper<'T>>.GetMethod("CompareCG", BindingFlags.NonPublic ||| BindingFlags.Static)
.MakeGenericMethod([|typeof<'T>|])
Delegate.CreateDelegate(typeof<Func<'T,'T,int>>, m) :?> Func<'T,'T,int>
// elif typeof<IComparable>.IsAssignableFrom(typeof<'T>) then
// let m =
// typeof<CompareHelper<'T>>.GetMethod("CompareC", BindingFlags.NonPublic ||| BindingFlags.Static)
// .MakeGenericMethod([|typeof<'T>|])
// Delegate.CreateDelegate(typeof<Func<'T,'T,int>>, m) :?> Func<'T,'T,int>
let ty = typeof<'T>
try
if not (typeof<IStructuralComparable>.IsAssignableFrom(ty))
&& isNull (Attribute.GetCustomAttribute(ty, typeof<NoComparisonAttribute>))
&& isNull (Attribute.GetCustomAttribute(ty, typeof<StructuralComparisonAttribute>))
&& not (ty.IsArray) then

// See #816, IComparable<'T> actually does not satisfy comparison constraint, but it should be preferred
if typeof<IComparable<'T>>.IsAssignableFrom(ty) then
let m =
typeof<CompareHelper<'T>>.GetMethod("CompareCG", BindingFlags.NonPublic ||| BindingFlags.Static)
.MakeGenericMethod([|ty|])
Delegate.CreateDelegate(typeof<Func<'T,'T,int>>, m) :?> Func<'T,'T,int>
elif typeof<IComparable>.IsAssignableFrom(ty) then
let m =
typeof<CompareHelper<'T>>.GetMethod("CompareC", BindingFlags.NonPublic ||| BindingFlags.Static)
.MakeGenericMethod([|typeof<'T>|])
Delegate.CreateDelegate(typeof<Func<'T,'T,int>>, m) :?> Func<'T,'T,int>
else null
else null
with _ -> null
dlg
Expand Down Expand Up @@ -128,6 +136,7 @@ module MapTree =
else if Type.op_Equality(typeof<'T>, typeof<TimeSpan>) then unbox<TimeSpan>(box(l)).CompareTo(unbox<TimeSpan>(box(r)))

else if Type.op_Equality(typeof<'T>, typeof<BigInteger>) then unbox<BigInteger>(box(l)).CompareTo(unbox<BigInteger>(box(r)))
else if Type.op_Equality(typeof<'T>, typeof<Guid>) then unbox<Guid>(box(l)).CompareTo(unbox<Guid>(box(r)))

else if Type.op_Equality(typeof<'T>, typeof<string>) then
// same as in GenericComparisonFast
Expand Down

0 comments on commit e9e22b4

Please sign in to comment.