refactor how impl self type vs iface type is stored (at least for classes) #2434

Closed
nikomatsakis opened this Issue May 24, 2012 · 1 comment

Comments

Projects
None yet
2 participants
@nikomatsakis
Contributor

nikomatsakis commented May 24, 2012

Near as I can tell, this is how things stand now:

  • Both impls and classes make use of an iface_ref, which has an id, a path and so forth
  • The type that the iface_ref refers to is stored in the node_types table under the id of the iface_ref
  • (So far this all makes sense)

But this is where things go off the rails.

  • For impls, the self type is stored in the tcache under the item id.
  • For classes, the self type seems to be stored in the tcache under the iface_ref's id.

(This is based on the "impl_id" that is stored in resolve.)

The idea of encapsulating an iface+impl pair into an iface-ref seems to make sense, but we probably want to either use separate tables to store the self type (i.e., not the tcache) or else use a separate id. Right now classes end up with different types in the tcache table and node_types table under the same ID. This seems unusual and I think it is the only place that it occurs.

Also, impls + classes should be as similar as possible. I think this was the original idea of introducing iface-ref, but there still seem to be some significant differences in how things work between the two. So we should erase that.

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism May 24, 2012

Contributor

Any differences between impls and classes that were introduced by me were for expedience (as I saw it at the time), not by design, so I definitely support simplifying the code in any way possible. (I'm on vacation until Tuesday, so, whoever gets to it first...)

Contributor

catamorphism commented May 24, 2012

Any differences between impls and classes that were introduced by me were for expedience (as I saw it at the time), not by design, so I definitely support simplifying the code in any way possible. (I'm on vacation until Tuesday, so, whoever gets to it first...)

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