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

warn_self_convention and copy? #273

Closed
wackywendell opened this issue Sep 1, 2015 · 4 comments
Closed

warn_self_convention and copy? #273

wackywendell opened this issue Sep 1, 2015 · 4 comments

Comments

@wackywendell
Copy link

I recently got a warn_self_convention lint on a type that implements Copy, something like this:

#[derive(Copy)]
pub struct Color(pub u8, pub u8, pub u8);

impl Color {
    /// Convert to a 3-tuple, from 0 to 1
    pub fn as_floats(self) -> (f32, f32, f32) {
        let Color(r,g,b) = self;
        (
            (r as f32) / (u8::max_value() as f32),
            (g as f32) / (u8::max_value() as f32),
            (b as f32) / (u8::max_value() as f32),
        )
    }
src/palette.rs:43:22: 43:30 warning: methods called `as_*` usually take self by reference; consider choosing a less ambiguous name, #[warn(wrong_self_convention)] on by default
src/palette.rs:43     pub fn as_floats(self) -> (f32, f32, f32) {

It seems to me that types that implement Copy should be permitted to use pass-by-value instead of pass-by-reference for as_, to_, and other such methods.

@birkenfeld
Copy link
Contributor

Agree. I'll see how easy it is to fix this.

@llogiq
Copy link
Contributor

llogiq commented Sep 1, 2015

Good catch! It should be possible to check if the Self type binds to Copy by checking the trait bounds.

@birkenfeld
Copy link
Contributor

Please check if it works with this PR applied.

@wackywendell
Copy link
Author

Yes, it does! Nice work!

Manishearth added a commit that referenced this issue Sep 1, 2015
methods: try to allow value self when type is Copy (fixes #273)
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

3 participants