On Thu, Sep 20, 2012 at 7:37 PM, Levente Uzonyi <leves at elte.hu> 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
> - 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.


I don't understand.  How else can one use Undeclared?  Any binding in a
method which is undefined should be in Undefined.  Hence when a class is
removed or when a class is renamed the binding should be moved to
Undeclared and left there, but when a class is renamed we can offer the
user the opportunity to redefine methods that refer to the class with the
old name (edit the methods to change the name to the new name).

One might think that on rename the binding can be reused and its key
changed.  But this would leave the methods referring to the new class but
with their source naming the old name, hence on recompile the methods would
now refer to the binding ... in Undeclared.  So the only correct thing for
me is to move the binding to Undeclared, and, if interactive, offer up
either a list browser of all references to the class, or an option to edit
those methods automatically.


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


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20120921/f1380908/attachment.htm