Code generation bug

Hi, the following seems to be a bug in code generation.

If I haven't lost the plot completely, the following two methods should both
produce infinite loops printing 10 to the Transcript each time around.

=============
    test
         Transcript clear.
        [| add |
        Transcript print: add; cr.
        add isNil ifTrue: [add := 10] ifFalse: [add := add + 10].
        add > 20 ifTrue: [^ self]]
            repeat.
=============

or:

=============
test2
    Transcript clear.
    [| add |
    Transcript print: add; cr.
    add isNil ifTrue: [add := 10] ifFalse: [add := add + 10].
    add > 20]
        whileFalse.
=============

In fact they both loop just three times, since it seems that the local variable
add is not reset to nil.

    -- chris

In the first case, when add becomes more than 20 the method will return
because of the caret sign.  So it correctly breaks out on the third
iteration.

In the second case, because of the whileFalse, the iteration will stop
once the last statement in the block returns true, which is when add is
greater than 20, or the third iteration.

Mike

I guess, that was your whole point, that it does return because the add
variable is not reset for each iteration.  Sorry.

Chris,

        currently Dolphin  has blue-book blocks so block temps are simply
syntactic sugar for method temps.  Example one is actually equivalent to

    test
         | add |
         Transcript clear.
         [Transcript print: add; cr.
          add isNil ifTrue: [add := 10] ifFalse: [add := add + 10].
          add > 20 ifTrue: [^ self]]
             repeat.

and example two similarly.  You need ANSI block semantics for the above
to work correctly.  Blair and Andy will be able to tell you when that'll be.

Chris Uppal wrote:
--
_______________,,,^..^,,,____________________________
Eliot Miranda              Smalltalk - Scene not herd

Eliot,

> You need ANSI block semantics for the above
> to work correctly.  Blair and Andy will be able to tell you when that'll
> be.

I can't see any obvious reason why Blue-book semantics should affect this.  For
one thing, there is nothing to stop the compiler generating code to
re-initialise the block local variable each time the block is evaluated
(although I don't think Dolphin does that, in general).  More importantly, both
of the examples are inlined by the compiler, so there is no block creation at
all.  D6 does have full block semantics, but (as I've already reported on the
beta test list), the same bug is present there.

    -- chris

Chris,

> all.  D6 does have full block semantics, but (as I've already reported on
> the
> beta test list), the same bug is present there.

... and if you bypass the inlining, by creating the block in a separate
accessor method, then Dolphin 6 behaves in the way you originally expected.

--
Ian

Use the Reply-To address to contact me.
Mail sent to the From address is ignored.

Eliot Miranda wrote:

When will VW get ANSI block semantics? You can modify the example
slightly to get VW to fail:
     test
          Transcript clear.
         [| add |
        4 = 5 ifTrue: [add := 0].
         Transcript print: add; cr.
         add isNil ifTrue: [add := 10] ifFalse: [add := add + 10].
         add > 20 ifTrue: [^ self]]
             repeat.


John Brant

Hi John,

     in vw7.4 ;)  We had another bug report (see 49241.st) on the same
bug recently.  Find attached the as-yet-unreviewed fix wot I wrote...
Use at your own risk...



John Brant wrote:
--
_______________,,,^..^,,,____________________________
Eliot Miranda              Smalltalk - Scene not herd

| c t |
c := OrderedCollection new.
1 to: 10 do: [ :i |
    t := i.
    c add: [t]].
c collect: [ :b | b value].

Transcript clear.
[| add |
 4 = 5 ifTrue: [add := 0].
 Transcript print: add; cr.
 add isNil ifTrue: [add := 10] ifFalse: [add := add + 10].
 add > 20 ifTrue: [^self]] repeat
<?xml version="1.0"?>

<st-source>
<time-stamp>From VisualWorks®, 7.3.1 of April 20, 2005 on May 6, 2005 at 12:00:18 pm</time-stamp>


<methods>
<class-id>Kernel.BlockAnalysisBlock</class-id> <category>testing</category>

<body package="System-Compiler-Support" selector="optimizedBlocksInside:do:">optimizedBlocksInside: homeBlock do: aBlock
        "Evaluate aBlock with any and all optimized blocks on the static chain from the receiver to some homeBlock."

        | block |
        block := parent.
        [block == homeBlock] whileFalse:
                [block isOptimized ifTrue: [aBlock value: block].
                 block := block parent]</body>
</methods>

<methods>
<class-id>Kernel.BlockAnalysisBlock</class-id> <category>printing</category>

<body package="System-Compiler-Support" selector="printOn:">printOn: aStream
        super printOn: aStream.
        node notNil ifTrue:
                [aStream nextPutAll: ' on: '.
                 node printOn: aStream]</body>
</methods>

<methods>
<class-id>Kernel.BlockAnalysisVariable</class-id> <category>printing</category>

<body package="System-Compiler-Support" selector="printOn:">printOn: aStream
        super printOn: aStream.
        node notNil ifTrue:
                [aStream nextPutAll: ' on: '.
                 node printOn: aStream]</body>
</methods>

<methods>
<class-id>Kernel.BlockAnalyzer</class-id> <category>enumerating</category>

<body package="System-Compiler-Support" selector="doBlock:arguments:body:">doBlock: aNode arguments: args body: seq

        | proxy |
        proxy := BlockAnalysisBlock new initialize; node: aNode from: self.
        currentBlock := proxy.
        allBlocks add: proxy.
        scope := Dictionary new -&gt; scope.

        varDefType := proxy isOptimized ifTrue: [#fixedTemp] ifFalse: [#arg].
        self doNodes: args.
        varDefType := nil.
        self doNode: seq.
        proxy isOptimized
                ifTrue: [args do: [:arg | | binding list |
                        binding := self bindingFor: arg name.
                        "We should be able to assume that binding is
                        not nil, and not a 'self' reference."
                        list := binding references reject:
                                        [:blk | blk isActive or: [blk isOptimized]].
                        list notEmpty ifTrue:
                                [proxy useComplexCodeIn: self.
                                 binding doCopy]]].
        proxy references do:
                [:baVar|
                 baVar home isOptimized ifFalse:
                  [proxy
                                optimizedBlocksInside: baVar home
                                do: [:optBlock| optBlock deOptimizeFor: self]]].
        proxy active: false.
        scope := scope value.
        currentBlock := currentBlock parent.</body>
</methods>

</st-source>

Chris Uppal wrote:

Whether the change in semantics can be effected by changing the VM or
the compiler or not, one doesn't change execution semantics lightly...


 > More importantly, both
> of the examples are inlined by the compiler, so there is no block creation at
> all.  D6 does have full block semantics, but (as I've already reported on the
> beta test list), the same bug is present there.

Ah, I didn't know.  That's good news.  Then it looks like Dolphin and VW
have the same bug, which is that when inlining blocks local block temps
are not correctly initialized on subsequent invocations of the inlined
block, which can be fixed either by not inlining or by inserting an
initialization.

--
_______________,,,^..^,,,____________________________
Eliot Miranda              Smalltalk - Scene not herd