make detect encoding functions constexpr by shikharish · Pull Request #920 · simdutf/simdutf

shikharish · 2026-01-13T10:22:19Z

Closes #891

Signed-off-by: Shikhar <shikharish05@gmail.com>

shikharish · 2026-01-13T11:13:44Z

cc: @lemire @pauldreik

lemire · 2026-01-13T11:54:02Z

@shikharish thanks. This will nicely close our gap. Let us include this in our next release.

pauldreik

Thanks for the PR! I am in a hurry so sorry for not doing a thorough review.
I think it is mostly fine but I have some questions.

include/simdutf/common_defs.h

include/simdutf/compiler_check.h

pauldreik · 2026-01-14T12:36:59Z

include/simdutf/encoding_types.h

-simdutf_warn_unused encoding_type check_bom(const uint8_t *byte, size_t length);
-simdutf_warn_unused encoding_type check_bom(const char *byte, size_t length);
+template <typename BytePtr>
+simdutf_warn_unused inline simdutf_constexpr14 encoding_type


let's use a macro guarded c++20 concept here to restrict the bytepr.

pauldreik · 2026-01-14T12:37:32Z

include/simdutf/encoding_types.h

+  return encoding_type::unspecified;
+}
+
+simdutf_warn_unused inline encoding_type check_bom(const char *byte,


should be really_inline

pauldreik · 2026-01-14T12:39:04Z

include/simdutf/implementation.h

 #include <simdutf/scalar/utf32_to_utf8/utf32_to_utf8.h>
 #include <simdutf/scalar/utf32_to_utf8/valid_utf32_to_utf8.h>
 #include <simdutf/scalar/utf8.h>
+#include <simdutf/scalar/detect.h>


why is this included twice?

pauldreik · 2026-01-14T12:39:42Z

include/simdutf/implementation.h

+#include <simdutf/scalar/utf8.h>
+#include <simdutf/scalar/utf16.h>
+#include <simdutf/scalar/utf32.h>
+#include <simdutf/scalar/detect.h>


it is ok to move around the includes, but remove those that are duplicates

pauldreik · 2026-01-14T12:40:06Z

include/simdutf/implementation.h

+#include <simdutf/scalar/utf32.h>
+#include <simdutf/scalar/detect.h>
+
+#if SIMDUTF_SPAN


why is this needed?

pauldreik · 2026-01-14T12:40:44Z

include/simdutf/implementation.h

 * of units processed if successful.
 */
-simdutf_warn_unused result
+simdutf_warn_unused simdutf_constexpr23 result


why was this changed?

tests/autodetect_constexpr_tests.cpp

pauldreik · 2026-01-14T20:50:52Z

include/simdutf/scalar/utf16.h

 }

-template <endianness big_endian>
+template <endianness big_endian, typename InputPtr>


please add a macro guarded c++20 concept check.

(and reused the already existing concept and style)

Signed-off-by: Shikhar <shikharish05@gmail.com>

lemire · 2026-01-16T17:24:03Z

@shikharish Could you have a look at @pauldreik's comments ? There are a few that are apparently outstanding (meaning that it is unclear whether you answered or changed the code accordingly).

shikharish · 2026-01-16T17:26:04Z

@lemire I made all the requested changes. There were a few things I misunderstood before but I've fixed them in the last few commits.

lemire · 2026-01-16T17:28:04Z

@pauldreik Can you check that your concerns were addressed?

pauldreik · 2026-01-16T21:22:48Z

include/simdutf/encoding_types.h

+#if SIMDUTF_CPLUSPLUS20
+  #include <concepts>
+template <typename BytePtr>
+concept check_bom_byteptr = requires(BytePtr p, size_t i) {


please look at the other constexpr changes recently merged and reuse the already existing concepts.

pauldreik · 2026-01-16T21:25:15Z

include/simdutf/scalar/utf16.h

 }

-template <endianness big_endian>
+template <endianness big_endian, typename InputPtr>


(and reused the already existing concept and style)

include/simdutf/scalar/detect.h

tests/autodetect_constexpr_tests.cpp

Signed-off-by: Shikhar <shikharish05@gmail.com>

shikharish · 2026-02-01T05:53:03Z

@pauldreik Please review.

pauldreik · 2026-02-01T11:35:59Z

@shikharish when you get review comments, please go through them one by one and fix/answer them. otherwise I have to go through everything again. there are still issues left from earlier comments.

what is also left to do:

update the README which says these two functions miss constexpr functionality.
a function template is already implicitly inline, don't mark as inline.
the changes in base64, why are they there? surprised to see them in this PR.

pauldreik · 2026-02-01T11:37:08Z

tests/autodetect_constexpr_tests.cpp

+#include <iostream>
+#include <array>


these don't seem to be used.

pauldreik · 2026-02-01T11:41:08Z

include/simdutf/scalar/utf16.h

+#if SIMDUTF_SPAN
+template <endianness big_endian, simdutf::detail::indexes_into_utf16 InputPtr>
+#else
+template <endianness big_endian, typename InputPtr>
+#endif


template <endianness big_endian, typename InputPtr> #if SIMDUTF_CPLUSPLUS20 requires simdutf::detail::indexes_into_utf16<InputPtr> #endif simdutf_warn_unused simdutf_constexpr23 bool

pauldreik · 2026-02-01T11:41:56Z

include/simdutf/scalar/utf16.h

 }

+template <endianness big_endian>
+inline simdutf_warn_unused simdutf_constexpr23 bool


this should be a simdutf really inline

make detect encoding functions constexpr

83388fb

Signed-off-by: Shikhar <shikharish05@gmail.com>

shikharish force-pushed the encoding-constexpr branch from 187b3c6 to 83388fb Compare January 13, 2026 10:47

minor fixes

0a0d74c

Signed-off-by: Shikhar <shikharish05@gmail.com>

pauldreik reviewed Jan 14, 2026

View reviewed changes

minor changes

d18b2ae

Signed-off-by: Shikhar <shikharish05@gmail.com>

shikharish requested review from lemire and pauldreik January 15, 2026 12:02

revert c++23 condition check

b831db8

Signed-off-by: Shikhar <shikharish05@gmail.com>

shikharish force-pushed the encoding-constexpr branch from 2b17857 to b831db8 Compare January 15, 2026 16:37

pauldreik requested changes Jan 16, 2026

View reviewed changes

improvements

46290ea

Signed-off-by: Shikhar <shikharish05@gmail.com>

shikharish requested a review from pauldreik January 21, 2026 11:39

pauldreik requested changes Feb 1, 2026

View reviewed changes

Conversation

shikharish commented Jan 13, 2026

Uh oh!

shikharish commented Jan 13, 2026

Uh oh!

lemire commented Jan 13, 2026

Uh oh!

pauldreik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lemire commented Jan 16, 2026

Uh oh!

shikharish commented Jan 16, 2026

Uh oh!

lemire commented Jan 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shikharish commented Feb 1, 2026

Uh oh!

pauldreik commented Feb 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants