Clean up String::create handling with HX_SMART_STRINGS disabled#1253
Open
tobil4sk wants to merge 4 commits intoHaxeFoundation:masterfrom
Open
Clean up String::create handling with HX_SMART_STRINGS disabled#1253tobil4sk wants to merge 4 commits intoHaxeFoundation:masterfrom
tobil4sk wants to merge 4 commits intoHaxeFoundation:masterfrom
Conversation
12 tasks
TConvertToUTF8 treats strings as null terminating if the length is 0, so rather than calculating the size manually we can set it to 0 if it the input to TCopyString is -1. For GCStringDup, -1 is handled as null terminating, but 0 is treated as a 0 length string. We can pass the length argument straight in without modification there. This avoids duplicate loops in both the TConvertToUTF8 and GCStringDup cases.
When smart strings are enabled, this always returns an empty string, however, if they are disabled this currently has different behaviour depending on the type of character passed in. This fixes the behaviour so that it also returns an empty string for wchar_t and char16_t strings when smart strings are disabled.
tobil4sk
force-pushed
the
fix/TCopyString-duplicate-iteration
branch
from
82dae60 to
b506ac9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
d119a07 fixed an issue with incorrect handling of
length < 0, however, it also introduced an unnecessary additional loop to get the length of the string, separate from the conversion/copy loop. This can be cleaned up becauseGCStringDupalready has the correct handling oflength < 0, and forTConvertToUTF8putting length as 0 has the same effect. This way the string only has to be iterated through once for the conversion/copy.Also, there was still broken behaviour when using
String::create(_, 0). WithHX_SMART_STRINGS, this always returns an empty string, whereas withoutHX_SMART_STRINGSthe behaviour depends on the char type. This PR fixes this so it always returns an empty string.This also adds a test for #849, to make sure it doesn't cause a regression. (Presumably, #814 is also covered by this test)