<div dir="ltr">Hi Eric,<div> I integrated your changes with a small modification for the TemporaryToInstanceVariable refactor to keep working.</div><div> I also added tests for unused variables.</div><div> </div><div> Now removing unused variables works correctly on block and removes temps. definition when no more is left.</div><div><br></div><div>Cheers!</div><div>Hernan</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Dec 13, 2019 at 5:08 PM Eric Brandwein <<a href="mailto:brandweineric@gmail.com">brandweineric@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Hernán, I checked out the methods in the category "Debugger Support", but I just found ways of obtaining which are the temporary variables and their indexes given a program counter and a context, and not of determining the positions of those variables. There is another way, though: we could traverse the ParseNode tree searching for the BlockNode whose code ranges (obtained with Encoder>>#rangeForNode:ifAbsent:) contain the ranges of the variable we want to rename and also declares it, and then traverse the subtree of that BlockNode searching for the usages of that temporary. I'll write a class that implements this and send it to this thread.</div><div><br></div><div>Another thing I wanted to say is that if my last ChangeSet is included in the image, we should change the TemporaryToInstanceVariable refactor too, because it assumes that the body of the MethodNode hasn't got the temporaries set, and so when one wants to transform a temporary declared in the method temporaries it says that there is already another block declaring the same temporary. I'll write a fix for that too.</div><div><br></div><div>Cheers,</div><div>Eric<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">El vie., 13 dic. 2019 a las 9:51, Hernan Wilkinson (<<a href="mailto:hernan.wilkinson@10pines.com" target="_blank">hernan.wilkinson@10pines.com</a>>) escribió:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">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.<div><br></div><div>Hernan.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 12, 2019 at 11:07 PM Eric Brandwein <<a href="mailto:brandweineric@gmail.com" target="_blank">brandweineric@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>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.</div><div><br></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">El jue., 12 dic. 2019 a las 20:04, Eric Brandwein (<<a href="mailto:brandweineric@gmail.com" target="_blank">brandweineric@gmail.com</a>>) escribió:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>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:</div><div><br></div><div>test</div><div> [ | var | ].</div><div><div> [ | var | var := 'var'. ^var ].</div><div><br></div><div>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. <br></div><div><br></div><div>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.<br></div><div><br></div><div>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.</div><div><br></div><div>Let me know what you think,</div><div>Eric<br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">El jue., 12 dic. 2019 a las 17:47, Hernan Wilkinson (<<a href="mailto:hernan.wilkinson@10pines.com" target="_blank">hernan.wilkinson@10pines.com</a>>) escribió:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Did it!! and also it removes the | if no more temp var is defined :-)<div><br></div><div>At github now.</div><div><br></div><div>Cheers!</div><div>Hernan</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 12, 2019 at 4:39 PM Hernan Wilkinson <<a href="mailto:hernan.wilkinson@10pines.com" target="_blank">hernan.wilkinson@10pines.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">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<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 12, 2019 at 4:34 PM Hernan Wilkinson <<a href="mailto:hernan.wilkinson@10pines.com" target="_blank">hernan.wilkinson@10pines.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Eric,<div> thank you for the fix! It is in github now.</div><div><br></div><div> 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.</div><div> I integrated the change anyway because it is a rare condition and it is better to remove unused temps on blocks.</div><div> If you can fix that new issue, the better, but I think it will not be so simple...</div><div><br></div><div> BTW, I also fixed another problem, it there was no temps in the method but unused temps in a block, it did not work.</div><div><br></div><div>Thanks!</div><div>Hernan.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 12, 2019 at 3:03 AM Eric Brandwein via Cuis-dev <<a href="mailto:cuis-dev@lists.cuis.st" target="_blank">cuis-dev@lists.cuis.st</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>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. <br></div><div><br></div><div>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.</div><div><br></div><div>Cheers,</div><div>Eric </div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">El jue., 12 dic. 2019 a las 2:23, Eric Brandwein (<<a href="mailto:brandweineric@gmail.com" target="_blank">brandweineric@gmail.com</a>>) escribió:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Hi all,</div><div><br></div><div>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.<br></div><div><br></div><div>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.<br></div><div><br></div><div>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.</div><div><br></div><div>Cheers,</div><div>Eric<br></div></div>
</blockquote></div>
-- <br>
Cuis-dev mailing list<br>
<a href="mailto:Cuis-dev@lists.cuis.st" target="_blank">Cuis-dev@lists.cuis.st</a><br>
<a href="https://lists.cuis.st/mailman/listinfo/cuis-dev" rel="noreferrer" target="_blank">https://lists.cuis.st/mailman/listinfo/cuis-dev</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal"><span style="font-weight:bold">Hernán Wilkinson</span><br>Agile Software Development, Teaching & Coaching</span></font></span></span></span></strong></span></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">Phone: +54-011</span></font></span></span></span></strong></span><font size="2" face="tahoma, sans-serif">-4893-2057</font></div><div><strong style="font-family:tahoma,sans-serif;font-size:xx-small"><span style="font-size:8pt"><span style="font-size:small"><font size="2"><span style="font-weight:normal">Twitter: @HernanWilkinson</span></font></span></span></strong></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">site: <a href="http://www.10pines.com/" style="color:rgb(17,65,112)" target="_blank">http://www.10Pines.com</a></span></font></span></span></span></strong></span></div><div><font face="tahoma, sans-serif"><span style="border-collapse:collapse">Address: Alem 896</span></font>, Floor 6, Buenos Aires, Argentina</div></div></div></div></div></div></div></div></div></div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal"><span style="font-weight:bold">Hernán Wilkinson</span><br>Agile Software Development, Teaching & Coaching</span></font></span></span></span></strong></span></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">Phone: +54-011</span></font></span></span></span></strong></span><font size="2" face="tahoma, sans-serif">-4893-2057</font></div><div><strong style="font-family:tahoma,sans-serif;font-size:xx-small"><span style="font-size:8pt"><span style="font-size:small"><font size="2"><span style="font-weight:normal">Twitter: @HernanWilkinson</span></font></span></span></strong></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">site: <a href="http://www.10pines.com/" style="color:rgb(17,65,112)" target="_blank">http://www.10Pines.com</a></span></font></span></span></span></strong></span></div><div><font face="tahoma, sans-serif"><span style="border-collapse:collapse">Address: Alem 896</span></font>, Floor 6, Buenos Aires, Argentina</div></div></div></div></div></div></div></div></div></div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal"><span style="font-weight:bold">Hernán Wilkinson</span><br>Agile Software Development, Teaching & Coaching</span></font></span></span></span></strong></span></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">Phone: +54-011</span></font></span></span></span></strong></span><font size="2" face="tahoma, sans-serif">-4893-2057</font></div><div><strong style="font-family:tahoma,sans-serif;font-size:xx-small"><span style="font-size:8pt"><span style="font-size:small"><font size="2"><span style="font-weight:normal">Twitter: @HernanWilkinson</span></font></span></span></strong></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">site: <a href="http://www.10pines.com/" style="color:rgb(17,65,112)" target="_blank">http://www.10Pines.com</a></span></font></span></span></span></strong></span></div><div><font face="tahoma, sans-serif"><span style="border-collapse:collapse">Address: Alem 896</span></font>, Floor 6, Buenos Aires, Argentina</div></div></div></div></div></div></div></div></div></div></div></div>
</blockquote></div>
</blockquote></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal"><span style="font-weight:bold">Hernán Wilkinson</span><br>Agile Software Development, Teaching & Coaching</span></font></span></span></span></strong></span></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">Phone: +54-011</span></font></span></span></span></strong></span><font size="2" face="tahoma, sans-serif">-4893-2057</font></div><div><strong style="font-family:tahoma,sans-serif;font-size:xx-small"><span style="font-size:8pt"><span style="font-size:small"><font size="2"><span style="font-weight:normal">Twitter: @HernanWilkinson</span></font></span></span></strong></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">site: <a href="http://www.10pines.com/" style="color:rgb(17,65,112)" target="_blank">http://www.10Pines.com</a></span></font></span></span></span></strong></span></div><div><font face="tahoma, sans-serif"><span style="border-collapse:collapse">Address: Alem 896</span></font>, Floor 6, Buenos Aires, Argentina</div></div></div></div></div></div></div></div></div></div></div></div>
</blockquote></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal"><span style="font-weight:bold">Hernán Wilkinson</span><br>Agile Software Development, Teaching & Coaching</span></font></span></span></span></strong></span></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">Phone: +54-011</span></font></span></span></span></strong></span><font face="tahoma, sans-serif" size="2">-4893-2057</font></div><div><strong style="font-family:tahoma,sans-serif;font-size:xx-small"><span style="font-size:8pt"><span style="font-size:small"><font size="2"><span style="font-weight:normal">Twitter: @HernanWilkinson</span></font></span></span></strong></div><div><span style="font-family:tahoma,sans-serif;font-size:xx-small;border-collapse:collapse"><strong><span style="font-size:8pt"><span><span style="font-size:small"><font size="2"><span style="font-weight:normal">site: <a href="http://www.10pines.com/" style="color:rgb(17,65,112)" target="_blank">http://www.10Pines.com</a></span></font></span></span></span></strong></span></div><div><font face="tahoma, sans-serif"><span style="border-collapse:collapse">Address: Alem 896</span></font>, Floor 6, Buenos Aires, Argentina</div></div></div></div></div></div></div></div></div></div></div></div>