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

Prohibit unused type parameters in impls. #447

Merged
merged 2 commits into from
Jan 5, 2015

Conversation

nikomatsakis
Copy link
Contributor

For well-defined semantics, we must ensure that impls do not have type parameters that do not appear within the trait or self-type.

Rendered view.

@nikomatsakis nikomatsakis self-assigned this Nov 6, 2014
@Thiez
Copy link

Thiez commented Nov 6, 2014

Would this change forbid any code that is currently in the Rust repo (either in the compiler or in the stdlib)?

@sfackler
Copy link
Member

sfackler commented Nov 6, 2014

cc rust-lang/rust#18549

@nikomatsakis
Copy link
Contributor Author

On Thu, Nov 06, 2014 at 08:49:12AM -0800, Thiez wrote:

Would this change forbid any code that is currently in the Rust repo (either in the compiler or in the stdlib)?

I have not investigated.

@pythonesque
Copy link
Contributor

Does this apply to lifetime parameters? If so, this might prevent certain patterns: e.g. 'a: 'b can only go in one direction, so if both type parameters must be bound to the struct this prevents currently safe code that relies on a non-struct-attached 'b (but maybe this is not useful in practice).

@nikomatsakis
Copy link
Contributor Author

On Mon, Nov 10, 2014 at 06:55:47AM -0800, Joshua Yanovski wrote:

Does this apply to lifetime parameters?

I imagine so, but it's less crucial.

If so, this might prevent certain patterns: e.g. 'a: 'b can
only go in one direction, so if both type parameters must be bound
to the struct this prevents currently safe code that relies on a
non-struct-attached 'b (but maybe this is not useful in
practice).

I think such code can always be rewritten in a similar fashion to the
examples from the RFC for type parameters, I'd be curious to see an
example.

@pythonesque
Copy link
Contributor

The example I was thinking of was something like:

struct Foo<'a> { foo: &'a () }

impl<'a: 'b, 'b> Foo<'a> {
    fn bar(&'a self, foo: &'b ()) { /* ... */ }
}

fn main() {}

The example is contrived but I'm not sure you can write it anymore (and I think I've written something similar at least once in the past where it wasn't contrived). I don't think you get the same problem with types because there's no analog for the outlives relation.

@nikomatsakis
Copy link
Contributor Author

On Tue, Nov 11, 2014 at 06:06:34AM -0800, Joshua Yanovski wrote:

The example I was thinking of was something like:

struct Foo<'a> { foo: &'a () }

impl<'a: 'b, 'b> Foo<'a> {
    fn bar(&'b mut self, foo: &'a ()) { /* ... */ }
}

fn main() {}

The example is contrived but I'm not sure you can write it anymore (and I think I've written something similar at least once in the past where it wasn't contrived).

This can be rewritten most explicitly as follows:

struct Foo<'a> { foo: &'a () }

impl<'a> Foo<'a> {
    fn bar<'b>(&'b mut self, foo: &'a ()) where 'a : 'b { /* ... */ }
}

fn main() {}

This rewrite requires full support for where clauses, which are not
yet fully implemented (soon, though).

That said, the where 'a : 'b is actually implied by the type of
self, which is &'b Foo<'a> -- a type like this always requires
that 'a outlives 'b (because no reference may outlive its
referent).

So in fact you could just drop the where clause and everything would
be fine.

@pythonesque
Copy link
Contributor

Sure, I realized that for the initial example it was implied (which is why I hastily changed it!), I just meant there were situations where you couldn't write it. But if where clause syntax will support it, then I have no misgivings :)

@nikomatsakis
Copy link
Contributor Author

@arielb1 showed me a nice example of where we can encounter problems today, even without requiring the cross-crate scenario that was concerning me: http://is.gd/hZSQ2N

@nikomatsakis
Copy link
Contributor Author

During a conversation in IRC, @huonw raised an example that demonstrates that, with associated types, some type parameters that would appear unconstrained are in fact constrained:

impl<T, X: Deref<Target=[T]>> Strided for X

Here T is deterministically derived from X. So the algorithm needs to take that into account.

@aturon
Copy link
Member

aturon commented Jan 5, 2015

This RFC has been accepted, as per the stated motivations. Lifetime parameters are not affected, however.

Tracking issue

@withoutboats
Copy link
Contributor

Why is this not permitted under this RFC?

trait Foo<T> { }
trait Bar { }

impl<T: Foo<U>, U> Bar for T { }

@nikomatsakis
Copy link
Contributor Author

@withoutboats because the type of U is unconstrained. The way to think about is like this:

Imagine that somebody just gave you the trait reference. In this case, that might be i32: Bar and asked you to find the impl. Would you have enough information to decide what U is?

Within a given crate, you might infer that based on what impls exist -- but then imagine that other crates come along and add more impls. Now your inference is ambiguous.

This is exactly the scenario in which the compiler finds itself, particularly with specialization at play.

@withoutboats
Copy link
Contributor

Thanks, that makes sense.

bors added a commit to rust-lang/rust that referenced this pull request Jul 31, 2016
typeck: use a TypeVisitor in ctp

Use a TypeVisitor in ctp instead of `ty::walk`

This fixes a few cases where a region could be projected out of a trait while not being constrained by the type parameters, violating rust-lang/rfcs#447 and breaking soundness. As such, this is a [breaking-change].

Fixes #35139

r? @eddyb
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

Successfully merging this pull request may close these issues.

6 participants