[Cuis-dev] Removing unused temps from inside blocks

Fri Dec 13 04:49:38 PST 2019

On Thu, Dec 12, 2019 at 8:04 PM Eric Brandwein <brandweineric at gmail.com>
wrote:

> Great! It works almost perfectly. The only bug that still stands is when
> there's two blocks in the same method with the same variable declared.
>

hehe it is great the you found it! I knew about it, there is the same
problem when renaming temps, and to solve it we need to compile the method
to get the assignment of variable per closure (something Eliot did and that
sadly it is coupled to the bytecode generation)

> For example, when saving this method:
>
> test
>     [ | var | ].
>     [ | var | var := 'var'. ^var ].
>
> It doesn't prompt to remove the variable var.
>

Not only that. If none of them are used, it will remove the last var...

> This is because the scopeTable of the Encoder class just saves the last
> VariableNode when there are many with the same name. The same bug affects
> the RenameTemporary refactoring. In fact, this example shows that
> referencing a variable just by its name inside a method is not enough; one
> needs the block where it's declared, too.
>

Yeap!

>
> Given this, maybe the keys of the Encoder's scopeTable should not be the
> names of the variables, but instead objects with the name of the variable
> and the information of the block where it is declared for each one. Does
> this sound good? A couple of methods should be changed for this to work;
> all the Encoder#rangesFor..., for instance.
>
> Another possible solution for this case in particular would be to look
> BlockNode by BlockNode for unused temp names, and querying the positions
> for the VariableNodes found instead of for the names of the temporaries.
> This doesn't solve the bug in RenameTemporary though.
>
>
Look at the method of the category "Debugger Support" of CompiledMethod.
The answer is there (not directly), but those messages and others are used
to get a dictionary that maps each var to the right block. The debugger
used it to solve the same issue we have. I did not think how to use this to
solve the whole problem but maybe you can :-), that would be great!

Cheers!
Hernan.

> Let  me know what you think,
> Eric
>
> El jue., 12 dic. 2019 a las 17:47, Hernan Wilkinson (<
> hernan.wilkinson at 10pines.com>) escribió:
>
>> Did it!! and also it removes the | if no more temp var is defined :-)
>>
>> At github now.
>>
>> Cheers!
>> Hernan
>>
>> On Thu, Dec 12, 2019 at 4:39 PM Hernan Wilkinson <
>> hernan.wilkinson at 10pines.com> wrote:
>>
>>> I think I know how to fix it... instead of removing each variable, we
>>> should collect the ranges to remove and after going through all the temps
>>> remove them. That way there is no need to re parse... I'll give it a try
>>>
>>> On Thu, Dec 12, 2019 at 4:34 PM Hernan Wilkinson <
>>> hernan.wilkinson at 10pines.com> wrote:
>>>
>>>> Hi Eric,
>>>>  thank you for the fix! It is in github now.
>>>>
>>>>  Re-parsing is not a problem because methods are usually small in
>>>> Smalltalk and the parser is fast enough, but it introduces an issue: when
>>>> you say you don't want to remove the variable and then you remove another
>>>> one, it will ask you again for the first one.
>>>>  I integrated the change anyway because it is a rare condition and it
>>>> is better to remove unused temps on blocks.
>>>>  If you can fix that new issue, the better, but I think it will not be
>>>> so simple...
>>>>
>>>>  BTW, I also fixed another problem, it there was no temps in the method
>>>> but unused temps in a block, it did not work.
>>>>
>>>> Thanks!
>>>> Hernan.
>>>>
>>>> On Thu, Dec 12, 2019 at 3:03 AM Eric Brandwein via Cuis-dev <
>>>> cuis-dev at lists.cuis.st> wrote:
>>>>
>>>>> Sorry, I just realised this doesn't work if you're erasing many
>>>>> variables at a time. The obvious solution is to just reparse each time a
>>>>> temporary variable is removed, and that is what this new attached ChangeSet
>>>>> does.
>>>>>
>>>>> This is maybe slow, and I guess the alternative would be to go through
>>>>> all the declarations of all blocks inside the parsed method from the bottom
>>>>> to the top, so as to not change the positions of other possibly unused
>>>>> declarations when removing one of them. Let me know if this other solution
>>>>> seems better or if the one that reparses every time is fine.
>>>>>
>>>>> Cheers,
>>>>> Eric
>>>>>
>>>>> El jue., 12 dic. 2019 a las 2:23, Eric Brandwein (<
>>>>> brandweineric at gmail.com>) escribió:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Currently, when you save a method with an unused temporary variable
>>>>>> declared, it prompts you to decide if you want to remove it or not. If you
>>>>>> click yes, it removes the declaration, but only if it's declared in the
>>>>>> method scope, and not if it is declared inside a block.
>>>>>>
>>>>>> To reproduce, just create a method with an unused temporary variable
>>>>>> declared inside a block, save it, and click 'Yes' on the popup that appears.
>>>>>>
>>>>>> This is because the Parser only looks for the variable in the temps
>>>>>> declared in the method scope. Here's a ChangeSet that fixes it making the
>>>>>> Parser look inside blocks too, taking advantage of the fact that the
>>>>>> Encoder has already found the positions for each variable.
>>>>>>
>>>>>> Cheers,
>>>>>> Eric
>>>>>>
>>>>> --
>>>>> Cuis-dev mailing list
>>>>> Cuis-dev at lists.cuis.st
>>>>> https://lists.cuis.st/mailman/listinfo/cuis-dev
>>>>>
>>>>
>>>>
>>>> --
>>>>
>>>> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
>>>> *Phone: +54-011*-4893-2057
>>>> *Twitter: @HernanWilkinson*
>>>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>>>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>>>
>>>
>>>
>>> --
>>>
>>> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
>>> *Phone: +54-011*-4893-2057
>>> *Twitter: @HernanWilkinson*
>>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>>
>>
>>
>> --
>>
>> *Hernán WilkinsonAgile Software Development, Teaching & Coaching*
>> *Phone: +54-011*-4893-2057
>> *Twitter: @HernanWilkinson*
>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>
>

-- 

*Hernán WilkinsonAgile Software Development, Teaching & Coaching*
*Phone: +54-011*-4893-2057
*Twitter: @HernanWilkinson*
*site: http://www.10Pines.com <http://www.10pines.com/>*
Address: Alem 896, Floor 6, Buenos Aires, Argentina
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20191213/69a966bc/attachment.htm>