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

ACP: impl TryFrom<char> for u16 #146

Closed
lukas-code opened this issue Dec 13, 2022 · 2 comments
Closed

ACP: impl TryFrom<char> for u16 #146

lukas-code opened this issue Dec 13, 2022 · 2 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@lukas-code
Copy link

Proposal

Problem statement

Implement TryFrom<char> for u16, similar to the existing TryFrom<char> for u8.

Motivation, use-cases

I have project, which (unfortunately) has to deal with UCS-2 encoded strings (UCS-2 is like UTF-16, but no surrogates). I am currently in the process of replacing as casts with safer alternatives and found that this impl was missing.

One example of using this feature for collecting an iterator of chars into a preallocated UCS-2 string: (playground)

use core::mem::MaybeUninit;
use core::slice;

#[derive(Debug)]
struct Error;

fn collect_ucs2(iter: impl IntoIterator<Item = char>, buffer: &mut [MaybeUninit<u16>]) -> Result<&[u16], Error> {
    let mut index = 0;
    for ch in iter {
        if index >= buffer.len() {
            return Err(Error);
        }
        let ucs2 = ch.try_into().map_err(|_| Error)?; // <- doesn't work :(
        buffer[index] = MaybeUninit::new(ucs2);
        index  = 1;
    }
    Ok(unsafe { slice::from_raw_parts(buffer.as_ptr().cast::<u16>(), index) })
}

fn main() {
    let mut buffer = [MaybeUninit::uninit(); 16];
    println!("{:X?}", collect_ucs2("hello".chars(), &mut buffer));
}

Solution sketches

Copy-paste the impl from u8, but replace u8 with u16:

impl TryFrom<char> for u16 {
    type Error = TryFromCharError;

    #[inline]
    fn try_from(c: char) -> Result<u16, Self::Error> {
        u16::try_from(u32::from(c)).map_err(|_| TryFromCharError(()))
    }
}

Links and related work

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.
@lukas-code lukas-code added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Dec 13, 2022
@scottmcm
Copy link
Member

My instinct is that this is too uncommon and situational to be a trait impl. I don't know that we want anything in traits that's UCS-2, since I don't think we even have UTF-16 in traits.

Overall, it just doesn't seem so bad to do

-let ucs2 = ch.try_into().map_err(|_| Error)?;
 let ucs2 = u32::from(ch).try_into().map_err(|_| Error)?;

so it's hard to pass the "we can't experiment with it because it's insta-stable" bar.

Is there a version of this that could be a method instead? The bar for fn encode_ucs2(self) -> Option<u16> or similar is much lower, though I'm still unsure whether it's something we want. And having the encoding in the name emphasizes that most people probably don't want it.

@m-ou-se
Copy link
Member

m-ou-se commented Jul 25, 2023

We discussed this in the libs-api meeting just now. There was some discussion on whether this would wrongly be used in situations where utf-16 should be used instead, but we mostly felt positive about the conversion. We didn't reach full consensus yet, but this change would be insta-stable (since it's a trait implementation), so can you send a PR to rust-lang/rust for this? Then we can propose an FCP and see if we can get consensus on it. Thanks!

@m-ou-se m-ou-se closed this as completed Jul 25, 2023
@lukas-code lukas-code mentioned this issue Jul 25, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 1, 2023
@bors
Auto merge of rust-lang#114065 - lukas-code:u16_from_char, r=dtolnay
361f8ba 
`impl TryFrom<char> for u16`

This PR implements the final missing `char` -> unsigned integer conversion.

ACP: rust-lang/libs-team#146

r? libs-api
`@rustbot` label  T-libs-api  needs-fcp -T-libs
RalfJung pushed a commit to RalfJung/miri that referenced this issue Sep 2, 2023
@bors
Auto merge of #114065 - lukas-code:u16_from_char, r=dtolnay
4ddb17a 
`impl TryFrom<char> for u16`

This PR implements the final missing `char` -> unsigned integer conversion.

ACP: rust-lang/libs-team#146

r? libs-api
`@rustbot` label  T-libs-api  needs-fcp -T-libs
@dtolnay dtolnay added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Nov 23, 2023
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
@bors
Auto merge of #114065 - lukas-code:u16_from_char, r=dtolnay
a400ef4 
`impl TryFrom<char> for u16`

This PR implements the final missing `char` -> unsigned integer conversion.

ACP: rust-lang/libs-team#146

r? libs-api
`@rustbot` label  T-libs-api  needs-fcp -T-libs
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
@bors
Auto merge of #114065 - lukas-code:u16_from_char, r=dtolnay
a82fc52 
`impl TryFrom<char> for u16`

This PR implements the final missing `char` -> unsigned integer conversion.

ACP: rust-lang/libs-team#146

r? libs-api
`@rustbot` label  T-libs-api  needs-fcp -T-libs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@m-ou-se @dtolnay @scottmcm @lukas-code