Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 934dc0d

Browse files
GrabYourPitchforksGrabYourPitchforks
committed
PR feedback
1 parent 4a91277 commit 934dc0d

File tree

4 files changed

+52
-57
lines changed

4 files changed

+52
-57
lines changed

‎src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -862,8 +862,6 @@ private static unsafe bool IsSortable(char *text, int length)
862862
internal unsafe int GetHashCodeOfStringCore(ReadOnlySpan<char> source, CompareOptions options)
863863
{
864864
Debug.Assert(!_invariantMode);
865-
866-
Debug.Assert(source != null);
867865
Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);
868866

869867
if (source.Length == 0)

‎src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,10 @@ internal static int LastIndexOfOrdinalCore(string source, string value, int star
111111

112112
return FindStringOrdinal(FIND_FROMEND, source, startIndex - count + 1, count, value, value.Length, ignoreCase);
113113
}
114-
114+
115115
private unsafe int GetHashCodeOfStringCore(ReadOnlySpan<char> source, CompareOptions options)
116116
{
117117
Debug.Assert(!_invariantMode);
118-
119-
Debug.Assert(source != null);
120118
Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);
121119

122120
if (source.Length == 0)
@@ -130,7 +128,7 @@ private unsafe int GetHashCodeOfStringCore(ReadOnlySpan<char> source, CompareOpt
130128
{
131129
int sortKeyLength = Interop.Kernel32.LCMapStringEx(_sortHandle != IntPtr.Zero ? null : _sortName,
132130
flags,
133-
pSource, source.Length,
131+
pSource, source.Length /* in chars */,
134132
null, 0,
135133
null, null, _sortHandle);
136134
if (sortKeyLength == 0)
@@ -152,7 +150,7 @@ private unsafe int GetHashCodeOfStringCore(ReadOnlySpan<char> source, CompareOpt
152150
{
153151
if (Interop.Kernel32.LCMapStringEx(_sortHandle != IntPtr.Zero ? null : _sortName,
154152
flags,
155-
pSource, source.Length,
153+
pSource, source.Length /* in chars */,
156154
pSortKey, sortKeyLength,
157155
null, null, _sortHandle) != sortKeyLength)
158156
{

‎src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,56 +1420,71 @@ internal int GetHashCodeOfString(string source, CompareOptions options)
14201420
{
14211421
throw new ArgumentNullException(nameof(source));
14221422
}
1423+
if ((options & ValidHashCodeOfStringMaskOffFlags) == 0)
1424+
{
1425+
// No unsupported flags are set - continue on with the regular logic
14231426

1424-
return GetHashCodeOfString(source.AsSpan(), options);
1425-
}
1427+
if (_invariantMode)
1428+
{
1429+
return ((options & CompareOptions.IgnoreCase) != 0) ? source.GetHashCodeOrdinalIgnoreCase() : source.GetHashCode();
1430+
}
14261431

1427-
internal int GetHashCodeOfString(ReadOnlySpan<char> source, CompareOptions options)
1428-
{
1429-
//
1430-
// Parameter validation
1431-
//
1432-
if ((options & ValidHashCodeOfStringMaskOffFlags) != 0)
1432+
return GetHashCodeOfStringCore(source, options);
1433+
}
1434+
else if (options == CompareOptions.Ordinal)
14331435
{
1434-
throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options));
1436+
// We allow Ordinal in isolation
1437+
return source.GetHashCode();
14351438
}
1436-
1437-
if (_invariantMode)
1439+
else if (options == CompareOptions.OrdinalIgnoreCase)
14381440
{
1439-
return ((options & CompareOptions.IgnoreCase) != 0) ? string.GetHashCodeOrdinalIgnoreCase(source) : string.GetHashCode(source);
1441+
// We allow OrdinalIgnoreCase in isolation
1442+
return source.GetHashCodeOrdinalIgnoreCase();
1443+
}
1444+
else
1445+
{
1446+
// Unsupported combination of flags specified
1447+
throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options));
14401448
}
1441-
1442-
return GetHashCodeOfStringCore(source, options);
14431449
}
14441450

14451451
public virtual int GetHashCode(string source, CompareOptions options)
14461452
{
1447-
if (source == null)
1448-
{
1449-
throw new ArgumentNullException(nameof(source));
1450-
}
1451-
1452-
return GetHashCode(source.AsSpan(), options);
1453+
// virtual method delegates to non-virtual method
1454+
return GetHashCodeOfString(source, options);
14531455
}
14541456

14551457
public int GetHashCode(ReadOnlySpan<char> source, CompareOptions options)
14561458
{
1457-
if (options == CompareOptions.Ordinal)
1459+
//
1460+
// Parameter validation
1461+
//
1462+
if ((options & ValidHashCodeOfStringMaskOffFlags) == 0)
1463+
{
1464+
// No unsupported flags are set - continue on with the regular logic
1465+
1466+
if (_invariantMode)
1467+
{
1468+
return ((options & CompareOptions.IgnoreCase) != 0) ? string.GetHashCodeOrdinalIgnoreCase(source) : string.GetHashCode(source);
1469+
}
1470+
1471+
return GetHashCodeOfStringCore(source, options);
1472+
}
1473+
else if (options == CompareOptions.Ordinal)
14581474
{
1475+
// We allow Ordinal in isolation
14591476
return string.GetHashCode(source);
14601477
}
1461-
1462-
if (options == CompareOptions.OrdinalIgnoreCase)
1478+
else if (options == CompareOptions.OrdinalIgnoreCase)
14631479
{
1480+
// We allow OrdinalIgnoreCase in isolation
14641481
return string.GetHashCodeOrdinalIgnoreCase(source);
14651482
}
1466-
1467-
//
1468-
// GetHashCodeOfString does more parameters validation. basically will throw when
1469-
// having Ordinal, OrdinalIgnoreCase and StringSort
1470-
//
1471-
1472-
return GetHashCodeOfString(source, options);
1483+
else
1484+
{
1485+
// Unsupported combination of flags specified
1486+
throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options));
1487+
}
14731488
}
14741489

14751490
////////////////////////////////////////////////////////////////////////

‎src/System.Private.CoreLib/shared/System/String.Comparison.cs

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -773,30 +773,15 @@ public static int GetHashCode(ReadOnlySpan<char> value)
773773
// A span-based equivalent of String.GetHashCode(StringComparison). Uses the specified comparison type.
774774
public static int GetHashCode(ReadOnlySpan<char> value, StringComparison comparisonType)
775775
{
776-
CultureInfo culture;
777-
CompareOptions compareOptions;
778-
779776
switch (comparisonType)
780777
{
781778
case StringComparison.CurrentCulture:
782-
culture = CultureInfo.CurrentCulture;
783-
compareOptions = CompareOptions.None;
784-
break;
785-
786779
case StringComparison.CurrentCultureIgnoreCase:
787-
culture = CultureInfo.CurrentCulture;
788-
compareOptions = CompareOptions.IgnoreCase;
789-
break;
780+
return CultureInfo.CurrentCulture.CompareInfo.GetHashCode(value, GetCaseCompareOfComparisonCulture(comparisonType));
790781

791782
case StringComparison.InvariantCulture:
792-
culture = CultureInfo.InvariantCulture;
793-
compareOptions = CompareOptions.None;
794-
break;
795-
796783
case StringComparison.InvariantCultureIgnoreCase:
797-
culture = CultureInfo.InvariantCulture;
798-
compareOptions = CompareOptions.IgnoreCase;
799-
break;
784+
return CultureInfo.InvariantCulture.CompareInfo.GetHashCode(value, GetCaseCompareOfComparisonCulture(comparisonType));
800785

801786
case StringComparison.Ordinal:
802787
return GetHashCode(value);
@@ -806,10 +791,9 @@ public static int GetHashCode(ReadOnlySpan<char> value, StringComparison compari
806791

807792
default:
808793
ThrowHelper.ThrowArgumentException(ExceptionResource.NotSupported_StringComparison, ExceptionArgument.comparisonType);
809-
throw null; // shouldn't be hit
794+
Debug.Fail("Should not reach this point.");
795+
return default;
810796
}
811-
812-
return culture.CompareInfo.GetHashCodeOfString(value, compareOptions);
813797
}
814798

815799
[MethodImpl(MethodImplOptions.AggressiveInlining)]

0 commit comments

Comments
 (0)