System.Text.Rune ref APIs and unit tests by GrabYourPitchforks · Pull Request #33395 · dotnet/corefx

GrabYourPitchforks · 2018-11-11T00:02:25Z

Reference assembly APIs and unit tests for dotnet/coreclr#20935

This PR will not pass CI until the underlying coreclr change makes its way over.

nil4 · 2018-11-11T09:44:05Z

src/System.Runtime/ref/System.Runtime.cs

+        public static bool TryCreate(uint value, out Rune result) { throw null; }
+        public bool TryEncode(Span<char> destination, out int charsWritten) { throw null; }
+        public static bool TryGetRuneAt(string input, int index, out Rune value) { throw null; }
+        public static double GetNumericValue(Rune value) { throw null; }


Does returning double enable any specific use-cases that e.g. long does not?

Unicode defines fractional values, such as U+00BD (½) for which a non-integral value is returned.

http://unicodefractions.com/ for some others.

We pull the data from https://www.unicode.org/Public/UCD/latest/ucd/extracted/DerivedNumericValues.txt.
Our test file has a specific test for the U+00BD case:

yield return new object[] { new UnicodeInfoTestData { ScalarValue = (Rune)(0xBD), UnicodeCategory = UnicodeCategory.OtherNumber, NumericValue = 0.5, IsControl = false, IsDigit = false, IsLetter = false, IsLetterOrDigit = false, IsLower = false, IsNumber = true, IsPunctuation = false, IsSeparator = false, IsSymbol = false, IsUpper = false, IsWhiteSpace = false } };

It gets even more fun, since some scalars can correspond to negative numeric values! :D
For example, U+0F33 TIBETAN DIGIT HALF ZERO ('༳') has a numeric value of -0.5.

Interesting. Is this API as generally useful as to be exposed directly on the Rune type?

To be honest I don't personally have a valid use case for it. It's there for parity with System.Char and System.Text.CharUnicodeInfo.

ahsonkhan · 2018-11-12T02:55:51Z

src/System.Runtime/ref/System.Runtime.cs

        FormKC = 5,
        FormKD = 6,
    }
+    public readonly struct Rune : IComparable<Rune>, IEquatable<Rune>


See if you can use the ref generation tool to auto-gen the reference so it has correct sort ordering and fully qualified type names (like System.IEquatable<Rune>).

msbuild /t:GenerateReferenceSource

However, for System.Runtime, there are some warts with the auto-gen tool, so you may have to undo some of the changes that it makes (to other types).

There's no documentation for how to run that tool or what parameters to give it. If somebody creates docs I can run it. Otherwise I'll stick with doing this by hand because at least I know by hand works. :)

There's no documentation for how to run that tool or what parameters to give it.

Agreed. Here is how I tend to do it (for System.Runtime):

Build coreclr release

Build corefx release with coreclr bits

Run msbuild /t:GenerateReferenceSource /p:ConfigurationGroup=Release from the System.Runtime/ref directory

Filter out all unrelated changes and extract the changes you care about (ignore certain attributes being removed).

https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#testing-with-private-coreclr-bits

Updated the ref here: #33512

Added documentation here: #33515

ahsonkhan · 2018-11-12T03:08:16Z

src/System.Runtime/ref/System.Runtime.cs

+        public static explicit operator Rune(int value) { throw null; }
+        public bool IsAscii { get { throw null; } }
+        public bool IsBmp { get { throw null; } }
+        public int Plane { get { throw null; } }


Should this be more descriptive: UnicodePlane?

I think current version should be fine considering the whole class is related to the Unicode code point

ahsonkhan · 2018-11-12T03:11:35Z

src/System.Runtime/ref/System.Runtime.cs

+        public bool IsAscii { get { throw null; } }
+        public bool IsBmp { get { throw null; } }
+        public int Plane { get { throw null; } }
+        public static Rune ReplacementChar { get { throw null; } }


ReplacementRune?

U+FFFD REPLACEMENT CHARACTER is the official Unicode name for this value. We can expand "Char" -> "Character", but we shouldn't use the name Rune here.

tarekgh · 2018-11-12T17:45:51Z

try to build all configurations to ensure building the packages as I expect the api compat will fail for .Net Native as the APIs wouldn't should up there yet and you'll need to add some entries to the baseline file.

GrabYourPitchforks · 2018-11-13T00:58:19Z

The latest commit is just in case we change the name Rune back to UnicodeScalar, pending API review. I just wanted to have the commit ready to go whatever the decision.

krwq · 2018-11-13T17:41:17Z

My 2c: @GrabYourPitchforks I understood the Rune since it is short and you can get used to a fairly weird sounding (at least for me) name, but since UnicodeScalar is long already I'd rather go with UnicodeCodePoint - that would at least match the spec :-)

tarekgh · 2018-11-13T17:48:57Z

@krwq Unicode Scalar is right term here. it is different than code point.

The code point is:
In the Unicode Standard, the codespace consists of the integers from 0 to 10FFFF16, comprising 1,114,112 code points available for assigning the repertoire of abstract characters. http://www.unicode.org/versions/Unicode11.0.0/ch02.pdf#G25564

while Unicode scalar is:
Unicode Scalar Value. Any Unicode code point except high-surrogate and low-surrogate code points. In other words, the ranges of integers 0 to D7FF16 and E00016 to 10FFFF16 inclusive. (See definition D76 in Section 3.9, Unicode Encoding Forms.) http://unicode.org/glossary/#unicode_scalar_value

This type is about Unicode Scalar

krwq · 2018-11-13T17:57:10Z

Thanks @tarekgh I can see in fact tests confirming the spec definition. I concur with the name in this case :-)

GrabYourPitchforks · 2018-11-14T01:29:32Z

The coreclr implementation is now checked in. I'll rebase this PR when the train makes its way to corefx, and hopefully everything will just pass. :) (fingers crossed)

stephentoub · 2018-11-14T18:56:20Z

@dotnet-bot test this please

GrabYourPitchforks · 2018-11-14T19:25:04Z

The train from coreclr into corefx is delayed to a build infrastructure issue. The issue appears to be resolved now but there's a small backlog. I'm hoping the train will arrive in a few hours, and then I can rebase on top of it and get this to pass.

… data on other platforms

Commit migrated from dotnet/corefx@7626a26

GrabYourPitchforks requested review from ahsonkhan, bartonjs and tarekgh November 11, 2018 00:02

GrabYourPitchforks mentioned this pull request Nov 11, 2018

Initial commit for System.Text.Rune dotnet/coreclr#20935

Merged

nil4 reviewed Nov 11, 2018

View reviewed changes

ahsonkhan reviewed Nov 12, 2018

View reviewed changes

GrabYourPitchforks force-pushed the rune branch from 904718e to 415cd7c Compare November 12, 2018 23:56

ahsonkhan mentioned this pull request Nov 15, 2018

Add documentation which explains how to update reference assembly sources #33515

Merged

System.Text.Rune ref APIs and unit tests

c7646c4

GrabYourPitchforks force-pushed the rune branch from be02a24 to c7646c4 Compare November 15, 2018 17:59

GrabYourPitchforks added 2 commits November 15, 2018 10:19

Add System.Text.Rune to ApiCompatBaseline files

ada0869

Only run Rune localization tests on Win8+ due to missing localization…

cbfbffb

… data on other platforms

GrabYourPitchforks merged commit 7626a26 into dotnet:master Nov 15, 2018

GrabYourPitchforks deleted the rune branch November 15, 2018 21:38

jlennox pushed a commit to jlennox/corefx that referenced this pull request Dec 16, 2018

System.Text.Rune ref APIs and unit tests (dotnet#33395)

8b617bc

karelz added this to the 3.0 milestone Dec 21, 2018

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

System.Text.Rune ref APIs and unit tests (dotnet/corefx#33395)

acc8460

Commit migrated from dotnet/corefx@7626a26

Conversation

GrabYourPitchforks commented Nov 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarekgh commented Nov 12, 2018

Uh oh!

GrabYourPitchforks commented Nov 13, 2018

Uh oh!

krwq commented Nov 13, 2018

Uh oh!

tarekgh commented Nov 13, 2018

Uh oh!

krwq commented Nov 13, 2018

Uh oh!

GrabYourPitchforks commented Nov 14, 2018

Uh oh!

stephentoub commented Nov 14, 2018

Uh oh!

GrabYourPitchforks commented Nov 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants