Skip to content
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.

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

deps: update simdutf to 2.2.1 #46208

Closed
wants to merge 1 commit into from

Conversation

nodejs-github-bot
Copy link
Collaborator

This is an automated update of simdutf to 2.2.1.

@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Jan 14, 2023
@gengjiawen gengjiawen 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-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 14, 2023
@nodejs-github-bot
Copy link
Collaborator Author

@nodejs-github-bot
Copy link
Collaborator Author

@gengjiawen gengjiawen 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

const auto 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

anonrig commented Jan 14, 2023

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...

  const char 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

lemire commented Jan 14, 2023

The simdutf PR is also a great idea. Go!

@anonrig
Copy link
Member

anonrig commented Jan 14, 2023

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

lemire commented Jan 14, 2023

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

anonrig commented Jan 14, 2023

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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants