[Cuis-dev] Removing unused temps from inside blocks

Fri Dec 13 04:50:56 PST 2019

Oh, I answered the previous mail without seeing this one... I'll take a
look at it, but also if you can take a look at what I mentioned in the
previous email it would be great.

Hernan.

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

> Here's the ChangeSet that goes through every BlockNode checking if its
> temporaries are unused, which fixes the issue of multiple temporaries with
> the same name in one method. I had to edit the main parsing method so as to
> add the temporaries to the BlockNode that represents the body. Given that
> every other BlockNode has its temporaries, it seemed logical that the body
> BlockNode should have them too.
>
>
>
> El jue., 12 dic. 2019 a las 20:04, Eric Brandwein (<
> brandweineric at gmail.com>) escribió:
>
>> 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. For
>> example, when saving this method:
>>
>> test
>>     [ | var | ].
>>     [ | var | var := 'var'. ^var ].
>>
>> It doesn't prompt to remove the variable 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.
>>
>> 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.
>>
>> 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/f28d3a53/attachment-0001.htm>