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

Implement String.IsAscii in shared CoreLib and use it for [Is]Normalize[d] for all runtimes#24373

Merged
jkotas merged 1 commit intodotnet:masterfrom
filipnavara:string_isascii
May 3, 2019
Merged

Implement String.IsAscii in shared CoreLib and use it for [Is]Normalize[d] for all runtimes#24373
jkotas merged 1 commit intodotnet:masterfrom
filipnavara:string_isascii

Conversation

@filipnavara
Copy link
Member

Based on discussion at dotnet/corefx#37358 (comment).

@marek-safar marek-safar requested a review from jkotas May 3, 2019 11:37
else {
FC_GC_POLL_NOT_NEEDED();
}
FC_RETURN_BOOL(IS_ASCII(state)); //This can indicate either high chars or special sorting chars.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess IS_ASCII can be removed as well

coreclr/src/vm/object.h

Lines 1044 to 1046 in b271aff

#define IS_ASCII(state) (((state)==STRING_STATE_SPECIAL_SORT) || ((state)==STRING_STATE_FAST_OPS))
#define IS_FAST_CASING(state) IS_ASCII(state)
#define IS_FAST_INDEX(state) IS_ASCII(state)

@@ -64,9 +64,6 @@ public extern int Length
// and not a - or '
[MethodImplAttribute(MethodImplOptions.InternalCall)]
internal extern bool IsFastSort();
Copy link
Member

@EgorBo EgorBo May 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsFastSort internal call should be removed as well if I understand it correctly.
Also I guess we can now remove that code that keeps String state, e.g. remove InternalCheckHighChars etc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsFastSort is still used in few places. Cleaning those up can be separate PR.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jkotas jkotas merged commit 0e15f47 into dotnet:master May 3, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants