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

Make JNI string conversions behave like ObjC string conversions #174

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jb-gcx
Copy link
Contributor

@jb-gcx jb-gcx commented Jun 29, 2024

When a C++ std::string contains invalid UTF-8, the result is an empty string when trying to pass it to ObjC. For JNI, it caused an exception. For consistency, I would like encoding errors to result in empty strings in JNI as well.

@li-feng-sc
Copy link
Contributor

Hi @jb-gcx,

Thanks for the contribution! This is an area I had hesitated for a long time.

On one hand, I kind of want to make programming errors stand out more so that they could be corrected earlier. Passing invalid characters as string is one of them, binary objects should be passed using the binary type. Changing the string to something else (such as empty string) may allow this kind of errors to slip past testing.

On the other hand, I understand it could be annoying when simply passing a string argument may require the caller to handle potential encoding errors.

I can't decided which is better, convenience or strictness.

Also, if you just want to return a fallback string when there is an encoding error, std::wstring_convert's constructor lets you pass byte_error and wide_error that will be used in case of error instead of throwing.

@jb-gcx
Copy link
Contributor Author

jb-gcx commented Jul 10, 2024

Thanks for the response @LiFengSC !

Regarding your worries about errors slipping past, I absolutely agree. I much prefer them to be visible in general.

The reason I feel confident about this change is that iOS already behaves this way. There are arguments for either style of dealing with errors, but I think the most important thing is to use the same style between platforms.

To be consistent, the options are:

  1. Change iOS to generate hard errors when encoding fails -> this is a change that will break existing user code at runtime, probably crashing their apps
  2. Change Android to fallback to empty strings like iOS does -> this will only break code that relied on exceptions being produced, which I would expect to be a very small minority, and it won't cause crashes

Lastly, about the byte_error and wide_error: this was my first approach, but it did not work. I think wstring_convert checks whether they're set by just checking if they're non-empty. Since we want to return empty strings, wstring_convert decides to throw instead. Cppreference is pretty unclear about this behavior, but here's a godbold to demonstrate.

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.

2 participants