Make NonZero<char> possible

[hkBst]

I'd like to use NonZero<char> for representing units of CStr in https://github.com/rust-lang/literal-escaper

[rustbot]

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[hanna-kruppe]

It’s not clear from the diff, but I guess since NonZero<T> works by trait impls on T, this can’t be feature-gated and would immediately allow NonZero<char> on stable?

[jieyouxu]

r? libs-api

[hkBst]

It’s not clear from the diff, but I guess since NonZero<T> works by trait impls on T, this can’t be feature-gated and would immediately allow NonZero<char> on stable?

That's a good point I hadn't considered. I'll look into whether a feature gate is possible.

[tgross35]

Could you add a test so we don't accidentally regress this? library/coretests/tests/num/niche_types.rs doesn't exist yet but probably would be the place.

[hkBst]

Could you add a test so we don't accidentally regress this? library/coretests/tests/num/niche_types.rs doesn't exist yet but probably would be the place.

Do you mean a test that just uses all the possible NonZero-able types as NonZeros? Meaning they are tested for being possible. Or is there another thing you meant?

[tgross35]

I did only mean something to check basic usage of the type, and was only thinking of char since the other types are covered by the NonZeroUxx doctests. It wouldn't be bad to just add the same kind of test for all the types, though.

[hkBst]

@tgross35 I added a simple test for NonZero::<char>. Let me know if it is okay, or if you have any more ideas to make it better.

[Amanieu]

I'd like to use NonZero<char> for representing units of CStr in rust-lang/literal-escaper

Could give some examples of how this is going to be used in literal-escaper?

[hkBst]

Could give some examples of how this is going to be used in literal-escaper?

The characters of a C string are checked for being non-null as a matter of correctness anyway, and it may be a good idea to preserve this in the type.

I have a PR that did this (as a side quest) and the relevant file is here: https://github.com/rust-lang/rust/blob/30822ec0ec25723f36f9e73c42d91a83dc121388/compiler/rustc_lexer/src/unescape.rs

/// Used for mixed utf8 string literals, i.e. those that allow both unicode
/// chars and high bytes.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum MixedUnit {
    /// Used for ASCII chars (written directly or via `\x01`..`\x7f` escapes)
    /// and Unicode chars (written directly or via `\u` escapes).
    ///
    /// For example, if '¥' appears in a string it is represented here as
    /// `MixedUnit::Char('¥')`, and it will be appended to the relevant byte
    /// string as the two-byte UTF-8 sequence `[0xc2, 0xa5]`
    Char(NonZero<char>),

    /// Used for high bytes (`\x80`..`\xff`).
    ///
    /// For example, if `\xa5` appears in a string it is represented here as
    /// `MixedUnit::HighByte(0xa5)`, and it will be appended to the relevant
    /// byte string as the single byte `0xa5`.
    HighByte(NonZero<u8>),
}

impl From<NonZero<char>> for MixedUnit {
    fn from(c: NonZero<char>) -> Self {
        MixedUnit::Char(c)
    }
}

impl From<NonZero<u8>> for MixedUnit {
    fn from(byte: NonZero<u8>) -> Self {
        if byte.get().is_ascii() {
            MixedUnit::Char(NonZero::new(byte.get() as char).unwrap())
        } else {
            MixedUnit::HighByte(byte)
        }
    }
}

impl TryFrom<char> for MixedUnit {
    type Error = EscapeError;

    fn try_from(c: char) -> Result<Self, EscapeError> {
        NonZero::new(c).map(MixedUnit::Char).ok_or(EscapeError::NulInCStr)
    }
}

impl TryFrom<u8> for MixedUnit {
    type Error = EscapeError;

    fn try_from(byte: u8) -> Result<Self, EscapeError> {
        NonZero::<u8>::new(byte).map(From::from).ok_or(EscapeError::NulInCStr)
    }
}

[Amanieu]

Seems reasonable.

@rfcbot merge

[rfcbot]

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[tgross35]

@hkBst mind rebasing to get a fresh set of CI runs? IIRC library/coretests/tests/num/mod.rs had some changes, I'd like to make sure checks pass before merging.

[dtolnay]

Thank you!

[dtolnay]

@bors r+

[bors]

📌 Commit a87cd55 has been approved by dtolnay

It is now in the queue for this repository.

[bors]

⌛ Testing commit a87cd55 with merge dcd1b16...

[bors]

💔 Test failed - checks-actions