Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Handle supertrait calls in default methods #4537

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

catamorphism commented Jan 19, 2013

r? @graydon Add a new method_super origin for supertrait methods. Also make
coherence create a table that maps pairs of trait IDs and self types
to impl IDs, so that it's possible to check a supertrait method
knowing only its index in its trait's methods (without knowing all
supertraits for a given trait).

As per #3979

Contributor

catamorphism commented Jan 19, 2013

n.b. This only handles supertrait method calls on self; it doesn't handle the case of passing self to a method that expects it to have a supertrait bound. I'll file a separate pull request for the latter case.

catamorphism added some commits Jan 17, 2013

@catamorphism catamorphism Handle supertrait calls in default methods
Add a new method_super origin for supertrait methods. Also make
coherence create a table that maps pairs of trait IDs and self types
to impl IDs, so that it's possible to check a supertrait method
knowing only its index in its trait's methods (without knowing all
supertraits for a given trait).

As per #3979
d78265a
@catamorphism catamorphism xfail transitive inheritance test -- I guess this doesn't work yet 5f123cc
Contributor

graydon commented Jan 22, 2013

r+ from a style and direction perspective (and it looks great); unfortunately it also looks Serious enough that I'm not real comfortable pretending to be able to judge semantic correctness. Maybe get someone who knows the numerous related invariants to double-check?

Contributor

catamorphism commented Jan 22, 2013

@nikomatsakis - Can you double-check?

Contributor

nikomatsakis commented Jan 23, 2013

@catamorphism ok will take a look today

Nit: I find this style of densely packing in comments and code really hard to read. This is a personal thing so do as you feel, but I would prefer this code to be formatted as so:

// <self_ty> is the self type for this method call
let self_ty = node_id_type(bcx, self.id);
let tcx = bcx.tcx();

// <impl_id> is the ID of the implementation of
// trait <trait_id> for type <self_ty>
let impl_id = ...;

This test should actually run something, or else be removed. It seem like it's trying to test the logic for the case where you have a call to a method on the supertype or a supertype?

It seems like this should be taking type parameter substitutions into account.

NIT: Can't you use a single tuple or something? The match below is somewhat awkward, and these variables are never set individually.

Later on, don't we expect that the combined type parameters of the method are receiver + method? It seems like here you are always using the tps from the self_ty, but in fact you want the appropriate tps for the supertype.

Contributor

catamorphism commented Jan 30, 2013

Merged in 90cee95 -- see #4678 for future work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment