remove unsafe from serde and add #![forbid(unsafe_code)]

[bkontur]

we are trying to use safe external dependencies as much as possible, means checked by cargo geiger,
and we found that, there is a only one unsafe usage in serde:
https://github.com/serde-rs/serde/blob/v1.0.130/serde/src/ser/impls.rs#L739

// We've only written ASCII bytes to the buffer, so it is valid UTF-8
serializer.serialize_str(unsafe { str::from_utf8_unchecked(&buf[..written]) })

do you think it is possible to rewrite this code without unsafe usage?
if yes, then you could add #![forbid(unsafe_code)] to the lib.rs and make it safe for cargo geiger

we made a temporary fork to support our needs here:
https://github.com/tezedge/serde/tree/cleanup-unsafe

[Lokathor]

Skipping the additional utf8 check when you know you've only written ascii is exactly the sort of speedup situation that having unsafe is supposed to enable.

[jonasbb]

It is worth noting that this unsafe code got introduced as part of #2001 to fix a performance issue. Any reversal of this should probably check that the performance loss is acceptable.

[danielhenrymantilla]

If it's just for one instance of unsafe, what about exposing an extra Cargo feature which would replace that unsafe usage with a checked body?

//! src/lib.rs
#![cfg_attr(feature = "forbid-unsafe", forbid(unsafe_code))]

let ascii_str = str::from_utf8(&buf[.. written]).unwrap_or_else(|_err| {
    #[cfg(feature = "forbid-unsafe")]
    unreachable!("Expected ASCII bytes only: {}", _err);

    #[cfg(not(feature = "forbid-unsafe"))]
    unsafe {
        ::core::hint::unreachable_unchecked();
    }
});
serializer.serialize_str(ascii_str)

That way the safety-(over-)zealous people would be able to ensure serde uses no unsafe, while the rest of the community can still be using the slightly more performant implementation that skips the unnecessary UTF-8 check.

[golddranks]

There is a draft RFC for Cargo applying patches, that makes it easier for downstream projects to customize their dependencies. This looks like a potential use case. Here's the draft: rust-lang/rfcs#3177

[dtolnay]

I would prefer not to make this change, but I do welcome scrutiny of whether any unsafe code in serde or its dependencies is correct. FWIW unsafe code in serde is probably scrutinized >10× more than the typical unsafe usage in the Rust standard library (but I still have never understood why people feel compelled to treat it so differently).