Bug in Dolphin closures

Hello,

 I've found the following bug in Dolphin Smalltalk v2.1 (patch level
2) while programming in a LISP-like style (whether or not it is a good
idea is another problem!) :

 | qsort |
qsort := [:list |
    list isEmpty
        ifTrue: [list]
        ifFalse: [| hi lo head tail |
            head := list first.
            tail := list copyFrom: 2 to: list size.
            hi := tail select: [:elt | elt > head].
            lo := tail select: [:elt | elt <= head].
            (qsort value: lo) add: head; addAll: (qsort value: hi);
yourself]].
qsort value: #(4 5 2 8 7 1 2 3) asOrderedCollection

Result is: an OrderedCollection(1 2 2 2)
instead of:  OrderedCollection (1 2 2 3 4 5 7 8) (given by
VisualWorks)

 Why do the closures lost their context ? Wouldn't it be better to
have non-reentrant blocks like Squeak ones (to avoid typing such
erroneous code) ?

 Regards,

 Sebastien.

Sebastien Furic wrote: [snip]

There does seem to be something a little odd here.  I'm using
DPRO 4.01.3 for the following.  I've rendered Sebastian's
codelet above as a method of OrderedCollection as:

quicksort
   | qsort |
   qsort :=
     [:list |
        list isEmpty
            ifTrue: [list]
           ifFalse: [| hi lo head tail |
                     head := list first.
                     tail := list copyFrom: 2 to: list size.
                     hi := tail select: [:elt | elt > head].
                     lo := tail select: [:elt | elt <= head].
                     (qsort value: lo) add: head;
                                    addAll: (qsort value: hi) ;
                                         yourself
                     ].
        ].
 ^qsort value: self.

So, if I do

     x := #(4 5 2 8 7 1 2 3) asOrderedCollection.

     x quicksort.

I get the same as him, namely,

     an OrderedCollection(1 2 2 2)

Second, if I annotate the above just a little do produce:

quick
   | qsort c r |
   c := 0.
   qsort :=
     [:list :d |
        c := 1 + c.
        Transcript cr ; show: ' Counter ' , c printString , ' d = ' ,
d printString.
        list isEmpty
            ifTrue: [list]
           ifFalse: [| hi lo head tail |
                     head := list first.
                     tail := list copyFrom: 2 to: list size.
                     hi := tail select: [:elt | elt > head].
                     lo := tail select: [:elt | elt <= head].
                     (qsort value: lo value: (1+d)) add: head;
                                    addAll: (qsort value: hi value:
(1+d)) ;
                                    yourself
                     ].
        ].
   r := qsort value: self value: 0.
   Transcript cr.
   ^r.

which is just the same method but with a couple of counter variables
added, I, of course, get the same result, but the Transcript display
is strange:

    Counter 1 d = 0
    Counter 2 d = 1
    Counter 3 d = 2
    Counter 4 d = 3
    Counter 5 d = 4
    Counter 6 d = 5
    Counter 7 d = 6
    Counter 8 d = 7
    Counter 9 d = 8

The d-value ought to be the same on the same level.

Finally, if I rewrite Sebastian's code altogether, as

   quicksort1
    | hi lo head tail result |
    self isEmpty
       ifTrue: [^self.]
      ifFalse: [ head := self first.
                 tail := (self copyFrom: 2 to: self size).
                 hi := tail select: [:elt | elt > head].
                 lo := tail select: [:elt | elt <= head].
                 hi := hi copy quicksort1.
                 lo := lo copy quicksort1.
                 result := lo.
                 result add: head.
                 result addAll: hi.
                 ^result.
                ].

having the recursion but no parameter passing block or #value:
selector, I get the correct result,

     an OrderedCollection(1 2 2 3 4 5 7 8)
 
For completeness I should report that moving the temporary list from
within the #ifFalse: block to the parameter-bearing block has no
effect.

If anyone wants, I've attached a package with these codes included as
loose
methods.

Good hunting.

--
---------------------------------------------------------------------
 Jan Theodore Galkowski                           [hidden email]
 The Smalltalk Idiom                     http://www.algebraist.com/
*********************************************************************
             "Smalltalk?  Yes, it's really that slick."
---------------------------------------------------------------------
Want to know more?  Check out
           http://www.dnsmith.com/SmallFAQ/
           http://www.object-arts.com/DolphinWhitePaper.htm
           http://st-www.cs.uiuc.edu/users/johnson/smalltalk/
*********************************************************************

Sebastien,

>  Why do the closures lost their context ?

Dolphin's lack of full block closures (where every recursive iteration
maintains it's own copy of the arguments) has been mentioned before in the
newsgroup. It's probably best to browse the archive to get a fuller idea of
what was said but I think Blair's post on 28/3/2000 (below) sums up OA's
feelings at the time - although I obviously don't know if they still feel
the same

Regards
    Ian

~-~-~- Blair on 28/3/2000

>...But isn't Dolphin supposed to have full block closures ?

Hmmm, interesting question. Philosphically yes, it is "supposed" to have
them, because it has always been our intention that it should because they
are an elegant concept, and we are of the school that think thats beauty is
probably more important than aalmost anything else in software. However in
practice simpler Smalltalk-80 style blocks always seem to be able to get the
job done, and this makes them a lower priority than they deserve. In fact I
think the main practical disadvantage of St-80 blocks is their tendency to
prolong the life of object nets by maintaining a reference
to their arguments after they have finished evaluating. Re-entrant uses of
blocks seemt to be quite rare, so this doesn't generally cause problems.