introduce storage marker trait

[srijs]

This is exploring the use of a marker trait to address #9.

[carllerche]

Thanks for putting this together. I agree that the ability to corrupt the UTF-8 string is not great. However, one goal is to be able to be able to store any "byte container" in String. I believe that this change would require byte containers to opt-in by implementing Storage?

[carllerche]

Thoughts @arthurprs @seanmonstar?

[carllerche]

Unfortunately, it seems that the marker trait is the least bad option here...

What we probably can do is:

• Only require the marker trait on safe construction functions.
• Deref, ... impls stay as they are today.

This would still allow for the unsafe constructor, which is an escape hatch to pass in types that don't implement Storage.

Thoughts?

[srijs]

Makes total sense -- just pushed an update that implements your suggestion.

I've renamed the trait to Safe for the time being, because Storage seemed to imply that this covers all possible storage types, when now in reality it only covers the types that are guaranteed to be safe to use as backing storage. Of course I'm happy to rename based on your preference.

[seanmonstar]

I find Safe to be a little too generic of a name. It's clear when you read the documentation for the trait, but when implementing for a local type, it'd force anyone to go read the documentation during a review. Compare what I mean:

unsafe impl string::Safe for MyBuffer {}

unsafe impl string::StableAsRef for MyBuffer {}

It's not so bad that I'd throw a fit, just though I'd add my 2 cents.

[srijs]

Oh, yeah, no worries, I‘m totally happy to rename this to something else. If we think e.g. StableAsRef reflects best what this trait does, let‘s use that name!

[carllerche]

Sounds good to me 👍

[srijs]

Renamed to StableAsRef and resolved conflicts!

[carllerche]

Thanks. Now the question is: Should string depend on bytes or should bytes depend on string (optional dep either way).

[srijs]

It seems like it might be a good idea to have a dependency on the crate that is less likely to introduce breaking changes. Which one that is I don't know -- string has the smaller surface area, however bytes has been around for longer, so is presumably more stable.

An alternative might be to move the trait into a separate crate that can be consumed from both, but I'm not sure that's worth it?

[srijs]

@carllerche let me know if there's anything I can do to help get this over the line!

[carllerche]

Sorry for the delay

[carllerche]

Thanks for the PR and finding the issue 👍