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

Design for work around trait lifetime issue #559

Closed
bvssvni opened this issue Jul 2, 2014 · 1 comment
Closed

Design for work around trait lifetime issue #559

bvssvni opened this issue Jul 2, 2014 · 1 comment

Comments

@bvssvni
Copy link
Member

bvssvni commented Jul 2, 2014

rust-lang/rust#15191 (comment)

To fix this issue and make Rust-Graphics work with Rust nightlies, the Field is redesigned.

  • This introduces an assumption that all fields are Copy kind
  • The emitted assembly seems to be equivalent Check if compiler optimizes fields by value #555
  • Relies on the compiler to optimize sequential copy-by-value instead of reference to stack value
  • The external look-and-feel of the API remains unchanged
  • No extra generic constraints are required on the impls
  • Can be changed back when Rust solves the issue above - within one day of work
  • Side effect is that let a = b.foo(...).bar(...); is allowed. Will not break future API to change back if the better temporary lifetime RFC is implemented RFC: Better temporary lifetimes (so e.g. .as_slice() works) rust-lang/rfcs#66

The old Field:

pub enum Field<'a, T> {
    Value(T),
    Borrowed(&'a T),
}

impl<'a, T> Field<'a, T> {
    #[inline(always)]
    pub fn get(&'a self) -> &'a T {
        match *self {
            Value(val) => val,
        }
    }
}

The new Field:

pub enum Field<T> {
    Value(T),
}

impl<T: Copy> Field<T> {
    #[inline(always)]
    pub fn get(&self) -> T {
        match *self {
            Value(val) => val,
        }
    }
}
@TyOverby
Copy link
Contributor

TyOverby commented Jul 2, 2014

You know, I actually like this one better. Any chances of keeping this by-copy version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants