Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crates should allow private impl of external traits for external structs #13721

Closed
alanfalloon opened this issue Apr 24, 2014 · 11 comments
Closed

Comments

@alanfalloon
Copy link

As you know, you can't provide an impl for a trait if neither the type nor the trait are defined in the current crate, you get:

error: cannot provide an extension implementation where both trait and type are not defined in this crate

For public implementations, this makes perfect sense, there are issues with collisions between implementations and sheer surprise. However, for a private implementation of the trait, this is a real hindrance.

Consider the case of std::path::Path not implementing std::fmt::Show. The rationale for closing #13009 is perfectly valid: we don't want people treating paths as strings. However, by refusing to add an implementation of Show, you have made that decision for all programs everywhere. In my case, I had a large struct with numerous Path elements that I wanted to print for debugging, but #[deriving(Show)] won't work because Path has no Show impl, and now I'm stuck either implementing it tediously from scratch, or switching to str for my path names.

The perfect compromise would have been to allow my crate to define a private Show impl for Path. There is precedent for this in other languages, go allows interfaces to implemented anywhere, for example.

@alanfalloon
Copy link
Author

Ran in to another use-case for #13009 that would be alleviated by allowing private impls: I'm trying to test an iterator that returns std::path::Path but I can't use assert_eq! on any of the results (neither .collect() to a vector, nor the Option<Path> returns of the .next() method itself) because of the missing std::fmt::Show trait. In this case, I could even gate the private impl in #[cfg(test)] to keep in the spirit of not using Path as a string in my normal code.

@lilyball
Copy link
Contributor

lilyball commented May 8, 2014

For the latter, you could map to a byte vector (either &[u8] or Vec<u8>, depending on lifetime requirements) and assert equality there. That will be equivalent to asserting equality on the Path (Path's impl of Eq actually just compares the internal byte vector).

As for your struct, here's a wacky idea. We could extend #[deriving(Show)] to let you override the display of individual fields. Something like

#[deriving(Show)]
struct MyStruct {
    x: int
    #[deriving_show="p.display()"]
    p: Path
    y: int
}

I don't know if this really is a great idea, but it's probably easier than coming up with some way to privately implement a trait for an external type.

@alanfalloon
Copy link
Author

I did something similar in my testing by wrapping my results in a struct with a Path member and defining Show on that.

The private implementations of impls though are still a very useful feature. Go leans fairly heavily on its ability to define ad-hoc local implementations of interfaces in a module.

@lilyball
Copy link
Contributor

lilyball commented May 8, 2014

@alanfalloon As I understand it, to implement an interface on an existing type in Go you need to actually produce a newtype and implement the interface on that. That's precisely what HandlerFunc is doing. And you can do the exact same thing in Rust.

struct ShowablePath(Path);

impl Show for ShowablePath {
    // ...
}

The downside is you need to explicitly unwrap ShowablePath to get at the underlying Path, but you could mitigate that by implementing Deref as well to do the unwrapping for you.

@dead10ck
Copy link

I'm very much in favor of this as well. It should be possible to extend existing types without having to wrap them in a unit struct.

@jaredly
Copy link

jaredly commented Nov 19, 2014

yes please! 👍

@aarondandy
Copy link

The possibilities opened by traits like this are part of why I got excited about Rust. 👍

@leonkunert
Copy link

+1

@sfackler
Copy link
Member

This should probably move over to rust-lang/rfcs. (cc @nick29581)

@rust-highfive
Copy link
Collaborator

This issue has been moved to the RFCs repo: rust-lang/rfcs#493

arcnmx pushed a commit to arcnmx/rust that referenced this issue Dec 17, 2022
Support `rustc_has_incoherent_inherent_impls`

Fixes us not resolving `<dyn Error>::downcast` now that `Error` moved to core, while that assoc function is declared in `alloc`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants