Expose missing String encoding conversion functions#104781
Expose missing String encoding conversion functions#104781Repiteo merged 1 commit intogodotengine:masterfrom
String encoding conversion functions#104781Conversation
Ivorforce
force-pushed
the
string-encode-complete
branch
from
2b8807f to
e9369fb
Compare
| memcpy(dst, p_char, p_length * sizeof(char32_t)); | ||
| *(dst + p_length) = _null; | ||
| void String::append_utf32_unchecked(const Span<char32_t> &p_span) { | ||
| const int prev_length = length(); |
There was a problem hiding this comment.
funny that you cache length() that is called twice, but not p_span.size() that is called 3 times.
There was a problem hiding this comment.
p_span.size() just compiles to an inlined field access (p_span._ptr). There's no need to cache it :)
Edit: Probably also no need to cache length() either, but prev_length has to be cached because length() changes after the resize.
Ivorforce
force-pushed
the
string-encode-complete
branch
from
e9369fb to
5ea8a6a
Compare
|
I found that |
Ivorforce
force-pushed
the
string-encode-complete
branch
from
5ea8a6a to
be55c43
Compare
| // NULL-terminated c string copy - automatically parse the string to find the length. | ||
| void append_latin1(const char *p_cstr) { | ||
| append_latin1(Span(p_cstr, p_cstr ? strlen(p_cstr) : 0)); | ||
| } | ||
| void append_utf32(const char32_t *p_cstr) { | ||
| append_utf32(Span(p_cstr, p_cstr ? strlen(p_cstr) : 0)); | ||
| } |
There was a problem hiding this comment.
These should get Error return types as well, no? I'd also group these by their Span equivalents below (changing their return type as well) to match the other functions
There was a problem hiding this comment.
These are currently in the private section of String. I plan to remove these anyway, when it's possible.
I could still add an Error return type, though I don't think they're called from anywhere except the constructor at this point.
| append_utf32((Span<char32_t> &)p_cstr); | ||
| #endif | ||
| } | ||
| void append_wstring(const wchar_t *p_cstr) { |
|
@Repiteo Is it fine to move ahead with this even with the functions you flagged not exposed? |
Repiteo
left a comment
Repiteo
left a comment
There was a problem hiding this comment.
Not exposing the private functions isn't a dealbreaker. Looks ready to roll after a rebase
Ivorforce
force-pushed
the
string-encode-complete
branch
from
be55c43 to
8023a8a
Compare
|
Needs rebase (again) |
…:utf32_unchecked` in `String` for high performance string copies. Expose `append_wstring` and `String::wstring` for platform strings.
Ivorforce
force-pushed
the
string-encode-complete
branch
from
8023a8a to
d1fd42b
Compare
|
I'm a rebase machine 😎 |
|
Thanks! |
3 participants
This can be seen as sort of a follow-up of recent PRs going in the same direction: I'm completing the function ensemble for string conversions with named encodings. In particular:
append_utf32gets anErrorreturn type, allowing the caller to react to poorly encoded strings.append_utf32_uncheckedis added (formerlycopy_from_unchecked), for high performance conversion from trusted strings.append_wstringis added, for conversion from platform defined strings.I want to soon begin eliminating implicit pointer-to-string conversions, so these functions will be needed :)
With
append_utf32_uncheckedavailable,String::operator+=can use that instead of the slowappend_utf32function. It should be several times faster than before.