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

Jump to bottom

Change SString::GetUTF8NoConvert to GetUTF8 that converts the SString #71101

Merged

jkotas merged 8 commits into dotnet:main from jkoritzinsky:sstring-getutf8

Jun 23, 2022

Member

jkoritzinsky commented Jun 21, 2022

This enables SString to get out of the UTF16 state and helps move us away from "SString's natural encoding is UTF16".

This is the first of multiple PRs based on work in #69748.


          Change SString::GetUTF8NoConvert to GetUTF8 that converts the SString

00913ac

This enables SString to get out of the UTF16 state and helps move us away from "SString's natural encoding is UTF16"

jkoritzinsky requested a review from jkotas

June 21, 2022 22:19

dotnet-issue-labeler bot added the area-VM-coreclr label

ghost assigned jkoritzinsky

jkotas approved these changes

View reviewed changes

Member

jkotas left a comment

Thank you!

jkotas reviewed

View reviewed changes

src/coreclr/vm/assemblynative.cpp Outdated Show resolved Hide resolved

jkoritzinsky commented

View reviewed changes

src/coreclr/vm/coreassemblyspec.cpp Outdated Show resolved Hide resolved

jkotas reviewed

View reviewed changes

src/coreclr/vm/coreassemblyspec.cpp Outdated Show resolved Hide resolved

jkotas reviewed

View reviewed changes

src/coreclr/vm/gdbjit.cpp Outdated Show resolved Hide resolved

jkoritzinsky commented

View reviewed changes

Member Author

jkoritzinsky left a comment

@jkotas same process applies for SetLiteral, correct?

src/coreclr/vm/coreassemblyspec.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/assemblynative.cpp Outdated Show resolved Hide resolved

jkotas reviewed

View reviewed changes

src/coreclr/vm/stubgen.cpp Outdated Show resolved Hide resolved

jkotas reviewed

View reviewed changes

src/coreclr/vm/jitinterface.cpp Show resolved Hide resolved

Member

jkotas commented Jun 21, 2022

@jkotas same process applies for SetLiteral, correct?

Yep


          Remove some stack scratch buffers that are unneeded now (as we can no…

3d8d510

…w convert the SString itself and in some cases it was already in UTF8) and implement PR feedback.

AaronRobinsonMSFT approved these changes

View reviewed changes

src/coreclr/utilcode/sstring.cpp Outdated Show resolved Hide resolved


          Update src/coreclr/utilcode/sstring.cpp

9e0754f

Co-authored-by: Aaron Robinson <arobins@microsoft.com>

jkotas reviewed

View reviewed changes

src/coreclr/vm/comdelegate.cpp Outdated Show resolved Hide resolved

jkotas reviewed

View reviewed changes

src/coreclr/vm/array.cpp Show resolved Hide resolved

jkotas reviewed

View reviewed changes

src/coreclr/utilcode/clrconfig.cpp Outdated Show resolved Hide resolved

jkoritzinsky added 4 commits

June 22, 2022 10:47


          Add SetAndConvertToUTF8 method and remove the GetUTF8 method variants…

36731bc

… with a scratch buffer.


          Merge branches 'sstring-getutf8' and 'sstring-getutf8' of github.com:…

d4c7bf4

…jkoritzinsky/runtime into sstring-getutf8


          Remove unneeded variables.

ada35f9


          Merge branch 'main' of github.com:dotnet/runtime into sstring-getutf8

0162fd9

jkotas reviewed

View reviewed changes

src/coreclr/vm/assemblynative.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/assemblynative.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/assemblynative.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/coreassemblyspec.cpp Outdated Show resolved Hide resolved

src/coreclr/vm/coreassemblyspec.cpp Outdated Show resolved Hide resolved


          Apply suggestions from code review

cfd06d1

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

Member Author

jkoritzinsky commented Jun 22, 2022

@jkotas any more feedback?

jkotas approved these changes

View reviewed changes

Member

jkotas left a comment

LGTM. Thank you!

jkotas merged commit d17741d into dotnet:main

jkoritzinsky deleted the sstring-getutf8 branch

June 23, 2022 01:06

ghost locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.