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
- 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