Re: A plan for fixing _fix_metaclass_compatibility

Dave,

I totally approve of this plan, specific comments below:

On Sep 7, 2008, at 2:01 PM, Dave Rolsky wrote:

> I'm discussing this here cause the plan is kinda long, and wouldn't  
> work well in IRC.
>
> First, the problem ...
>
> With Moose::Util::MetaRole, it's quite easy to end up with a  
> metaclass that is not Moose::Meta::Class. Generaly, it's a subclass  
> of MMC with 1+ roles applied to the subclass. Let's call a subclass  
> that differs from its parent only by the roles it does a "role-only  
> subclass".
>
> Right now, Moose does not handle these sorts of role-only  
> subclasses well when trying to determine if two classes have  
> compatible metaclasses. If two classes each have different role- 
> only subclasses as their metaclass, Moose complains that these  
> metaclasses aren't compatible.
>
> Here's a more concrete example:
>
>   ClassA
>      |
>      |
>   ClassB
>
> ClassB inherits from ClassA. Here's the metaclasses:
>
>              Moose::Meta::Class
>              /                 \
>             /                   \
>         MetaSubA              MetaSubB
>      (for ClassA)             (for ClassB)
>
> The two metaclass subclasses are each role-only subclasses (doing  
> the roles RoleA and RoleB respectively).
>
> When we declare ClassB as a subclass of ClassA, Moose dies. What  
> _should_ happen is that Moose detects that the two metaclasses are  
> role-only, _and_ that they share a common ancestor. Then it can  
> simply make a new metaclass for ClassB which does both RoleA and  
> RoleB, as a subclass of MetaSubA.
>
> Poof, now they're compatible again.

Yes, I think this sounds very sane.

> So that's what I'd like to implement. The algorithm goes like this:
>
> 1. Look for a common parent class between the two metaclasses  
> (often this will end up being Moose::Meta::Class). If there is  
> none, there's an incompatibility. If there are multiple common  
> parents, give up, because I don't want to deal with multiple  
> inheritance ;)
>
> 2. For each class in the chain from a metaclass to the common  
> parent, see if that class is a role-only subclass.
>
> 3a. If all of these subclasses are role-only subclasses _in both  
> chains_, we just need to combine all the roles done in both chains,  
> and make a new subclass for said roles.
>
> 3b. If any of the classes in the chains are _not_ role-only  
> subclasses, we have a real incompatibility, and die just like we do  
> now.
>
> The catch is how to determine if a given metaclass is a role-only  
> subclass, and this is where we get to changing Class::MOP and Moose  
> to support this.
>
> The basic algorithm I've thought of to detect a role-only subclass  
> looks like this. Compare all of this class's attributes and methods  
> to its parent. For any that are specific to the class, see if they  
> come from a role.

This seems like possibly expensive, I wonder if we can't maybe add  
some more meta-data to the metaclass that would maybe track this?  
This is possibly a premature optimization, but I suspect it would  
also mean you wouldn't have to do all the stuff described below too :)

> The last part is what's not really possible with today's CMOP &  
> Moose. We have no good way of knowing the source of an attribute or  
> method, and I want to change that.
>
> My plan is the following:
>
> * Whenever we add a method (add_method or alias_method), we need to  
> accept more than just a name and a sub reference. Instead, we can  
> take a parameter like "source_role" (in Moose) or maybe just  
> "source_package" to make it generate and applicable to CMOP.
>
> * We can do the same thing for attributes. Additionally, I'd like  
> to be able to specify an associated_attribute for methods generated  
> from an attribute. This I want to add just cause we can, and it  
> seems like useful info.
>
> There's one potential stumbling block here. Right now, Method  
> objects are not generated until someone asks for one via an  
> introspection method. To make all this work I think we'll need to  
> generate them as part of add_method, or even just accept such an  
> object in add_method. This means we'll be generating these objects  
> during compile/load time, and paying the cost up front.
>
> So what do people think of this plan?
>
> I'd like to start working on it soon, because the metaclass  
> compatibility thing is biting me in actual code, and working around  
> it is kind of a pain, plus Moose should just get this stuff right  
> anyway.
>
>
> -dave
>
> /*==========================
> VegGuide.Org
> Your guide to all that's veg
> ==========================*/