Remove SString ANSI representation. #71186
Conversation
99% of usages were just ASCII, so switch to using UTF8 as the underlying representation as it will have the same semantics for ASCII strings. The main area of concern would be in the implementation of `SString::VPrintf(CHAR*, ...)`. I believe this will work correctly, but I'd like a second pair of eyes to verify.
src/coreclr/utilcode/sstring.cpp
Outdated
| @@ -1939,6 +1795,7 @@ void SString::VPrintf(const CHAR *format, va_list args) | |||
| CONTRACT_VOID | |||
| { | |||
| INSTANCE_CHECK; | |||
| PRECONDITION(GetRepresentation() == REPRESENTATION_ASCII || GetRepresentation() == REPRESENTATION_UTF8); | |||
There was a problem hiding this comment.
Why do we care about the representation of the existing content? The existing content will be overwritten so it should not matter.
There was a problem hiding this comment.
For some reason I got confused between the AppendPrintf method and this one and didn't realize that this one overrides all of the contents.
| @@ -127,7 +124,7 @@ void CHECK::Trigger(LPCSTR reason) | |||
| pMessage->AppendASCII(m_condition); | |||
| #endif | |||
|
|
|||
| messageString = pMessage->GetANSI(*pScratch); | |||
| messageString = pMessage->GetUTF8(); | |||
There was a problem hiding this comment.
If the message string is Utf8 now, we should use OutputDebugStringUtf8 below to print it.
There was a problem hiding this comment.
Also, fix DbgAssertDialog to expect Utf8. It ends up hitting OutputDebugStringA(szExpr) that has similar problem.
I think so as well. Could you please do a quick ad-hoc test with non-ASCII Utf8 string to verify it? |
|
I did a quick test and it seems like it works (the outputs match the input), but I was having issues getting MSVC to actually correctly handle non-ASCII unicode characters so the input didn't look right (but the underlying bytes were correct). |
99% of usages were just ASCII, so switch to using UTF8 as the underlying representation as it will have the same semantics for ASCII strings.
The main area of concern would be in the implementation of
SString::VPrintf(CHAR*, ...). I believe this will work correctly, but I'd like a second pair of eyes to verify.Builds off of some of the ideas in #69748