Skip to content

Prep CompareInfo for spanification #32385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 14, 2020
Merged

Prep CompareInfo for spanification #32385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1fb2514
Update SortKey.GetHashCode not to call directly into Marvin
GrabYourPitchforks Feb 13, 2020
f23d3c5
Spanify CompareInfo p/invoke APIs
GrabYourPitchforks Feb 13, 2020
82f98ea
Only call GetLastError under debug mode
GrabYourPitchforks Mar 13, 2020
b3e0855
PR feedback - cleanup signature of FindStringOrdinal
GrabYourPitchforks Mar 13, 2020
8dda77a
PR feedback - use BOOL over bool
GrabYourPitchforks Mar 13, 2020
d76a5ed
Fix missing edge case checks in IndexOfOrdinal
GrabYourPitchforks Mar 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Globalization.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ internal static partial class Interop
{
internal static unsafe partial class Kernel32
{
// Under debug mode only, we'll want to check the error codes
// of some of the p/invokes we make.

#if DEBUG
private const bool SetLastErrorForDebug = true;
#else
private const bool SetLastErrorForDebug = false;
#endif

internal const uint LOCALE_ALLOW_NEUTRAL_NAMES = 0x08000000; // Flag to allow returning neutral names/lcids for name conversion
internal const uint LOCALE_ILANGUAGE = 0x00000001;
internal const uint LOCALE_SUPPLEMENTAL = 0x00000002;
Expand Down Expand Up @@ -52,7 +61,7 @@ internal static extern int LCMapStringEx(
void* lpReserved,
IntPtr sortHandle);

[DllImport("kernel32.dll", EntryPoint = "FindNLSStringEx")]
[DllImport("kernel32.dll", EntryPoint = "FindNLSStringEx", SetLastError = SetLastErrorForDebug)]
internal static extern int FindNLSStringEx(
char* lpLocaleName,
uint dwFindNLSStringFlags,
Expand Down Expand Up @@ -85,14 +94,14 @@ internal static extern int CompareStringOrdinal(
int cchCount2,
bool bIgnoreCase);

[DllImport("kernel32.dll", EntryPoint = "FindStringOrdinal")]
[DllImport("kernel32.dll", EntryPoint = "FindStringOrdinal", SetLastError = SetLastErrorForDebug)]
internal static extern int FindStringOrdinal(
uint dwFindStringOrdinalFlags,
char* lpStringSource,
int cchSource,
char* lpStringValue,
int cchValue,
int bIgnoreCase);
BOOL bIgnoreCase);

[DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
internal static extern bool IsNLSDefinedString(
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Invariant.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ private SortKey InvariantCreateSortKey(string source, CompareOptions options)
}
}

return new SortKey(Name, source, options, keyData);
return new SortKey(this, source, options, keyData);
}

private static void InvariantCreateSortKeyOrdinal(ReadOnlySpan<char> source, Span<byte> sortKey)
Expand Down
85 changes: 24 additions & 61 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,64 +43,22 @@ private void InitSort(CultureInfo culture)
}
}

internal static unsafe int IndexOfOrdinalCore(string source, string value, int startIndex, int count, bool ignoreCase)
{
Debug.Assert(!GlobalizationMode.Invariant);

Debug.Assert(source != null);
Debug.Assert(value != null);

if (value.Length == 0)
{
return startIndex;
}

if (count < value.Length)
{
return -1;
}

if (ignoreCase)
{
fixed (char* pSource = source)
{
int index = Interop.Globalization.IndexOfOrdinalIgnoreCase(value, value.Length, pSource + startIndex, count, findLast: false);
return index != -1 ?
startIndex + index :
-1;
}
}

int endIndex = startIndex + (count - value.Length);
for (int i = startIndex; i <= endIndex; i++)
{
int valueIndex, sourceIndex;

for (valueIndex = 0, sourceIndex = i;
valueIndex < value.Length && source[sourceIndex] == value[valueIndex];
valueIndex++, sourceIndex++) ;

if (valueIndex == value.Length)
{
return i;
}
}

return -1;
}

internal static unsafe int IndexOfOrdinalCore(ReadOnlySpan<char> source, ReadOnlySpan<char> value, bool ignoreCase, bool fromBeginning)
{
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert(!value.IsEmpty);

Debug.Assert(source.Length != 0);
Debug.Assert(value.Length != 0);
// Ordinal (non-linguistic) comparisons require the length of the target string to be no greater
// than the length of the search space. Since our caller already checked for empty target strings,
// the below check also handles the case of empty search space strings.

if (source.Length < value.Length)
{
return -1;
}

Debug.Assert(!source.IsEmpty);

if (ignoreCase)
{
fixed (char* pSource = &MemoryMarshal.GetReference(source))
Expand Down Expand Up @@ -199,9 +157,14 @@ private static unsafe int CompareStringOrdinalIgnoreCase(ref char string1, int c
{
Debug.Assert(!GlobalizationMode.Invariant);

Debug.Assert(count1 > 0);
Debug.Assert(count2 > 0);

fixed (char* char1 = &string1)
fixed (char* char2 = &string2)
{
Debug.Assert(char1 != null);
Debug.Assert(char2 != null);
return Interop.Globalization.CompareStringOrdinalIgnoreCase(char1, count1, char2, count2);
}
}
Expand All @@ -215,6 +178,9 @@ private unsafe int CompareString(ReadOnlySpan<char> string1, string string2, Com
Debug.Assert(string2 != null);
Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);

// Unlike NLS, ICU (ucol_getSortKey) allows passing nullptr for either of the source arguments
// as long as the corresponding length parameter is 0.

fixed (char* pString1 = &MemoryMarshal.GetReference(string1))
fixed (char* pString2 = &string2.GetRawStringData())
{
Expand All @@ -227,6 +193,9 @@ private unsafe int CompareString(ReadOnlySpan<char> string1, ReadOnlySpan<char>
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);

// Unlike NLS, ICU (ucol_getSortKey) allows passing nullptr for either of the source arguments
// as long as the corresponding length parameter is 0.

fixed (char* pString1 = &MemoryMarshal.GetReference(string1))
fixed (char* pString2 = &MemoryMarshal.GetReference(string2))
{
Expand Down Expand Up @@ -540,7 +509,6 @@ private unsafe bool StartsWith(ReadOnlySpan<char> source, ReadOnlySpan<char> pre
{
Debug.Assert(!GlobalizationMode.Invariant);

Debug.Assert(!source.IsEmpty);
Debug.Assert(!prefix.IsEmpty);
Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);

Expand All @@ -553,7 +521,7 @@ private unsafe bool StartsWith(ReadOnlySpan<char> source, ReadOnlySpan<char> pre
}
else
{
fixed (char* pSource = &MemoryMarshal.GetReference(source))
fixed (char* pSource = &MemoryMarshal.GetReference(source)) // could be null (or otherwise unable to be dereferenced)
fixed (char* pPrefix = &MemoryMarshal.GetReference(prefix))
{
return Interop.Globalization.StartsWith(_sortHandle, pPrefix, prefix.Length, pSource, source.Length, options);
Expand All @@ -565,13 +533,12 @@ private unsafe bool StartsWithOrdinalIgnoreCaseHelper(ReadOnlySpan<char> source,
{
Debug.Assert(!GlobalizationMode.Invariant);

Debug.Assert(!source.IsEmpty);
Debug.Assert(!prefix.IsEmpty);
Debug.Assert(_isAsciiEqualityOrdinal);

int length = Math.Min(source.Length, prefix.Length);

fixed (char* ap = &MemoryMarshal.GetReference(source))
fixed (char* ap = &MemoryMarshal.GetReference(source)) // could be null (or otherwise unable to be dereferenced)
fixed (char* bp = &MemoryMarshal.GetReference(prefix))
{
char* a = ap;
Expand Down Expand Up @@ -636,13 +603,12 @@ private unsafe bool StartsWithOrdinalHelper(ReadOnlySpan<char> source, ReadOnlyS
{
Debug.Assert(!GlobalizationMode.Invariant);

Debug.Assert(!source.IsEmpty);
Debug.Assert(!prefix.IsEmpty);
Debug.Assert(_isAsciiEqualityOrdinal);

int length = Math.Min(source.Length, prefix.Length);

fixed (char* ap = &MemoryMarshal.GetReference(source))
fixed (char* ap = &MemoryMarshal.GetReference(source)) // could be null (or otherwise unable to be dereferenced)
fixed (char* bp = &MemoryMarshal.GetReference(prefix))
{
char* a = ap;
Expand Down Expand Up @@ -696,7 +662,6 @@ private unsafe bool EndsWith(ReadOnlySpan<char> source, ReadOnlySpan<char> suffi
{
Debug.Assert(!GlobalizationMode.Invariant);

Debug.Assert(!source.IsEmpty);
Debug.Assert(!suffix.IsEmpty);
Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);

Expand All @@ -709,7 +674,7 @@ private unsafe bool EndsWith(ReadOnlySpan<char> source, ReadOnlySpan<char> suffi
}
else
{
fixed (char* pSource = &MemoryMarshal.GetReference(source))
fixed (char* pSource = &MemoryMarshal.GetReference(source)) // could be null (or otherwise unable to be dereferenced)
fixed (char* pSuffix = &MemoryMarshal.GetReference(suffix))
{
return Interop.Globalization.EndsWith(_sortHandle, pSuffix, suffix.Length, pSource, source.Length, options);
Expand All @@ -721,13 +686,12 @@ private unsafe bool EndsWithOrdinalIgnoreCaseHelper(ReadOnlySpan<char> source, R
{
Debug.Assert(!GlobalizationMode.Invariant);

Debug.Assert(!source.IsEmpty);
Debug.Assert(!suffix.IsEmpty);
Debug.Assert(_isAsciiEqualityOrdinal);

int length = Math.Min(source.Length, suffix.Length);

fixed (char* ap = &MemoryMarshal.GetReference(source))
fixed (char* ap = &MemoryMarshal.GetReference(source)) // could be null (or otherwise unable to be dereferenced)
fixed (char* bp = &MemoryMarshal.GetReference(suffix))
{
char* a = ap + source.Length - 1;
Expand Down Expand Up @@ -773,13 +737,12 @@ private unsafe bool EndsWithOrdinalHelper(ReadOnlySpan<char> source, ReadOnlySpa
{
Debug.Assert(!GlobalizationMode.Invariant);

Debug.Assert(!source.IsEmpty);
Debug.Assert(!suffix.IsEmpty);
Debug.Assert(_isAsciiEqualityOrdinal);

int length = Math.Min(source.Length, suffix.Length);

fixed (char* ap = &MemoryMarshal.GetReference(source))
fixed (char* ap = &MemoryMarshal.GetReference(source)) // could be null (or otherwise unable to be dereferenced)
fixed (char* bp = &MemoryMarshal.GetReference(suffix))
{
char* a = ap + source.Length - 1;
Expand Down Expand Up @@ -836,7 +799,7 @@ private unsafe SortKey CreateSortKey(string source, CompareOptions options)
}
}

return new SortKey(Name, source, options, keyData);
return new SortKey(this, source, options, keyData);
}

private static unsafe bool IsSortable(char *text, int length)
Expand Down
Loading