-
Notifications
You must be signed in to change notification settings - Fork 116
add simdutf_constexpr macro #859
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
Conversation
f2c301a to
f63b351
Compare
f63b351 to
d75c48e
Compare
|
This is reasonable. Let us wait for the tests to go green. |
| simdutf_really_inline void utf16fix_block(char16_t *out, const char16_t *in) { | ||
| const char16_t replacement = scalar::utf16::replacement<big_endian>(); | ||
| auto swap_if_needed = [](uint16_t c) -> uint16_t { | ||
| simdutf_constexpr auto swap_if_needed = [](uint16_t c) -> uint16_t { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this feels a bit misplaced?
I think the constexpr should go inside the lambda, with the ternary replaced with an if.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anonrig Can you answer this comment ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is kind of unnecessary and can be moved into the correct place.
|
A related question: what would it take for us to require C++17 instead of C++11 as build requirement? Do we have any idea of how large proportion of the simdutf users are stuck at C++11? |
Obviously, I would just skip to C++20 if I could. Let us see what people use: |
|
I am blaming the failed CI tests on Cloudflare. @anonrig's PR should be generally safe. |
Fixes #856
Up until the last commit in this PR, we didn't enabled C++17 on macos workflow even though our README said otherwise...