Generate unsafe code conditionally with feature

[mpfaff]

Only generated unsafe code when a feature (named "unsafe" or something) is enabled. It would be fine for this feature to be on by default, but would allow use in projects that forbid unsafe code.

[neoeinstein]

Unfortunately, this is not possible when also providing the borrowed form of the type. Converting a string slice into the borrowed type requires the use of unsafe. I discuss this further in a comment regarding the unsafe requirement over in a Reddit thread.

In particular, this crate is about making sure that the unsafe code required is implemented precisely and correctly every time. I'll also note that each use of unsafe in the macro generation is documented with SAFETY comments that identifies the implicit contracts that are being upheld at each step.

One potential workaround that I didn't mention in that Reddit thread would require exposing hidden "safe" functions from this crate that aren't really safe, and I don't want to put out a public API that (particularly in a hidden, undocumented API) lies to the consumer. If you have a crate that absolutely must forbid unsafe rather than just deny it, then I would recommend segregating the braids into something like a mycrate_braids crate, generating the braids there, and importing them into mycrate.

A final, and much more restrictive option, would be to have a means of generating only the owned wrappers. That could be reasonable, but it does mean that you can't just borrow a refrence out of an existing buffer, you'd always need to operate with the owned value (or references to that owned value) in order to carry the type's invariants along. Perhaps that's desirable.

What are your thoughts on this?

[mpfaff]

I'd say that for crates that absolutely must avoid unsafe code, an inability to generate borrowed wrappers is a fair tradeoff.

I will note that it's not entirely clear to me from reading the README what the purpose of the borrowed wrappers are. To me MyWrapperRef looks equivalent to &MyWrapper (although I did see an example passing &MyWrapperRef which I take to mean it serves a different purpose).

I've looked at all the uses of unsafe code and while I do think I understand what they're doing and why, I still think making it optional would be a good idea.

[neoeinstein]

The difference between MyWrapper and MyWrapperRef is analagous to the difference between String and str or PathBuf and Path. You certainly can pass around a &String, but that looks weird, means that if you've been give &str, you must copy the value into an owned String before taking a reference to that to share it. This can also make it awkward to use as the key for a BTreeMap or HashMap, as you can't just pass a borrowed version of the key; you'll need to create an owned version.

There's a second reason that most idiomatic Rust APIs that don't need ownership of the value take &str, &OsStr, &Path, &[u8], etc. instead of &String, &OsString, &PathBuf, &Vec<u8>: The latter add a second layer of indirection that can have significant performance penalties. A &String is a pointer to a pointer to the actual data, but &str is just a pointer to the actual data (with lenght). Similarly, having &MyWrapperRef allows you to borrow the underlying value without adding unnecessary levels of indirection.

More context about the relative costs here can be found on this StackOverflow answer.

I'll look at adding this request as a parameter to the crate. At that point, it basically will include the new and as_str functions and auto-derive: Clone, Hash, PartialEq, Eq, Debug, Display, FromStr, From/TryFrom for String and &str, and if requested, Serialize and Deserialize.

[mpfaff]

Oh, that makes a lot of sense. Thanks for explaining it.