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

Improve documentation and usage of -[NSString UTF8String] #628

Merged
merged 4 commits into from
Jun 13, 2024

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Jun 12, 2024

-[NSString UTF8String] (or the equivalent NSString::as_str) is used several times in our implementation, but when transitioning to #563, we must ensure that this usage is still sound even if the string is interior mutable.

Luckily, Foundation already ensures this for us by checking if the string is mutable in the implementation of UTF8String (and does so since macOS 10.6, I checked by downloading and disassembling the old binaries), so we needn't worry here.

This works because UTF8String returns a copy when called on mutable
strings.
This should improve performance of using NSString's Display impl.
@madsmtm madsmtm added documentation Improvements or additions to documentation enhancement New feature or request A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates A-framework Affects the framework crates and the translator for them labels Jun 12, 2024
@madsmtm madsmtm force-pushed the ns-string-as-str branch 2 times, most recently from 03c6fa4 to f106e32 Compare June 12, 2024 23:51
@madsmtm madsmtm merged commit 303edaf into master Jun 13, 2024
19 checks passed
@madsmtm madsmtm deleted the ns-string-as-str branch June 13, 2024 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Affects the framework crates and the translator for them A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant