Skip to content

Forbid type impls on typedefs #6087

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

Closed
wants to merge 14 commits into from

Conversation

pcwalton
Copy link
Contributor

r? @brson

@brendanzab
Copy link
Member

D:

Will this forbid:

trait BaseVec<T> {
    fn from_value(value: T) -> Self;
}

struct Vec3<T> { x: T, y: T, z: T }

impl<T: Copy> BaseVec<T> for Vec3<T> {
    fn from_value(value: T) -> Vec3<T> { Vec3 { x: value, y: value, z: value } }
}

type Vec3f = Vec3<float>;

impl Vec3f {
    fn from_value(value: float) -> Vec3f { BaseVec::from_value(value) }
}

fn main() {
    let v = Vec3f::from_value(3.0);
}

@pcwalton
Copy link
Contributor Author

Yes, that will not work. Is it possible to restructure your code with something like:

impl<T:Num> Vec3<T> {
    fn from_value(value: T) -> Self { ... }
}

?

@brendanzab
Copy link
Member

Dang, that's a major pain. That would become problematic for doing type impls which use different trait constraints on T. Eg. some static methods work for a general value of T, where as others may only work for T: Num. Because you can't do multiple impls on the same type, I'm stuck. It also means losing the ability for the user to decide their vector type up front.

I don't want to give up on the generic type class approach, but it might be easier now to create a big macro to generate each vector type instead. :(

My goal is to produce a beautiful API for my users, with minimal code duplication and possibility of bugs on my end. But I currently feel like I don't really have the tools to do that, and the solutions I was using are now being taken away. I'll have to do more thinking – I'm sure there are other ways around this.

@brendanzab
Copy link
Member

This is current code in lmath:

macro_rules! vec3_type(
    ($name:ident <bool>) => (
        pub impl $name {
            #[inline(always)] fn new(x: bool, y: bool, z: bool) -> $name { BaseVec3::new(x, y, z) }
            #[inline(always)] fn from_value(v: bool) -> $name { BaseVec::from_value(v) }

            #[inline(always)] fn dim() -> uint { 3 }
            #[inline(always)] fn size_of() -> uint { sys::size_of::<$name>() }
        }
    );
    ($name:ident <$T:ty>) => (
        pub impl $name {
            #[inline(always)] fn new(x: $T, y: $T, z: $T) -> $name { BaseVec3::new(x, y, z) }
            #[inline(always)] fn from_value(v: $T) -> $name { BaseVec::from_value(v) }
            #[inline(always)] fn identity() -> $name { NumVec::identity() }
            #[inline(always)] fn zero() -> $name { NumVec::zero() }

            #[inline(always)] fn unit_x() -> $name { NumVec3::unit_x() }
            #[inline(always)] fn unit_y() -> $name { NumVec3::unit_y() }
            #[inline(always)] fn unit_z() -> $name { NumVec3::unit_z() }

            #[inline(always)] fn dim() -> uint { 3 }
            #[inline(always)] fn size_of() -> uint { sys::size_of::<$name>() }
        }
    );
)

// GLSL-style type aliases, corresponding to Section 4.1.5 of the [GLSL 4.30.6 specification]
// (http://www.opengl.org/registry/doc/GLSLangSpec.4.30.6.pdf).

// a three-component single-precision floating-point vector
pub type vec3  = Vec3<f32>;
// a three-component double-precision floating-point vector
pub type dvec3 = Vec3<f64>;
// a three-component Boolean vector
pub type bvec3 = Vec3<bool>;
// a three-component signed integer vector
pub type ivec3 = Vec3<i32>;
// a three-component unsigned integer vector
pub type uvec3 = Vec3<u32>;

vec3_type!(vec3<f32>)
vec3_type!(dvec3<f64>)
vec3_type!(bvec3<bool>)
vec3_type!(ivec3<i32>)
vec3_type!(uvec3<u32>)

// Rust-style type aliases
pub type Vec3f   = Vec3<float>;
pub type Vec3f32 = Vec3<f32>;
pub type Vec3f64 = Vec3<f64>;
pub type Vec3i   = Vec3<int>;
pub type Vec3i8  = Vec3<i8>;
pub type Vec3i16 = Vec3<i16>;
pub type Vec3i32 = Vec3<i32>;
pub type Vec3i64 = Vec3<i64>;
pub type Vec3u   = Vec3<uint>;
pub type Vec3u8  = Vec3<u8>;
pub type Vec3u16 = Vec3<u16>;
pub type Vec3u32 = Vec3<u32>;
pub type Vec3u64 = Vec3<u64>;
pub type Vec3b   = Vec3<bool>;

vec3_type!(Vec3f<float>)
vec3_type!(Vec3f32<f32>)
vec3_type!(Vec3f64<f64>)
vec3_type!(Vec3i<int>)
vec3_type!(Vec3i8<i8>)
vec3_type!(Vec3i16<i16>)
vec3_type!(Vec3i32<i32>)
vec3_type!(Vec3i64<i64>)
vec3_type!(Vec3u<uint>)
vec3_type!(Vec3u8<u8>)
vec3_type!(Vec3u16<u16>)
vec3_type!(Vec3u32<u32>)
vec3_type!(Vec3u64<u64>)
vec3_type!(Vec3b<bool>)

Here is the types in use in rray:

// Intiate the actual shooting of rays and tracing for a given pixel
fn doTrace(s: &Scene, sp: &SceneParams, posn: &Pixel) -> Colour {

    // If antialias is on break the pixel into 4 'sub pixels'
    let subPixels = if sp.antialias {
        ~[Vec2f32::new(posn.x + 0.25, posn.y + 0.25),
          Vec2f32::new(posn.x + 0.25, posn.y + 0.75),
          Vec2f32::new(posn.x + 0.75, posn.y + 0.25),
          Vec2f32::new(posn.x + 0.75, posn.y + 0.75)]
    } else {
        ~[Vec2f32::new(posn.x, posn.y)]
    };

    // Evenly weight the colour contribution of each sub pixel
    let coef = 1.0 / (subPixels.len() as f32);

    do subPixels.foldr(Vec3f32::zero()) |cs, results| {
        let currentPixel = sp.topPixel
                            .add_v(&sp.horVec.mul_t(sp.aspectRatio * cs.x))
                            .add_v(&s.up.mul_t(-cs.y));
        let ray = currentPixel.sub_v(&s.camera);
        let colour = trace(s.primitives, &s.ambient, &ray, &s.camera, s.lights);

        colour.mul_t(coef).add_v(&results)
    }
}

As you can see, the typedefs make the code very clean on the user side.

It was a workaround in the first place, so I shouldn't be too annoyed that it was plugged. I'll probably have to use the macro method used in q3.

@brendanzab
Copy link
Member

@pcwalton Hmm, just to refresh my memory – I know we had a discussion about this last year on IRC, but why can't we do this again?

trait A {
    fn static_method(a: A) -> Self;
    fn method(&self) -> Self;
}

impl A for int {
    fn static_method(a: A) -> int { a + 2 }
    fn method(&self) -> int { *self + 2 }
}

fn tester<T: A>(a: T) {
    let x = a.method();
    let y = T::static_method(a);
}

From a user point of view (without understanding the compiler) it seems that this should work. Accessing the method on a value with a type that implements A works, but accessing a static method on a type that implements A doesn't. Hence you get questions like this popping up every so often.

If that was allowed, then you'd be able to do:

type Vec3f = Vec3<float>;

fn main() {
    let v = Vec3f::from_value(1.0);
}

Notice I could use 1.0, instead of 1.0f. At the moment, you'd get an unconstrained type error.

It would also allow you to do cool things like float::NaN() or i64::max_value().

@pcwalton
Copy link
Contributor Author

@brendanzab
Copy link
Member

@pcwalton Thanks for the write-up. That would be incredibly useful. I think losing use statements in traits is an ok tradeoff.

@brendanzab
Copy link
Member

@pcwalton Might this be possible in the future?

trait Signed {
    fn abs(&self) -> Self;
}

impl Signed for float {
    fn abs(&self) -> float { ... }
}

fn main() {
    let a = float::abs(3.0);
}

@pcwalton pcwalton closed this Apr 30, 2013
@pcwalton
Copy link
Contributor Author

@bjz No, that's the functional-style versus OO-style associated items issue. You will however be able to write impl S = Signed for float; ... S::abs(x);

@brendanzab
Copy link
Member

Oh. forgot about that!

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

Successfully merging this pull request may close these issues.

6 participants