Pooh, this one is harder than I thought.


Maybe we should log tempWriteCounts, too, and make sure it is lower or equal to one where we check #tempReadCounts now?

Eliot, can you maybe share your thoughts on this issue before I go ahead?


Best,

Christoph

________________________________
Von: Uzonyi Levente <leves at caesar.elte.hu> im Auftrag von Levente Uzonyi <leves at caesar.elte.hu>
Gesendet: Dienstag, 23. November 2021 12:55:38
An: Thiede, Christoph
Cc: squeak-dev at lists.squeakfoundation.org
Betreff: Re: Merge Request: #caseOf:otherwise: with arguments

Hi Christoph,

I've got no experience with the decompiler at all.
I had a look at your changes. It does what I expect it to do in the case I
mentioned but it breaks when the variable is used more than once:

| x |
x := 1.
(x := 1)
         caseOf: {
                 [ 2 ] -> [ 2 ].
                 [ 3 ] -> [ 3 ] }
         otherwise: [ x + 1 ]


will be decompiled as

x := 1.
1 caseOf: {
         [2] -> [2].
         [3] -> [3]}
          otherwise: [:x | x + 1]]

Notice that x is not defined when it first assigned to in the
decompiled code.

The same problem is present with #ifNil:#ifNotNil: too:

| x |
x := 1.
(x := 1)
         ifNil: [ 2 ]
         ifNotNil: [ x + 1 ]

is decompiled as

x := 1.
1
         ifNil: [2]
         ifNotNil: [:x | x + 1]

with the Compiler-mt.462 as well.

I understand that the decompiler cannot be perfect without the
compiler storing additional information in the bytecodes but I think
it should be possible
- to always generate the compact version with the block arguments unless the same variable is used outside of the expression's scope, and
- to never generate invalid code.


Levente

On Mon, 22 Nov 2021, christoph.thiede at student.hpi.uni-potsdam.de wrote:

> Hi Levente,
>
> please see Compiler-ct.464 (inbox). To you as a more experienced decompiler developer, does this implementation look convincing? If yes, I can merge it myself now. :-)
>
> Best,
> Christoph
>
> ---
> Sent from Squeak Inbox Talk
>
> On 2021-11-18T14:04:16+01:00, marcel.taeumel at hpi.de wrote:
>
> > Hi Levente --
> >
> > Thanks. Decompiler only adds the block-arg for not-inlined caseOfs. See ObjectTest >> #testCaseOfOtherwise.
> >
> > Hmm... Christoph? =) Otherwise (ha!) I will take a look at it soon-ish.
> >
> > Best,
> > Marcel
> > Am 18.11.2021 13:51:02 schrieb Levente Uzonyi <leves at caesar.elte.hu>:
> > Hi Marcel,
> >
> > I just checked how this works, and noticed that the decompiler cannot
> > recreate the original code if the otherwise: block has an argument.
> >
> >
> > Levente
> >
> > On Thu, 18 Nov 2021, Marcel Taeumel wrote:
> >
> > > Merged.
> > >
> > > Am 21.02.2020 17:33:03 schrieb Thiede, Christoph :
> > >
> > > This changeset makes it possible to accept an argument in the otherwise block of a #caseOf:otherwise: call.
> > >
> > >
> > > Diff of the #caseOf:otherwise: implementation:
> > >
> > > caseOf: aBlockAssociationCollection otherwise: aBlock
> > >
> > > "The elements of aBlockAssociationCollection are associations between blocks.  Answer the evaluated value of the first association in aBlockAssociationCollection whose evaluated key equals the receiver.  If no
> > > match is found, answer the result of evaluating aBlock."
> > >
> > >
> > > aBlockAssociationCollection associationsDo:
> > >
> > > [:assoc | (assoc key value = self) ifTrue: [^assoc value value]].
> > >
> > > - ^ aBlock value
> > >
> > > + ^ aBlock cull: self
> > >
> > >
> > > "| z | z := {[#a]->[1+1]. ['b' asSymbol]->[2+2]. [#c]->[3+3]}. #b caseOf: z otherwise: [0]"
> > >
> > > "| z | z := {[#a]->[1+1]. ['d' asSymbol]->[2+2]. [#c]->[3+3]}. #b caseOf: z otherwise: [0]"
> > >
> > > "The following are compiled in-line:"
> > >
> > > "#b caseOf: {[#a]->[1+1]. ['b' asSymbol]->[2+2]. [#c]->[3+3]} otherwise: [0]"
> > >
> > > "#b caseOf: {[#a]->[1+1]. ['d' asSymbol]->[2+2]. [#c]->[3+3]} otherwise: [0]"
> > >
> > > + "#b caseOf: {[#a]->[1+1]. ['d' asSymbol]->[2+2]. [#c]->[3+3]} otherwise: [:x | x halt]"
> > >
> > >
> > > Furthermore, the changeset includes a necessary modification of MessageNode >> #transformCase: so that the otherwise argument can be compiled in-line.
> > >
> > > Last but not least, I wrote some tests for #caseOf:[otherwise:].
> > >
> > >
> > > Please review!
> > >
> > > (In a later change, it would be possible to allow arguments for the association key blocks as well. But I love short feedback loops, so let's assess this one first :-))
> > >
> > >
> > > Best,
> > >
> > > Christoph
> > >
> > >
> > >
> > -------------- next part --------------
> > An HTML attachment was scrubbed...
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211118/e87b1cbc/attachment.html>
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211127/f2471265/attachment.html>