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

Type bounds are ignored in trait and impl function definitions #24011

Closed
fhahn opened this issue Apr 3, 2015 · 9 comments
Closed

Type bounds are ignored in trait and impl function definitions #24011

fhahn opened this issue Apr 3, 2015 · 9 comments
Assignees
Labels
A-type-system Area: Type system P-medium Medium priority

Comments

@fhahn
Copy link
Contributor

fhahn commented Apr 3, 2015

It seems that rustc does not check if the type paramater satisfy the declared type bounds in trait and impl function definitions, but only for bare functions. Rustc fails on the following code with the following, rather confusing, error message: type &mut std::collections::hash::set::HashSet<[u8]> does not implement any method in scope named insert

But I think that rustc should fail at an earlier stage, because [u8] does not implement core::marker::Sized.

use std::collections::HashSet;

struct Bar;

impl Bar {
    fn test(bar: &mut HashSet<[u8]>) {
        let x: [u8; 3] = [1, 2, 3];
        bar.insert(x);
    }
}

trait Foo {
    fn test(&self, _: &mut HashSet<[u8]>);
}

The following code is a more general example and does not depend on any library.

trait Cons {
    fn myFn(&self);
}

struct Foo<T: Cons> {
    field: T
}

trait Kaboom {
    fn foo(&self, x: Foo<u8>) {}
   // this does NOT fail, but Foo<u8> is no valid type
}

struct KaboomStruct;

impl KaboomStruct {
    fn bar(&self, x: Foo<u8>) {}
   // this does NOT fail, but Foo<u8> is no valid type
}

fn foo(x: Foo<u8>) {}
// this fails, beacuse u8 does not implement Cons
@fhahn
Copy link
Contributor Author

fhahn commented Apr 3, 2015

In this commit, I managed to reuse the existing infrastructure to check the bounds of impl function items. I do not really have an in depth understanding of the well formedness checks, but if this approach is reasonable I am pretty sure it is possible to avoid most of the duplicated code.

But I could not get this approach to work for trait items, because it relies on blank_fn_ctxt.

@pnkfelix pnkfelix added the A-type-system Area: Type system label Apr 3, 2015
@pnkfelix
Copy link
Member

pnkfelix commented Apr 3, 2015

AFAICT, there is no unsoundness exposed by these examples, since any attempt to create instances of the types one needs to pass (to invoke the functions that are unchecked here) will fail ... right?

I agree there is a bug here; I just want to double-check the above, in terms of how to triage it.

@fhahn
Copy link
Contributor Author

fhahn commented Apr 3, 2015 via email

@pnkfelix
Copy link
Member

pnkfelix commented Apr 3, 2015

cc @nikomatsakis

@nikomatsakis
Copy link
Contributor

I've noticed this sort of thing before. I can look into it. Probably worth trying to do before 1.0.

@nikomatsakis
Copy link
Contributor

triage: P-high (1.0)

@rust-highfive rust-highfive added the P-medium Medium priority label Apr 4, 2015
@rust-highfive rust-highfive added this to the 1.0 milestone Apr 4, 2015
@nikomatsakis nikomatsakis self-assigned this Apr 4, 2015
@apasel422
Copy link
Contributor

This is related to #19182.

@apasel422
Copy link
Contributor

A similar issue with default type parameters that compiles successfully:

trait Bar {}
struct Foo<T = usize>(T) where T: Bar;

Foo<usize> cannot be constructed, but it doesn't make sense to allow T to be defaulted to usize. It seems that this could be a backwards compatibility issue, if we ever wanted to forbid this nonsensical declaration in the future. The same goes for default type parameters in other kinds of items.

@steveklabnik steveklabnik removed this from the 1.0 milestone May 21, 2015
@arielb1
Copy link
Contributor

arielb1 commented Aug 14, 2015

Fixed by #27641.

@arielb1 arielb1 closed this as completed Aug 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

7 participants