On Fri, 21 Sep 2012, Levente Uzonyi wrote:

> Thu, 20 Sep 2012, Eliot Miranda wrote:
>
>> 
>> 
>> On Thu, Sep 20, 2012 at 1:45 PM, Levente Uzonyi <leves at elte.hu> wrote:
>>       On Thu, 20 Sep 2012, Colin Putney wrote:
>>
>>             On Thu, Sep 20, 2012 at 11:28 AM, Levente Uzonyi 
>> <leves at elte.hu> wrote:
>>
>>                   In order to fix this bug, I came up with two ideas:
>>                   1) Recompile all methods which point to the old class 
>> when the class is
>>                   removed. This helps, because the class will be undeclared 
>> at that point, so
>>                   a new binding will be stored in Undeclared and the same 
>> binding will be used
>>                   when the class is added again. The drawback of this 
>> solution is that it only
>>                   fixes references in methods and it's slow, since it has 
>> to scan all methods
>>                   in the system.
>> 
>>
>>             Couldn't we just move the existing binding to Undeclared? That 
>> might
>>             be unnecessary if there are no references, but I don't think it 
>> would
>>             hurt.
>> 
>> 
>> I tried that, but ran into another issue related to Undeclared. Class >> 
>> #rename: checks if the new class name is included in Undeclared. And if it 
>> is, it'll #inform: about it. The problem is that Undeclared holds a strong
>> reference to its bindings, so if we remove a class which has no references 
>> and we move it to Undeclared, then it will linger there, until someone 
>> calls #cleanOutUndeclared. This is the weakness I described in solution #2.
>> 
>> To work around this we can remove the unreferenced keys from Undeclared in 
>> Class >> #rename:, but it means, that all methods in the system will be 
>> scanned for each renamed class.
>> 
>> 
>> That's a price well worth paying. Traversing the class hierarchy to fid 
>> methods is quite quick (much faster than allInstances) and class rename is 
>> rare (i.e. a devlopment time activity, not a typical runtime activity).  So 
>> IMO this
>> is the way to go.
>
> I guess I wasn't clear, because #allInstances is not used by the solution I 
> suggested. It does two things:
> When a class is removed, then
> 1) traverse the class hierarchy and search for methods which refer to the 
> binding
> 2) recompile the methods found in step 1
>
> The alternative is to - nil out the value of the binding and move it to 
> Undeclared when the class is removed

There's a test which expects that a removed class's method's class is not 
nil, so setting the value of the binding to nil doesn't work.


Levente

> - when Undeclared is accessed from Class >> #rename: (and who knows which 
> other methods) remove the unreferenced keys from it by traversing the class 
> hierarchy searching for methods that refer to the binding. If no such method 
> was found then the binding is removed from Undeclared.
>
> The reason why I don't like the alternative is that we don't know if there 
> are other methods which use Undeclared in a similar way and therefore must be 
> rewritten. The only reason I know about Class >> #rename: is because a test 
> was failing when I moved the binding to Undeclared. It might not be that hard 
> to find the methods, but it's still extra work to be done. I'll check it out 
> though, because class renaming is definitely more rare than class removal.
>
>
> Levente