Eric M. Ludlam writes:
> On 04/10/2012 05:32 AM, David Engster wrote:
>> Eric M. Ludlam writes:
>>> I don't know why this behavior changed, but it was definitely with
>>> invocation order. Fortunately, support for methods not bound to any
>
>>> specific class was added a while back, so I checked in a change that
>>> removed all the methods on eieio-default-superclass with methods with no
>>> class. That prevents the invocation order from getting in the way.
>>>
>>> I also found some inconsistencies with re-evaluating classes at runtime
>>> that made debugging a bit of a challenge.
>>
>> Funny thing is that I cannot reproduce the original bug with newtrunk,
>> which is... good, I guess? I'm a bit wary that there might be a bug
>> regarding method invocation; those guys are terribly hard to pin
>> down. But if newtrunk works without your latest patch, I guess I'll
>> revert that when I do the final merge?
>
> Hmmm. Good question.
>
> The problem with method invocation order is that the first primary
> method you encounter, if it doesn't call (call-next-method) prevents any
> later method from running. That's important if you want to really
> override and not just augment a method.
>
> In eieio-speedbar, it defined a bunch of methods on
> eieio-default-superclass as a way of allowing subclasses to only
> override what they really need to, and providing default behavior for
> objects that do not inherit from eieio-speedbar. Thus, if you have a
> speedbar object that refers to non-speedbar objects that should be
> displayed in speedbar, things still work.
>
> Due to multiple inheritance, if the method invocation travels down some
> branch and bumps into the default superclass, things stop w/out looking
> at methods on some other branch of the tree. That's why EDE defines the
> method invocation order to force traversal down the first branch, toward
> these eieio speedbar methods. Except ede-proj.el had the multiple
> inheritance list backward. Ok, so I thought that was an easy fix,
> except it didn't fix anything. Very strange.
Oh, I see. This problem is a regression due to your expanded EDE
security fix, where you added eieio-persistent to the superclasses of
ede-proj-project. I did not merge that one into newtrunk, which explains
why it works there. That's a relief, actually. I was worried that there
were some problems due to the changes Stefan did in the lexbind-branch.
Indeed, the classes are the wrong way round there. If you just switch
them, it will still not work because you did not change the
method-invocation-order to depth-first. While you defined that in the
ede-project-placeholder class, this has no effect here since EIEIO
cannot know that until it has reached that superclass -- at least that's
how I understand it. Actually, my rule of thumb is: when you start to
worry about method invocation order, this is a sure sign that you should
start to refactor your code. ;-) Therefore, I think your fix is indeed
The Right Thing.
On a more general note, this method-invocation-order thingy is really
asking for trouble; what happens if you mix breadth/depth-first in your
line of inheritance? I know, "Don't do that, then", but still: We should
at least document *where* this has to specified (basically in every
class from which on the invocation order matters?).
-David
|