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.

Sign up for GitHub

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

Jump to bottom

Define non-panicking UTF encoding methods on char #52579

Closed
strake opened this issue Jul 20, 2018 · 2 comments
Closed

Define non-panicking UTF encoding methods on char #52579

strake opened this issue Jul 20, 2018 · 2 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@strake
Copy link
Contributor

strake commented Jul 20, 2018

i.e. pub fn encode_utf8(self, &mut [u8]) -> Option<&mut str> which returns None if buffer too short

PR to follow
@strake strake changed the title Define non-panicking UTF decoding methods on char Define non-panicking UTF encoding methods on char Jul 20, 2018
@hellow554
Copy link
Contributor

hellow554 commented Jul 23, 2018 •
edited
Loading

I wonder, why you just don't provide a 4-byte buffer which will always succeed (even on a embedded platform, that is space for a 4 byte array ;) )

Panics if the buffer is not large enough. A buffer of length four is large enough to encode any char.

@strake strake mentioned this issue Aug 9, 2018
Closed
@estebank estebank added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 8, 2019
@Mark-Simulacrum Mark-Simulacrum mentioned this issue Dec 23, 2019
Merged
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Dec 24, 2019
@Mark-Simulacrum
Rollup merge of rust-lang#67569 - Mark-Simulacrum:opt-char-encode, r=…
cb5e4ab 
…oli-obk

Clean up unsafety in char::encode_utf8

This originally started as an attempt to allow LLVM to optimize through
encode_utf8 to detect the try_encode_utf8 case (rust-lang#52579, rust-lang#52580), but due to a
typo my conclusion that my optimizations were successful was incorrect.

Furthermore, as far as I can tell, this optimization is probably just not
possible with LLVM today.  This [code](https://rust.godbolt.org/z/JggRj4)
compiles down to a long series of compares, notably, two identical series of
compares. That essentially means that LLVM is today unable to see that these two
ifs are identical and as such can be merged and then realize that no value of
the if condition can result in a call to `please_delete`. As such, for now, we
do not attempt to specifically optimize for that case.
Centril added a commit to Centril/rust that referenced this issue Dec 24, 2019
@Centril
Rollup merge of rust-lang#67569 - Mark-Simulacrum:opt-char-encode, r=…
20d5df9 
…oli-obk

Clean up unsafety in char::encode_utf8

This originally started as an attempt to allow LLVM to optimize through
encode_utf8 to detect the try_encode_utf8 case (rust-lang#52579, rust-lang#52580), but due to a
typo my conclusion that my optimizations were successful was incorrect.

Furthermore, as far as I can tell, this optimization is probably just not
possible with LLVM today.  This [code](https://rust.godbolt.org/z/JggRj4)
compiles down to a long series of compares, notably, two identical series of
compares. That essentially means that LLVM is today unable to see that these two
ifs are identical and as such can be merged and then realize that no value of
the if condition can result in a call to `please_delete`. As such, for now, we
do not attempt to specifically optimize for that case.
@rylev
Copy link
Member

rylev commented Jun 11, 2021

Triage: closing since the associated PR was closed by the lang team.

@rylev rylev closed this as completed Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@hellow554 @strake @rylev @estebank