Deriving external traits on external structs

help
41

They still could with this proposal. Keep in mind:

  • Currently, nothing prevents the trait author from being the one to implement a trait, which would also lead to the issues you describe.
  • The proposal does not stop the struct author from providing an implementation.

The set of source code possible with this proposal is a superset of the set of source code possible with the current restrictions. Yes, external crates cannot access private fields and so they would not be able to provide implementations which depended on them, but if they don't have a need for the private fields in their implementation then why restrict it artificially?

42

I've created a thread over at the internals forum:

2 Likes
43

To add a semi-closing remark about this particular issue: For the time being, I forked the repo and attempted to implement the recommended serde derive feature, only to notice that some of the structs are using types from yet another crate which does not implement {Serialize, Deserialize}. This means, with the current limitations, I have to create another pull request for bytesize, wait for it to go through and get pushed to crates.io (because, presumably, the maintainer of systemstat would want to depend on the stable crates.io version rather than a github link), make a pull request for systemstat, and then wait for that to push to crates.io before pushing my crate.

This is what I meant by killing the ecosystem. The fact that this happens most commonly, at the moment, with serde, and the fix is to make a recommendation that all crates with public structs implement Serialize and Deserialize is not a sustainable solution.

Edit: Turns out bytesize has already implemented the optional serde dependency in their GitHub repo, but hasn't pushed a new version to crates.io yet, so it's just a waiting game. This is not a complaint with bytesize, this is exactly the sort of issue I was describing, and will only get worse.

As more crates are created, data structures will become more deeply nested. New traits will be created which are not as generally applicable as Serialize, Deserialize, and therefore do not warrant inclusion in a general recommendation for all crates. Users wanting to combine such structs and traits will be left with an option of

  • Contacting multiple maintainers with pull requests to derive or implement an optional feature within their crate which has little to do with them and perhaps little general applicability.
  • Creating a newtype pattern and then manually implement the trait, duplicating the work of the implementation and introducing the possibility that the implementation of the derived trait and their own go out of sync.
  • Duplicating the external structs in their own crate so that they can leverage the derive functionality and avoid creating their own implementation, with the resulting overhead of converting between the external struct and their own, along with the additional fragility related to depending on the entirety of the public struct.

None of these are good.

6 Likes
44

this is a reading for a week at least. Anyway, I'll just drop 2 cents here, maybe there's stuff planned or implemented already since those postings times.

In literally last 10 minutes, I found out that 3rd party struct (roxmltree) Error does not implement From (or To) for String, so it cannot be converted into String if I use it like

let document = Document::parse(str)?;

in a fn returning Result<String, String>

Then it turned out that an enum from the same trait does not implement Display so again..

I understand (some of) the concerns about orphan rules but may be it could be relaxed a bit for something that's not a library, for example? or fenced behing some #[allow_me_go_nuts] flag? Just asking )

Maybe there're more or less "idiomatic" ways of doing what I want without breaking orphan rules?

2 Likes
45

The idiomatic way of doing these is (usually) to create a newtype that wraps the type you want to implement something for:

struct XmlError(roxmltree::Error);
impl From<XmlError> for String {
    // implement it here
}
// in your function
let document = Document::parse(str).map_err(XmlError)?;

Though in general I'd recommend against returning a String as an error type.

As a sidenote, roxmltree::Error implements Display, so you can just use that to get a String:

let document = Document::parse(str).map_err(|e| e.to_string())?;
1 Like