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

Ability to automatically derive traits on newtypes #8353

Closed
Seldaek opened this issue Aug 6, 2013 · 13 comments
Closed

Ability to automatically derive traits on newtypes #8353

Seldaek opened this issue Aug 6, 2013 · 13 comments

Comments

@Seldaek
Copy link
Contributor

Seldaek commented Aug 6, 2013

Disclaimer

Please bear with me, this contains long code samples but you can skip through most of them until the last one if you are only mildly interested, they're just there to illustrate my point.

Problem

While adding comments on rand::rng() I figured that it would be a lot better if the function would return a generic "interface" type for Rngs instead of returning the type of the particular struct it uses at the moment. However after discussing this with @kballard and @cmr on IRC, it seems that it is now pretty much impossible to achieve.

I'll go through the variants I researched, and then propose a solution (which may not be realistic, I have no clue about rustc internals, but I hope it is).

Failed attempt 1: Returning traits as a makeshift interface

This failed miserably because it's just not how the language works at all, and I got that now, but it's included for completeness.

use std::rand::{IsaacRng, XorShiftRng, RngUtil, Rng};

fn main() {
    let rng = rng();
    printfln!(rng.gen_bytes(10));
}

pub fn rng<T: Rng + RngUtil>() -> T {
    IsaacRng::new()
}

pub fn weak_rng<T: Rng + RngUtil>() -> T {
    XorShiftRng::new()
}
test.rs:6:4: 7:1 error: mismatched types: expected `T` but found `std::rand::IsaacRng` (expected type parameter but found struct std::rand::IsaacRng)
test.rs:6     IsaacRng::new()
test.rs:7 }
test.rs:10:4: 11:1 error: mismatched types: expected `T` but found `std::rand::XorShiftRng` (expected type parameter but found struct std::rand::XorShiftRng)
test.rs:10     XorShiftRng::new()
test.rs:11 }
test.rs:15:14: 15:31 error: the type of this value must be known in this context
test.rs:15     printfln!(rng.gen_bytes(10));
                         ^~~~~~~~~~~~~~~~~

Failed attempt 2: Using newtypes to "hide" the real implementation

This would be an ok workaround, it's not perfect but it seems to be workable given the constraints of the language and isn't too crazy, however it fails because it is missing the proper implementations:

use std::rand::{IsaacRng, XorShiftRng, RngUtil, Rng};

fn main() {
    let rng = rng();
    printfln!(rng.gen_bytes(10));
}

struct StrongRng(IsaacRng);

pub fn rng() -> StrongRng {
    StrongRng(IsaacRng::new())
}

struct WeakRng(XorShiftRng);

pub fn weak_rng() -> WeakRng {
    WeakRng(XorShiftRng::new())
}
test.rs:19:14: 19:31 error: failed to find an implementation of trait std::rand::Rng for StrongRng
test.rs:19     printfln!(rng.gen_bytes(10));
                         ^~~~~~~~~~~~~~~~~

Failed attempt 3: Implementing a decorator newtype

This probably is workable unless I don't get how to work around the compiler errors that are left, but I assume it's not a dead end. The problem though is that it is extremely verbose and seriously painful to write all this boilerplate just to have a straight decorator wrapping the underlying struct.

use std::rand::{IsaacRng, XorShiftRng, RngUtil, Rng, Weighted, Rand};

fn main() {
    let rng = rng();
    printfln!(rng.gen_bytes(10));
}

struct StrongRng(IsaacRng);
impl Rng for StrongRng {
    pub fn next(&mut self) -> u32 {
        return (**self).next();
    }
}
impl RngUtil for StrongRng {
    fn gen<T:Rand>(&mut self) -> T {
        return (**self).gen();
    }
    fn gen_int_range(&mut self, start: int, end: int) -> int {
        return (**self).gen_int_range(start, end);
    }
    fn gen_uint_range(&mut self, start: uint, end: uint) -> uint {
        return (**self).gen_uint_range(start, end);
    }
    fn gen_char_from(&mut self, chars: &str) -> char {
        return (**self).gen_char_from(chars);
    }
    fn gen_weighted_bool(&mut self, n: uint) -> bool {
        return (**self).gen_weighted_bool(n);
    }
    fn gen_str(&mut self, len: uint) -> ~str {
        return (**self).gen_str(len);
    }
    fn gen_bytes(&mut self, len: uint) -> ~[u8] {
        return (**self).gen_bytes(len);
    }
    fn choose<T:Clone>(&mut self, values: &[T]) -> T {
        return (**self).choose(values);
    }
    fn choose_option<T:Clone>(&mut self, values: &[T]) -> Option<T> {
        return (**self).choose_option(values);
    }
    fn choose_weighted<T:Clone>(&mut self, v : &[Weighted<T>]) -> T {
        return (**self).choose_weighted(v);
    }
    fn choose_weighted_option<T:Clone>(&mut self, v: &[Weighted<T>]) -> Option<T> {
        return (**self).choose_weighted_option(v);
    }
    fn weighted_vec<T:Clone>(&mut self, v: &[Weighted<T>]) -> ~[T] {
        return (**self).weighted_vec(v);
    }
    fn shuffle<T:Clone>(&mut self, values: &[T]) -> ~[T] {
        return (**self).shuffle(values);
    }
    fn shuffle_mut<T>(&mut self, values: &mut [T]) {
        (**self).shuffle_mut(values);
    }
}

pub fn rng() -> StrongRng {
    StrongRng(IsaacRng::new())
}

struct WeakRng(XorShiftRng);
impl Rng for WeakRng {
    pub fn next(&mut self) -> u32 {
        return (**self).next();
    }
}
impl RngUtil for WeakRng {
    fn gen<T:Rand>(&mut self) -> T {
        return (**self).gen();
    }
    fn gen_int_range(&mut self, start: int, end: int) -> int {
        return (**self).gen_int_range(start, end);
    }
    fn gen_uint_range(&mut self, start: uint, end: uint) -> uint {
        return (**self).gen_uint_range(start, end);
    }
    fn gen_char_from(&mut self, chars: &str) -> char {
        return (**self).gen_char_from(chars);
    }
    fn gen_weighted_bool(&mut self, n: uint) -> bool {
        return (**self).gen_weighted_bool(n);
    }
    fn gen_str(&mut self, len: uint) -> ~str {
        return (**self).gen_str(len);
    }
    fn gen_bytes(&mut self, len: uint) -> ~[u8] {
        return (**self).gen_bytes(len);
    }
    fn choose<T:Clone>(&mut self, values: &[T]) -> T {
        return (**self).choose(values);
    }
    fn choose_option<T:Clone>(&mut self, values: &[T]) -> Option<T> {
        return (**self).choose_option(values);
    }
    fn choose_weighted<T:Clone>(&mut self, v : &[Weighted<T>]) -> T {
        return (**self).choose_weighted(v);
    }
    fn choose_weighted_option<T:Clone>(&mut self, v: &[Weighted<T>]) -> Option<T> {
        return (**self).choose_weighted_option(v);
    }
    fn weighted_vec<T:Clone>(&mut self, v: &[Weighted<T>]) -> ~[T] {
        return (**self).weighted_vec(v);
    }
    fn shuffle<T:Clone>(&mut self, values: &[T]) -> ~[T] {
        return (**self).shuffle(values);
    }
    fn shuffle_mut<T>(&mut self, values: &mut [T]) {
        (**self).shuffle_mut(values);
    }
}
pub fn weak_rng() -> WeakRng {
    WeakRng(XorShiftRng::new())
}
test.rs:70:14: 70:31 error: multiple applicable methods in scope
test.rs:70     printfln!(rng.gen_bytes(10));
                         ^~~~~~~~~~~~~~~~~
note: in expansion of fmt!
<std-macros>:226:20: 226:37 note: expansion site
<std-macros>:224:4: 231:5 note: in expansion of printfln!
test.rs:70:4: 70:33 note: expansion site
test.rs:98:4: 100:5 note: candidate #1 is `__extensions__::gen_bytes`
test.rs:98     fn gen_bytes(&mut self, len: uint) -> ~[u8] {
test.rs:99         return (**self).gen_bytes(len);
test.rs:100     }
test.rs:70:14: 70:31 note: candidate #2 is `std::rand::__extensions__::gen_bytes`
test.rs:70     printfln!(rng.gen_bytes(10));
                         ^~~~~~~~~~~~~~~~~
note: in expansion of fmt!
<std-macros>:226:20: 226:37 note: expansion site
<std-macros>:224:4: 231:5 note: in expansion of printfln!
test.rs:70:4: 70:33 note: expansion site
test.rs:79:0: 122:1 error: multiple applicable methods in scope
test.rs:79 impl RngUtil for StrongRng {
test.rs:80     fn gen<T:Rand>(&mut self) -> T {
test.rs:81         return (**self).gen();
test.rs:82     }
test.rs:83     fn gen_int_range(&mut self, start: int, end: int) -> int {
test.rs:84         return (**self).gen_int_range(start, end);
           ...
test.rs:134:0: 177:1 error: multiple applicable methods in scope
test.rs:134 impl RngUtil for WeakRng {
test.rs:135     fn gen<T:Rand>(&mut self) -> T {
test.rs:136         return (**self).gen();
test.rs:137     }
test.rs:138     fn gen_int_range(&mut self, start: int, end: int) -> int {
test.rs:139         return (**self).gen_int_range(start, end);
            ...
error: aborting due to 3 previous errors

Proposal

The ideal way would be to allow the newtype to derive traits, so that making a decorator is not a PITA anymore. As you see right now it fails because the derive can't find the right impl, but I think it should just create it on the fly instead of complaining. That way we get explicit "interface" definition by defining which traits are implemented on the newtype, and we can return that and avoid a BC fiasco when we need to change the underlying RNG (see #8349 for more info on that).

I'd be happy to try and help implement this feature because I strongly believe this is needed, but to be honest I have no clue where to even start so pointers would be nice if the consensus is that it is a good thing and that it is at all feasible :)

use std::rand::{IsaacRng, XorShiftRng, RngUtil, Rng};

fn main() {
    let rng = rng();
    printfln!(rng.gen_bytes(10));
}

#[derive(Rng, RngUtil)]
struct StrongRng(IsaacRng);

pub fn rng() -> StrongRng {
    StrongRng(IsaacRng::new())
}

#[derive(Rng, RngUtil)]
struct WeakRng(XorShiftRng);

pub fn weak_rng() -> WeakRng {
    WeakRng(XorShiftRng::new())
}
test.rs:70:14: 70:31 error: failed to find an implementation of trait std::rand::Rng for StrongRng
test.rs:70     printfln!(rng.gen_bytes(10));
                         ^~~~~~~~~~~~~~~~~
note: in expansion of fmt!
@Seldaek
Copy link
Contributor Author

Seldaek commented Aug 6, 2013

So as pointed out by folks on IRC, the 3rd attempt is indeed workable and I am an idiot, this makes it run fine:

use std::rand::{IsaacRng, XorShiftRng, RngUtil, Rng};

fn main() {
    let mut rng = rng();
    printfln!(rng.gen_bytes(10));
}

struct StrongRng(IsaacRng);
impl Rng for StrongRng {
    pub fn next(&mut self) -> u32 {
        return (**self).next();
    }
}

pub fn rng() -> StrongRng {
    StrongRng(IsaacRng::new())
}

struct WeakRng(XorShiftRng);
impl Rng for WeakRng {
    pub fn next(&mut self) -> u32 {
        return (**self).next();
    }
}

pub fn weak_rng() -> WeakRng {
    WeakRng(XorShiftRng::new())
}

Still the proposed solution is shorter and nicer with less boilerplate.

@emberian
Copy link
Member

emberian commented Aug 7, 2013

I imagine a more generic syntaxext, for encoding "Proxy Objects". For example:

#[Proxy(Rng, (*self))]
struct WeakRng(XorShiftRng);

would expand to

struct WeakRng(XorShiftRng);

impl Rng for WeakRng {
    pub fn next(&mut self) -> u32 {
        (*self).next()
    }
}

This would also allow:

#[Proxy(SomeTrait, self.a)]
struct Foo {
    a: SomeT,
    b: OtherT
}

The form is #[Proxy(TraitName, Expression)], and it implements every method on TraitName as Expression.method(). Maybe this isn't feasible though, I'm not sure if the attribute syntax can handle arbitrary Rust expressions.

@huonw?

@glaebhoerl
Copy link
Contributor

The problem with that is it breaks down if you have Self in the return type, for example... if we just report an error for that then why not, I guess. The same idea has popped up in other places like #7773 and the Overloadable deference operator discussion on the ML.

The idea in the OP is GHC's GeneralizedNewtypeDeriving (bunch of detail there, might be worth reading), which I support. One notable property of it is that it re-uses the same impl for the newtype as for the base (appropriately transmuted), instead of generating a new one, which isn't necessarily true of the more general Proxy idea.

@graydon
Copy link
Contributor

graydon commented Aug 7, 2013

Plausible. Though you know that all the cool kids would call this delegate, not proxy. I don't have a strong feeling on this. Much older designs of rust had some mechanisms like this but I never worked out a satisfactory / orthogonal way of bolting it on to the language without feeling totally ad-hoc. This looks ... at least more self-contained than any of the messes I made in those sketches?

(ProxyDelegateFactoryDecoratorOneMoreLevelOfIndirection joke goes here)

@emberian
Copy link
Member

emberian commented Aug 7, 2013

Delegate, proxy, whatever. I'm not jiggy with the jive yet. @pcwalton expressed concern that GeneralizedNewtypeDeriving broke GHC's type system, and that it still hasn't been fixed. I don't know anything about such things, I just know that text-based code generation similar to the attribute I suggest works for a lot of things 😄

@emberian
Copy link
Member

emberian commented Aug 7, 2013

In any case the compiler could probably do inlining and other stuff to make the cost go away, those "proxy" or "delegate" methods are awful small

@glaebhoerl
Copy link
Contributor

Here's the paper discussing the problem and GHC's solution to it, if anyone's interested.

The basic shape of it is that GNTD acts as if, for the purposes of deriving those traits, the newtype and its base were the same type. And, representationally, they are. But otherwise they aren't, and if you have ways to distinguish them and "branch" on the difference at the type level - GADTs, type-level functions (~associated types), what have you - you can derive a contradiction. I don't know if Rust has any way to do that right now (those examples are things it doesn't have), but it's something to keep in mind. If GNTD were restricted to only deriving impls you could also write by hand, that would remove the potential for conflict, so if we don't want to add a similar distinction to the type system between equality of types and equality of representations as GHC will soon have, that might be the way to go.

The broader issue the paper discusses is that while you can cast between a newtype and its base at no cost, that's no longer true if they're embedded in another type. E.g. if you have struct Age(int), the only way to go between ~[Age] and ~[int] in safe code is to map the newtype cast (a no-op) over the elements and create a new array, while (if the newtype is not opaque to you) it should be safe to cast directly. I don't know whether Rust has bumped into this problem yet.

@huonw
Copy link
Member

huonw commented Aug 7, 2013

This cannot work as a syntax extension: it has no idea what methods a given trait has, nor their signature. It might work if we extend it to something like:

#[deriving(Rng(method="fn next(&mut self) -> u32"))]
...

so the syntax extension had access to the signatures.

In any case, one doesn't need to implement RngUtil by hand (or any ...Util trait, by convention), it has an impl of the form impl<R: Rng> RngUtil for R, i.e. every Rng has all those methods automatically.

Also, I proposed something very similar to this in #7773. (edit: Whoops, already linked to.)

@emberian
Copy link
Member

emberian commented Aug 7, 2013

@huonw attributes can be handled at any point in the pipeline, what is the problem with doing this during/after typecheck?

@huonw
Copy link
Member

huonw commented Aug 7, 2013

@cmr, none at all, but it means it can't be a normal #[deriving].

In any case the "Returning traits as a makeshift interface" is essentially a trait object: fn strong_rng() -> ~Rng. (this says "there exists an Rng that this function returns", the <T: Rng> ... -> T, says "this function can return whatever type implementing Rng the caller wishes").

@pnkfelix
Copy link
Member

pnkfelix commented Nov 4, 2013

I think the Decorator approach is fine here, at least for 1.0. Yes, it involves boilerplate, which is a (small) shame. But I don't want people distracted by this bug in the short term.

We used to have a "Far future" milestone to nominate and/or tag such items, but I guess it is gone now. So, what, do I explicitly nominate this for "P-low" ? I don't know if that conveys my intent correctly.

@alexcrichton
Copy link
Member

This RFC seems related: rust-lang/rfcs#186

@rust-highfive
Copy link
Collaborator

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

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

8 participants