[Vm-dev] Fix for label duplications in Slang's case expansion

[Guillermo Polito]

> El 24 ene 2020, a las 21:22, Eliot Miranda <eliot.miranda at gmail.com> escribió:
> 
> Hi Guille,
> 
> On Fri, Jan 24, 2020 at 7:57 AM Guillermo Polito <guillermopolito at gmail.com <mailto:guillermopolito at gmail.com>> wrote:
>  
> Hi all,
> 
> with Federico, a student from Argentina, we have found a problem in the label generation inside expanded cases in Slang.
> The problem comes from the fact that expanded cases (using the pragma <expandCase>, typically used for bytecodes), duplicate the entire tree with its inlines too, also duplicating labels inside it, which generates compilation errors due to label redefinition.
> 
> I’ve written a test for the case:
> 
>         | translation method codeGenerator |
>         method := self class >> #methodWithCases.
>         translation := method asTranslationMethodOfClass: TMethod.
>         codeGenerator := CCodeGeneratorGlobalStructure new.
>         codeGenerator addMethod: ((self class >> #expanded) asTranslationMethodOfClass: TMethod).
>         codeGenerator addMethod: translation.
> 
>         codeGenerator prepareMethods.
>         codeGenerator inlineDispatchesInMethodNamed: #methodWithCases localizingVars: #().
>         self assert: translation statements first cases first statements last isLabel.
>         self assert: translation statements first cases third statements last isLabel.
> 
>         self
>                 deny: translation statements first cases first statements last label
>                 equals: translation statements first cases third statements last label.
> 
> 
> 
> And I’ve made a fix that renames labels on each expanded case, to ensure non shared labels are unique within each case.
> I’ve put it in vmmaker's inbox. I hope there are no problems regarding timestamps, I’ve been careful about that ^^.
> 
> Well, all the timestamps are gone.  But it took me a couple of minutes to extend the Monticello browser to filter-out all the methods that only differ in timestamps.  And that is the point.  We are in control of our destiny using Monticello, not nearly as much as with git.  

Inspecting the mcz, I see the timestamps were gone, sorry.
Still, I think the problem is deeper than git/not git. Because when doing the patch I carefully loaded latest VMMaker.oscog version.
I supposed that forcing loading in monticello would reload everything. I (wrongly) guessed it should have installed timestamps too.
Then I applied my change from there.

Even just now, I got a latest fresh pharo9,
 - downloaded latest VMMaker from source.squeak.org <http://source.squeak.org/>
 - made a diff and a merge with the version I generated in the inbox
    => both empty => I cannot see problems related to missing timestamps in the UI :/
And no git was involved *at all* here.

I was maybe supposed to see changes in timestamps here? Does MC diff show that at all?
Now that I think about it, I don’t have any memories of it. But that could have helped me spot the problem before.
Maybe I could patch MC diff in Pharo to take timestamps into account?



> Anyway, thank you.  i can now see the two changed methods, can easily integrate, and give them a meaningful timestamp.  Can you email me the test method?

I’ll attach a changeset in a couple of minutes ^^

>  
> 
> Commit + commit message below:
> 
> Name: VMMaker.oscog-GuillermoPolito.2676
> Author: GuillermoPolito
> Time: 24 January 2020, 4:27:09.847804 pm
> UUID: 49bcc820-4659-0d00-b258-043c01043dd2
> Ancestors: VMMaker.oscog-eem.2675
> 
> Fix in slang case statement expansion labels.
> 
> During expansion in case statements, trees are duplicated and expanded.
> However, labels inside those trees are duplicated using the same name, producing compilation problems due to label conflicts/redefinition.
> 
> This fix uses #renameLabelsForInliningInto: when expanding case statements to rewrite labels generating unique labels.
> 
> Cheers,
> Guille
> 
> 
> -- 
> _,,,^..^,,,_
> best, Eliot

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20200127/0419da78/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PastedGraphic-11.png
Type: image/png
Size: 336185 bytes
Desc: not available
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20200127/0419da78/attachment-0001.png>