Skip to content

rustc doesn't reject incompatible trait and impl methods w.r.t argument bounds #13629

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
edwardw opened this issue Apr 19, 2014 · 12 comments
Closed
Milestone

Comments

@edwardw
Copy link
Contributor

edwardw commented Apr 19, 2014

The following is extracted from libnative / libgreen implementation of std::rt::Runtime:

pub trait Runtime {
    fn spawn_sibling(&self, f: proc():Send);
}

pub struct Foo;
impl Runtime for Foo {
    fn spawn_sibling(&self, _f: proc()) {}
}

fn main() {}

The implementation misses the Send bound of one of its argument so it should not compile, but currently it does.

@alexcrichton
Copy link
Member

Oh dear, that's bad. Nominating.

@edwardw
Copy link
Contributor Author

edwardw commented Apr 19, 2014

I found this when fiddling with variance in order to fix #12223 and #13497. They seem to be all related.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 19, 2014
These were mistakenly not updated as part of the removal of the Send bound by
default on procedures.

cc rust-lang#13629
@edwardw edwardw changed the title rustc doesn't detect incompatible trait and impl methods w.r.t argument bounds rustc doesn't reject incompatible trait and impl methods w.r.t argument bounds Apr 20, 2014
bors added a commit that referenced this issue Apr 22, 2014
These were mistakenly not updated as part of the removal of the Send bound by
default on procedures.

cc #13629
@brson brson added this to the 1.0 milestone Apr 25, 2014
@brson
Copy link
Contributor

brson commented Apr 25, 2014

1.0 backcompat lang.

@edwardw
Copy link
Contributor Author

edwardw commented May 7, 2014

Consider the example given above and the one below:

pub trait T1 {}
pub struct S;
impl T1 for S {}

pub trait Runtime {
    fn spawn_sibling(&self) -> Box<T1:Send>;
}

pub struct Foo;
impl Runtime for Foo {
    fn spawn_sibling(&self) -> Box<T1> {
        box S as Box<T1>
    }
}

fn main() {}

Intuitively, they are both wrong. But currently rustc accepts the former and rejects the latter. The reasoning here seems to be: in the first example, f is at a contra-variant position, in order to be compatible, the argument being used in the implementation (proc()) must be contra-variant to the one being used in the trait definition (proc():Send), i.e. proc():Send <: proc(), which happens to be true. In the second example, the type in question is at a co-variant position, but Box<T1> <: Box<T:Send> is not true, thus an error.

So by the usual variance rule, the current behavior is actually correct! Very counter-intuitive. I'm not sure why built-in bounds here makes an exception to the variance rule.

@nikomatsakis
Copy link
Contributor

cc me

@nikomatsakis
Copy link
Contributor

I am not sure yet that this is broken. I think it is expected behavior.

@nikomatsakis
Copy link
Contributor

Specifically, impls can require less than traits require. The idea is: if I didn't know the impls type, and I only know the trait type (as in a generic fn), I will pass in a proc():Send. The impl will then be invoked. Since a proc:Send <: proc, this is ok. In other words, the impl is ok with any proc, not just a sendable one.

@nikomatsakis
Copy link
Contributor

That said, I'm not sure there is much purpose to permitting this kind of subtyping, and it might make sense to tighten up the rules just so people don't get surprised. The original goal was to permit impls to act as a kind of refinement, so that in cases where we did know the static type, we could use the tighter types from the impl, rather than the more generic types from the trait, but I don't think this is how our method resolution will work anyway. And you could always declare an inherent method for places where this matters.

@flaper87
Copy link
Contributor

flaper87 commented May 9, 2014

cc @flaper87

@edwardw
Copy link
Contributor Author

edwardw commented May 10, 2014

Also, the "Taming the Wildcards" paper does mention that a type parameter lower bound is contra-variant in itself. The way we use built-in bounds here, i.e. at least Send, makes it lower bound, I presume. If such an interpretation is correct, then the first example will become invalid while the second valid.

Either way, this is rather subtle and could be a surprise.

@pcwalton
Copy link
Contributor

Nominating for closure as a dupe of #2687.

@brson
Copy link
Contributor

brson commented Jun 19, 2014

Closing as expected behavior.

@brson brson closed this as completed Jun 19, 2014
Manishearth pushed a commit to Manishearth/rust that referenced this issue Nov 23, 2022
… r=jonas-schievink

feat: Make "Remove dbg!()" assist work on selections

Fixes rust-lang/rust-analyzer#12114
flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 7, 2024
…ip1995

Return iterator must not capture lifetimes in Rust 2024

In Rust 2024, by default lifetimes will be captured which does not reflect the reality since we return an iterator of `DefId` which do not capture the input parameters.

changelog: none
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

6 participants