Conversation
pauldreik
left a comment
pauldreik
left a comment
There was a problem hiding this comment.
For this to be complete, I think we should have constexpr span overloads and also a test that at least calls the new functions.
| simdutf_base64_options options, | ||
| simdutf_last_chunk_handling_options last_chunk_options); | ||
|
|
||
| /* single-character base64 validation */ |
There was a problem hiding this comment.
what is the usecase for these two functions?
There was a problem hiding this comment.
Ok so the main use case is when using stop_before_partial. In these cases, you deliberately stop before the partial block (duh). But without the details part, you don't know where you stopped!!!
The stop_before_partial part is useful when you are decoding a stream of base64. Let us say that I give you a large string and you cut it off in segments on 4096 and one segment ends with 3 base64 characters. Now, you can't yet decode them because maybe there is a fourth character coming!
It is also useful in case of errors although that's not why I care about it.
|
|
||
| You can also check whether a single character is a valid base64 character using `base64_valid`: | ||
| ```cpp | ||
| bool is_valid = simdutf::base64_valid('A'); // true |
There was a problem hiding this comment.
am I missing something, or are these only available as c functions?
There was a problem hiding this comment.
They are C++ functions !!!
|
@pauldreik Excellent comments. I think I have addressed them. |
There was a problem hiding this comment.
Pull request overview
This PR expands the public Base64 API by exposing base64_valid and base64_to_binary_details, and wires these APIs through the C/C++ interfaces with accompanying documentation and tests.
Changes:
- Add public
base64_to_binary_detailsoverloads (C++ free functions + span/constexpr usage) and document behavior. - Add C API equivalents for “details” decoding and single-character base64 validation.
- Update CLI tool and add runtime + compile-time tests covering the newly public APIs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/fastbase64.cpp | Switches CLI decoding to the new public base64_to_binary_details free function. |
| src/implementation.cpp | Adds public C++ free-function wrappers for base64_to_binary_details (char/char16_t). |
| include/simdutf/implementation.h | Documents and declares the new public base64_to_binary_details APIs (incl. span overloads). |
| src/simdutf_c.cpp | Implements new C API wrappers returning a simdutf_full_result. |
| include/simdutf_c.h | Adds simdutf_full_result and declares new C API entry points. |
| tests/base64_tests.cpp | Adds runtime tests for base64_to_binary_details and base64_valid. |
| tests/constexpr_base64_tests.cpp | Adds constexpr tests demonstrating compile-time base64_to_binary_details. |
| README.md | Documents base64_to_binary_details / base64_valid usage and semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| simdutf::full_result r = simdutf::base64_to_binary_details( | ||
| input_data.data(), total_input, output_buffer.data(), options, | ||
| simdutf::last_chunk_handling_options::loose); |
There was a problem hiding this comment.
This change switches the CLI from get_active_implementation()->base64_to_binary_details(...) to the public free function simdutf::base64_to_binary_details(...), which (per the implementation pattern in this repo) routes through the default implementation. If the CLI supports selecting/changing the active implementation at runtime, this will ignore that selection and change behavior/perf. Prefer calling simdutf::get_active_implementation()->base64_to_binary_details(...) here to preserve existing CLI semantics while still benefiting from the newly-public API elsewhere.
| simdutf::full_result r = simdutf::base64_to_binary_details( | |
| input_data.data(), total_input, output_buffer.data(), options, | |
| simdutf::last_chunk_handling_options::loose); | |
| simdutf::full_result r = | |
| simdutf::get_active_implementation()->base64_to_binary_details( | |
| input_data.data(), total_input, output_buffer.data(), options, | |
| simdutf::last_chunk_handling_options::loose); |
| simdutf::full_result r = simdutf::base64_to_binary_details( | ||
| input_data.data(), total_input, output_buffer.data(), options, | ||
| simdutf::last_chunk_handling_options::stop_before_partial); |
There was a problem hiding this comment.
This change switches the CLI from get_active_implementation()->base64_to_binary_details(...) to the public free function simdutf::base64_to_binary_details(...), which (per the implementation pattern in this repo) routes through the default implementation. If the CLI supports selecting/changing the active implementation at runtime, this will ignore that selection and change behavior/perf. Prefer calling simdutf::get_active_implementation()->base64_to_binary_details(...) here to preserve existing CLI semantics while still benefiting from the newly-public API elsewhere.
Co-authored-by: Copilot <[email protected]>
|
I think this looks good to merge!
|
This exposes two new functions:
base64_validandbase64_to_binary_detailsas public. They were previously not presented as public.