Skip to content

Conversation

@nodejs-github-bot
Copy link
Collaborator

This is an automated update of simdutf to 2.2.1.

@nodejs-github-botnodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Jan 14, 2023
@gengjiawengengjiawen added request-ci Add this label to start a Jenkins CI on a PR. fast-track PRs that do not need to wait for 48 hours to land. labels Jan 14, 2023
@github-actions

This comment was marked as outdated.

@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 14, 2023
@nodejs-github-bot
Copy link
CollaboratorAuthor

@nodejs-github-bot
Copy link
CollaboratorAuthor

@gengjiawengengjiawen removed the fast-track PRs that do not need to wait for 48 hours to land. label Jan 14, 2023
@addaleax
Copy link
Member

Looks like instead of fixing the function naming/documentation for big endian (which I think would have been the better choice by far) simdutf decided to do a full breaking change and treat char16_t*s as char* instead…

I guess this means that we'll need to add something like

constauto simdutf_convert_utf8_to_utf16 = IsLittleEndian() ? simdutf::convert_utf8_to_utf16le : simdutf::convert_utf8_to_utf16be;

for all simdutf functions we intend to use and then use that instead.

@anonrig Would you be up for adding functions to simdutf that just use host endianness, as one would expect for functions that take a char16_t argument? e.g. simdutf::convert_utf8_to_utf16?

@anonrig
Copy link
Member

Thanks for the mention @addaleax. I'll add this to simdutf.

@lemire
Copy link
Member

lemire commented Jan 14, 2023

On a little-endian system, then the following test should pass...

constchar utf8source[] = "\xe7\x8c\xab"; char16_t u16output; size_t u16len = simdutf::convert_utf8_to_utf16le(utf8source, 3, &u16output); size_t u16len = simdutf::convert_utf8_to_utf16le(utf8source, 3, &u16output); EXPECT_EQ(u16len, 1u); EXPECT_EQ(u16output, 0x732B);

On a big-endian system, you will need to replace the last line with...

EXPECT_EQ(u16output, 0x2B73);

(I understand that it is not what you want.)

@lemire
Copy link
Member

The simdutf PR is also a great idea. Go!

@anonrig
Copy link
Member

When my pull request is merged, we can release a new version on simdutf, and update node.js (and remove the endianness test)

@lemire
Copy link
Member

Please consider release 3.0.0 which contains @anonrig's pull request.

https://github.com/simdutf/simdutf/releases/tag/v3.0.0

@anonrig
Copy link
Member

I'll close this pull request, and wait for the github workflow to pick up the 3.0.0 release. Thanks everyone!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependenciesPull requests that update a dependency file.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@nodejs-github-bot@addaleax@anonrig@lemire@gengjiawen