On 20 June 2013 12:58, David T. Lewis <lewis at mail.msen.com> wrote:
> On Thu, Jun 20, 2013 at 12:20:00PM +0200, Nicolas Cellier wrote:
>> The bug is that (true ifTrue:[^true] ifFalse:[^false]) won't push any value
>> on the stack (same as emitEffect: rather than emitValue:)
>> The decompiler wants to find the argument of self value: ... on the stack
>> and it's not there.
>>
>> So either we should forbid such construct at compile time
>> (emit an 'un-reachable code' error when emitValue: does not emit any value)
>
> The original code example (before I reduced it to a trivial version)
> was this:
>
>     < aNumberWithUnits
>         (self compareUnits: aNumberWithUnits)
>             ifTrue: [self value: (aNumberWithUnits value < self value
>                             ifTrue: [^ true]
>                             ifFalse: [^ false])]
>             ifFalse: [^ Error new signal: 'Incompatible unit types.']
>
> This does not have the unreachable branch, and it displays the same
> decompiler error.

The #value: send to self is unreachable because both of the inner
#ifTrue:ifFalse: blocks force a return.

frank

> Dave
>
>
>
>>
>> Or patch Decompiler like mad to work around...
>> (but Decompiler's methods are yet well below usual quality in term of
>> complexity - long methods, many instance variables, incredibly complex
>> state, ...)
>>
>> I agree, this is going to be a rare problem only for newbies, because an
>> experimented Smalltalker will avoid writing unreachable code.
>> So, the third solution is DO NOTHING, or more exactly add a test and an
>> expected failure.
>> Like that we'll say next time, hey bad luck, this is a known limitation.
>>
>> Nicolas
>>
>>
>>
>> 2013/6/20 Frank Shearar <frank.shearar at gmail.com>
>>
>> > On 20 June 2013 02:12, David T. Lewis <lewis at mail.msen.com> wrote:
>> > > On Thu, Jun 20, 2013 at 01:26:14AM +0200, Nicolas Cellier wrote:
>> > >> See
>> > >>
>> > http://stackoverflow.com/questions/17129583/smalltalk-collection-is-empty-error-when-saving/17202938#17202938
>> > >>
>> > >> This bug was found by a newbie on pharo 1.1, but it's still there in 4.5
>> > >> trunk image
>> > >>
>> > >> Try to decompile this (no matter in which class)
>> > >>
>> > >> < aNumberWithUnits
>> > >>     (self compareUnits: aNumberWithUnits)
>> > >>         ifTrue: [self value: ((aNumberWithUnits value) < (self value)
>> > >> ifTrue: [^true] ifFalse: [^false]).]
>> > >>         ifFalse: [^Error new signal: 'Incompatible unit types.'].
>> > >>
>> > >> Thanks to the newbie!
>> > >> I would just have hated that!
>> > >> Learning from one's own error is great, but from a dysfunctional system
>> > not
>> > >> so :(
>> > >
>> > > Good bug catch! Here is a reduced version of the method that produces the
>> > > same error when decompiled:
>> > >
>> > >
>> > >   Foo>>decompilerBug
>> > >       self value: (true
>> > >           ifTrue: [^ true]
>> > >           ifFalse: [^ false])
>> >
>> > I'm guessing that this is because
>> >
>> > true
>> >   ifTrue: [^ true]
>> >   ifFalse: [^ false]
>> >
>> > decompiles as
>> >
>> > true ifTrue: [^ true]
>> > ^ false
>> >
>> > ?
>> >
>> > I'm not terribly worried about claims of a "dysfunctional system".
>> > This bug is so severe that noone has noticed it before! I agree that
>> > we need to fix the bug, but the original code makes no sense in the
>> > first place. #value: is unreachable.
>> >
>> > frank
>> >
>> > > Dave
>> > >
>> > >
>> >
>> >
>
>>
>
>