Skip to content

add: windows wstring support for padded_str#2537

Merged
lemire merged 4 commits intosimdjson:masterfrom
hiteshmk05:feature/windows-wstring-support
Nov 5, 2025
Merged

add: windows wstring support for padded_str#2537
lemire merged 4 commits intosimdjson:masterfrom
hiteshmk05:feature/windows-wstring-support

Conversation

@hiteshmk05
Copy link
Contributor

@hiteshmk05 hiteshmk05 commented Nov 3, 2025

Description

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation / tests
  • Other (please describe):

@lemire
Copy link
Member

lemire commented Nov 3, 2025

Internally, we pass the filename string to std::fopen which, under Windows, may not be interpreted as UTF-8. It depends.

@lemire
Copy link
Member

lemire commented Nov 3, 2025

Instead of converting to UTF8, which might not work well, I recommend you replace the call to fopen by _wfopen internally, thus avoiding the conversion.

Have a look at

inline simdjson_result<padded_string> padded_string::load(std::string_view filename) noexcept {

You could just copy paste this old code, and replace fopen by _wfopen.

@hiteshmk05
Copy link
Contributor Author

Instead of converting to UTF8, which might not work well, I recommend you replace the call to fopen by _wfopen internally, thus avoiding the conversion.

Have a look at

inline simdjson_result<padded_string> padded_string::load(std::string_view filename) noexcept {

You could just copy paste this old code, and replace fopen by _wfopen.

i was thinking of this approach but was not sure if it was the right way.

@lemire
Copy link
Member

lemire commented Nov 4, 2025

Looks good to me.

@Nelson-numerical-software could you review since you created the issue?

@Nelson-numerical-software

Looks good to me.

@Nelson-numerical-software could you review since you created the issue?

Thanks for this works ! It is good for me too

@lemire lemire merged commit 77d73b0 into simdjson:master Nov 5, 2025
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants