From 6647c6f50230897ea426c875d635c11c4c45e0f7 Mon Sep 17 00:00:00 2001 From: lilinus Date: Thu, 25 Apr 2024 11:17:14 +0200 Subject: [PATCH 1/4] Simplify some CompareTo methods --- .../src/System/DateTimeOffset.cs | 18 ++---- .../System.Private.CoreLib/src/System/Guid.cs | 61 +------------------ .../System/Runtime/InteropServices/NFloat.cs | 14 +---- 3 files changed, 7 insertions(+), 86 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs b/src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs index 77f1153181f67..fce89e9b41da4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs +++ b/src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs @@ -571,26 +571,16 @@ public static int Compare(DateTimeOffset first, DateTimeOffset second) => int IComparable.CompareTo(object? obj) { if (obj == null) return 1; - if (!(obj is DateTimeOffset)) + if (obj is not DateTimeOffset other) { throw new ArgumentException(SR.Arg_MustBeDateTimeOffset); } - DateTime objUtc = ((DateTimeOffset)obj).UtcDateTime; - DateTime utc = UtcDateTime; - if (utc > objUtc) return 1; - if (utc < objUtc) return -1; - return 0; + return CompareTo(other); } - public int CompareTo(DateTimeOffset other) - { - DateTime otherUtc = other.UtcDateTime; - DateTime utc = UtcDateTime; - if (utc > otherUtc) return 1; - if (utc < otherUtc) return -1; - return 0; - } + public int CompareTo(DateTimeOffset other) => + UtcDateTime.CompareTo(other.UtcDateTime); // Checks if this DateTimeOffset is equal to a given object. Returns // true if the given object is a boxed DateTimeOffset and its value diff --git a/src/libraries/System.Private.CoreLib/src/System/Guid.cs b/src/libraries/System.Private.CoreLib/src/System/Guid.cs index 911fb9f7184a3..7ce5edfc3fbb8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Guid.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Guid.cs @@ -958,68 +958,11 @@ public int CompareTo(object? value) { return 1; } - if (!(value is Guid)) + if (value is not Guid other) { throw new ArgumentException(SR.Arg_MustBeGuid, nameof(value)); } - Guid g = (Guid)value; - - if (g._a != _a) - { - return GetResult((uint)_a, (uint)g._a); - } - - if (g._b != _b) - { - return GetResult((uint)_b, (uint)g._b); - } - - if (g._c != _c) - { - return GetResult((uint)_c, (uint)g._c); - } - - if (g._d != _d) - { - return GetResult(_d, g._d); - } - - if (g._e != _e) - { - return GetResult(_e, g._e); - } - - if (g._f != _f) - { - return GetResult(_f, g._f); - } - - if (g._g != _g) - { - return GetResult(_g, g._g); - } - - if (g._h != _h) - { - return GetResult(_h, g._h); - } - - if (g._i != _i) - { - return GetResult(_i, g._i); - } - - if (g._j != _j) - { - return GetResult(_j, g._j); - } - - if (g._k != _k) - { - return GetResult(_k, g._k); - } - - return 0; + return CompareTo(other); } public int CompareTo(Guid value) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NFloat.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NFloat.cs index 37c39bbc029a2..273a8a3d31f6b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NFloat.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NFloat.cs @@ -780,19 +780,7 @@ public int CompareTo(object? obj) { if (obj is NFloat other) { - if (_value < other._value) return -1; - if (_value > other._value) return 1; - if (_value == other._value) return 0; - - // At least one of the values is NaN. - if (NativeType.IsNaN(_value)) - { - return NativeType.IsNaN(other._value) ? 0 : -1; - } - else - { - return 1; - } + return CompareTo(other); } else if (obj is null) { From 5dab47d411871a4eb0be77968046cc528ae04f76 Mon Sep 17 00:00:00 2001 From: lilinus Date: Thu, 25 Apr 2024 11:21:52 +0200 Subject: [PATCH 2/4] Simplify some more CompareTo methods --- src/libraries/System.Private.CoreLib/src/System/Byte.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/Char.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/Decimal.cs | 5 ++--- src/libraries/System.Private.CoreLib/src/System/Int16.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/Int32.cs | 6 +----- src/libraries/System.Private.CoreLib/src/System/Int64.cs | 6 +----- src/libraries/System.Private.CoreLib/src/System/SByte.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/UInt16.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/UInt32.cs | 6 +----- src/libraries/System.Private.CoreLib/src/System/UInt64.cs | 6 +----- 10 files changed, 16 insertions(+), 33 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Byte.cs b/src/libraries/System.Private.CoreLib/src/System/Byte.cs index 31a2bccf21bf8..140f1ec2844ad 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Byte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Byte.cs @@ -58,12 +58,12 @@ public int CompareTo(object? value) { return 1; } - if (!(value is byte)) + if (value is not byte other) { throw new ArgumentException(SR.Arg_MustBeByte); } - return m_value - (((byte)value).m_value); + return CompareTo(other); } public int CompareTo(byte value) diff --git a/src/libraries/System.Private.CoreLib/src/System/Char.cs b/src/libraries/System.Private.CoreLib/src/System/Char.cs index a44d61f61a2ae..257cf2167ad7e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Char.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Char.cs @@ -140,12 +140,12 @@ public int CompareTo(object? value) { return 1; } - if (!(value is char)) + if (value is not char other) { throw new ArgumentException(SR.Arg_MustBeChar); } - return m_value - ((char)value).m_value; + return CompareTo(other); } public int CompareTo(char value) diff --git a/src/libraries/System.Private.CoreLib/src/System/Decimal.cs b/src/libraries/System.Private.CoreLib/src/System/Decimal.cs index f2d89445f834b..e794eb2ee582d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Decimal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Decimal.cs @@ -420,11 +420,10 @@ public int CompareTo(object? value) { if (value == null) return 1; - if (!(value is decimal)) + if (value is not decimal other) throw new ArgumentException(SR.Arg_MustBeDecimal); - decimal other = (decimal)value; - return DecCalc.VarDecCmp(in this, in other); + return CompareTo(other); } public int CompareTo(decimal value) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int16.cs b/src/libraries/System.Private.CoreLib/src/System/Int16.cs index af144fb860c92..cfc49aa946a6b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int16.cs @@ -60,9 +60,9 @@ public int CompareTo(object? value) return 1; } - if (value is short) + if (value is short other) { - return m_value - ((short)value).m_value; + return CompareTo(other); } throw new ArgumentException(SR.Arg_MustBeInt16); diff --git a/src/libraries/System.Private.CoreLib/src/System/Int32.cs b/src/libraries/System.Private.CoreLib/src/System/Int32.cs index 2f91c2c2a5970..dd3431600aade 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int32.cs @@ -63,13 +63,9 @@ public int CompareTo(object? value) return 1; } - // NOTE: Cannot use return (_value - value) as this causes a wrap - // around in cases where _value - value > MaxValue. if (value is int i) { - if (m_value < i) return -1; - if (m_value > i) return 1; - return 0; + return CompareTo(i); } throw new ArgumentException(SR.Arg_MustBeInt32); diff --git a/src/libraries/System.Private.CoreLib/src/System/Int64.cs b/src/libraries/System.Private.CoreLib/src/System/Int64.cs index ca170d8c903ed..a68b2964687e3 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int64.cs @@ -60,13 +60,9 @@ public int CompareTo(object? value) return 1; } - // Need to use compare because subtraction will wrap - // to positive for very large neg numbers, etc. if (value is long i) { - if (m_value < i) return -1; - if (m_value > i) return 1; - return 0; + return CompareTo(i); } throw new ArgumentException(SR.Arg_MustBeInt64); diff --git a/src/libraries/System.Private.CoreLib/src/System/SByte.cs b/src/libraries/System.Private.CoreLib/src/System/SByte.cs index fb4734d41b69f..dff18a50ec7b6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SByte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SByte.cs @@ -62,11 +62,11 @@ public int CompareTo(object? obj) { return 1; } - if (!(obj is sbyte)) + if (obj is not sbyte other) { throw new ArgumentException(SR.Arg_MustBeSByte); } - return m_value - ((sbyte)obj).m_value; + return CompareTo(other); } public int CompareTo(sbyte value) diff --git a/src/libraries/System.Private.CoreLib/src/System/UInt16.cs b/src/libraries/System.Private.CoreLib/src/System/UInt16.cs index fcc863a6310a8..bdadbd9782e6d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/UInt16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/UInt16.cs @@ -56,9 +56,9 @@ public int CompareTo(object? value) { return 1; } - if (value is ushort) + if (value is ushort other) { - return (int)m_value - (int)(((ushort)value).m_value); + return CompareTo(other); } throw new ArgumentException(SR.Arg_MustBeUInt16); } diff --git a/src/libraries/System.Private.CoreLib/src/System/UInt32.cs b/src/libraries/System.Private.CoreLib/src/System/UInt32.cs index 8487630193ec5..782102b7c8065 100644 --- a/src/libraries/System.Private.CoreLib/src/System/UInt32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/UInt32.cs @@ -57,13 +57,9 @@ public int CompareTo(object? value) return 1; } - // Need to use compare because subtraction will wrap - // to positive for very large neg numbers, etc. if (value is uint i) { - if (m_value < i) return -1; - if (m_value > i) return 1; - return 0; + return CompareTo(i); } throw new ArgumentException(SR.Arg_MustBeUInt32); diff --git a/src/libraries/System.Private.CoreLib/src/System/UInt64.cs b/src/libraries/System.Private.CoreLib/src/System/UInt64.cs index dc12c5c006a82..35992be8d1c8f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/UInt64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/UInt64.cs @@ -57,13 +57,9 @@ public int CompareTo(object? value) return 1; } - // Need to use compare because subtraction will wrap - // to positive for very large neg numbers, etc. if (value is ulong i) { - if (m_value < i) return -1; - if (m_value > i) return 1; - return 0; + return CompareTo(i); } throw new ArgumentException(SR.Arg_MustBeUInt64); From 73a141836025e3a8c7905ca1713601d30c5d87a0 Mon Sep 17 00:00:00 2001 From: lilinus Date: Thu, 25 Apr 2024 15:31:55 +0200 Subject: [PATCH 3/4] Revert "Simplify some more CompareTo methods" This reverts commit 5dab47d411871a4eb0be77968046cc528ae04f76. --- src/libraries/System.Private.CoreLib/src/System/Byte.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/Char.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/Decimal.cs | 5 +++-- src/libraries/System.Private.CoreLib/src/System/Int16.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/Int32.cs | 6 +++++- src/libraries/System.Private.CoreLib/src/System/Int64.cs | 6 +++++- src/libraries/System.Private.CoreLib/src/System/SByte.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/UInt16.cs | 4 ++-- src/libraries/System.Private.CoreLib/src/System/UInt32.cs | 6 +++++- src/libraries/System.Private.CoreLib/src/System/UInt64.cs | 6 +++++- 10 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Byte.cs b/src/libraries/System.Private.CoreLib/src/System/Byte.cs index 140f1ec2844ad..31a2bccf21bf8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Byte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Byte.cs @@ -58,12 +58,12 @@ public int CompareTo(object? value) { return 1; } - if (value is not byte other) + if (!(value is byte)) { throw new ArgumentException(SR.Arg_MustBeByte); } - return CompareTo(other); + return m_value - (((byte)value).m_value); } public int CompareTo(byte value) diff --git a/src/libraries/System.Private.CoreLib/src/System/Char.cs b/src/libraries/System.Private.CoreLib/src/System/Char.cs index 257cf2167ad7e..a44d61f61a2ae 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Char.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Char.cs @@ -140,12 +140,12 @@ public int CompareTo(object? value) { return 1; } - if (value is not char other) + if (!(value is char)) { throw new ArgumentException(SR.Arg_MustBeChar); } - return CompareTo(other); + return m_value - ((char)value).m_value; } public int CompareTo(char value) diff --git a/src/libraries/System.Private.CoreLib/src/System/Decimal.cs b/src/libraries/System.Private.CoreLib/src/System/Decimal.cs index e794eb2ee582d..f2d89445f834b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Decimal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Decimal.cs @@ -420,10 +420,11 @@ public int CompareTo(object? value) { if (value == null) return 1; - if (value is not decimal other) + if (!(value is decimal)) throw new ArgumentException(SR.Arg_MustBeDecimal); - return CompareTo(other); + decimal other = (decimal)value; + return DecCalc.VarDecCmp(in this, in other); } public int CompareTo(decimal value) diff --git a/src/libraries/System.Private.CoreLib/src/System/Int16.cs b/src/libraries/System.Private.CoreLib/src/System/Int16.cs index cfc49aa946a6b..af144fb860c92 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int16.cs @@ -60,9 +60,9 @@ public int CompareTo(object? value) return 1; } - if (value is short other) + if (value is short) { - return CompareTo(other); + return m_value - ((short)value).m_value; } throw new ArgumentException(SR.Arg_MustBeInt16); diff --git a/src/libraries/System.Private.CoreLib/src/System/Int32.cs b/src/libraries/System.Private.CoreLib/src/System/Int32.cs index dd3431600aade..2f91c2c2a5970 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int32.cs @@ -63,9 +63,13 @@ public int CompareTo(object? value) return 1; } + // NOTE: Cannot use return (_value - value) as this causes a wrap + // around in cases where _value - value > MaxValue. if (value is int i) { - return CompareTo(i); + if (m_value < i) return -1; + if (m_value > i) return 1; + return 0; } throw new ArgumentException(SR.Arg_MustBeInt32); diff --git a/src/libraries/System.Private.CoreLib/src/System/Int64.cs b/src/libraries/System.Private.CoreLib/src/System/Int64.cs index a68b2964687e3..ca170d8c903ed 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Int64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Int64.cs @@ -60,9 +60,13 @@ public int CompareTo(object? value) return 1; } + // Need to use compare because subtraction will wrap + // to positive for very large neg numbers, etc. if (value is long i) { - return CompareTo(i); + if (m_value < i) return -1; + if (m_value > i) return 1; + return 0; } throw new ArgumentException(SR.Arg_MustBeInt64); diff --git a/src/libraries/System.Private.CoreLib/src/System/SByte.cs b/src/libraries/System.Private.CoreLib/src/System/SByte.cs index dff18a50ec7b6..fb4734d41b69f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SByte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SByte.cs @@ -62,11 +62,11 @@ public int CompareTo(object? obj) { return 1; } - if (obj is not sbyte other) + if (!(obj is sbyte)) { throw new ArgumentException(SR.Arg_MustBeSByte); } - return CompareTo(other); + return m_value - ((sbyte)obj).m_value; } public int CompareTo(sbyte value) diff --git a/src/libraries/System.Private.CoreLib/src/System/UInt16.cs b/src/libraries/System.Private.CoreLib/src/System/UInt16.cs index bdadbd9782e6d..fcc863a6310a8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/UInt16.cs +++ b/src/libraries/System.Private.CoreLib/src/System/UInt16.cs @@ -56,9 +56,9 @@ public int CompareTo(object? value) { return 1; } - if (value is ushort other) + if (value is ushort) { - return CompareTo(other); + return (int)m_value - (int)(((ushort)value).m_value); } throw new ArgumentException(SR.Arg_MustBeUInt16); } diff --git a/src/libraries/System.Private.CoreLib/src/System/UInt32.cs b/src/libraries/System.Private.CoreLib/src/System/UInt32.cs index 782102b7c8065..8487630193ec5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/UInt32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/UInt32.cs @@ -57,9 +57,13 @@ public int CompareTo(object? value) return 1; } + // Need to use compare because subtraction will wrap + // to positive for very large neg numbers, etc. if (value is uint i) { - return CompareTo(i); + if (m_value < i) return -1; + if (m_value > i) return 1; + return 0; } throw new ArgumentException(SR.Arg_MustBeUInt32); diff --git a/src/libraries/System.Private.CoreLib/src/System/UInt64.cs b/src/libraries/System.Private.CoreLib/src/System/UInt64.cs index 35992be8d1c8f..dc12c5c006a82 100644 --- a/src/libraries/System.Private.CoreLib/src/System/UInt64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/UInt64.cs @@ -57,9 +57,13 @@ public int CompareTo(object? value) return 1; } + // Need to use compare because subtraction will wrap + // to positive for very large neg numbers, etc. if (value is ulong i) { - return CompareTo(i); + if (m_value < i) return -1; + if (m_value > i) return 1; + return 0; } throw new ArgumentException(SR.Arg_MustBeUInt64); From 957cb10be3a4eb939f2340b8632f252cf3e52839 Mon Sep 17 00:00:00 2001 From: lilinus Date: Wed, 8 May 2024 11:18:21 +0200 Subject: [PATCH 4/4] Use DateTime.CompareTo in DatetimeOffset --- .../System.Private.CoreLib/src/System/DateTimeOffset.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs b/src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs index fce89e9b41da4..5baf1e5076b6e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs +++ b/src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs @@ -576,11 +576,11 @@ int IComparable.CompareTo(object? obj) throw new ArgumentException(SR.Arg_MustBeDateTimeOffset); } - return CompareTo(other); + return DateTime.Compare(UtcDateTime, other.UtcDateTime); } public int CompareTo(DateTimeOffset other) => - UtcDateTime.CompareTo(other.UtcDateTime); + DateTime.Compare(UtcDateTime, other.UtcDateTime); // Checks if this DateTimeOffset is equal to a given object. Returns // true if the given object is a boxed DateTimeOffset and its value