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

RES: prefer trait methods before private inherent methods #6685

Merged
merged 1 commit into from Jan 24, 2021

Conversation

vlad20012
Copy link
Member

@vlad20012 vlad20012 commented Jan 16, 2021

Improves the #5661 fix: now resolve it to visible trait impl instead of private inherent impl:

use foo::{Foo, Trait};

mod foo {
    pub struct Foo;
    impl Foo {
        // Private
        fn get(&self) { println!("struct"); }
    }

    pub trait Trait {
        fn get(&self);
    }
    impl Trait for Foo {
        fn get(&self) { println!("trait"); }
    }    //X
}

fn main() {
    let f = foo::Foo;
    f.get();
    //^
}

c.c. @Kobzol

@vlad20012 vlad20012 added the fix Pull requests that fix some bug(s) label Jan 16, 2021
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better :) Maybe we could keep the old test with

impl Foo {
    pub fn get(&self) { println("struct"); }
}

?

@vlad20012
Copy link
Member Author

@Kobzol Hmm yep, Let's keep it

@vlad20012
Copy link
Member Author

@Kobzol Oh, actually there isn't a multiresolve because inherent impl wins

@Kobzol
Copy link
Member

Kobzol commented Jan 17, 2021

Oh, ok, even better :)

@vlad20012
Copy link
Member Author

@Undin the PR awaiting the regression testing repairs :)

@Undin
Copy link
Member

Undin commented Jan 18, 2021

@vlad20012 as we discussed in person, regression-2 is only about stdlib and yeah, it's broken currently for some reason. All other regression tests work as expected so looks like you don't need to wait

@vlad20012 vlad20012 added this to the v140 milestone Jan 24, 2021
@vlad20012
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jan 24, 2021
6685: RES: prefer trait methods before private inherent methods r=vlad20012 a=vlad20012

Improves the #5661 fix: now resolve it to visible trait impl instead of private inherent impl:

```rust
use foo::{Foo, Trait};

mod foo {
    pub struct Foo;
    impl Foo {
        // Private
        fn get(&self) { println!("struct"); }
    }

    pub trait Trait {
        fn get(&self);
    }
    impl Trait for Foo {
        fn get(&self) { println!("trait"); }
    }    //X
}

fn main() {
    let f = foo::Foo;
    f.get();
    //^
}
```

c.c. @Kobzol 


Co-authored-by: vlad20012 <beskvlad@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jan 24, 2021

Build failed:

@Kobzol
Copy link
Member

Kobzol commented Jan 24, 2021

bors retry

EDIT: Oops, this is not my PR, I automatically retried it, since I got a CI bors notification in e-mail :D

@bors
Copy link
Contributor

bors bot commented Jan 24, 2021

Build succeeded:

@bors bors bot merged commit d65a3af into master Jan 24, 2021
@bors bors bot deleted the res-private-inherent-method branch January 24, 2021 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests that fix some bug(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants