On Wed, Nov 26, 2014 at 01:56:32PM -0800, Eliot Miranda wrote:
> On Wed, Nov 26, 2014 at 1:26 PM, Levente Uzonyi <leves at elte.hu> wrote:
> 
> > On Wed, 26 Nov 2014, David T. Lewis wrote:
> >
> >  On Wed, Nov 26, 2014 at 08:01:49PM +0100, Levente Uzonyi wrote:
> >>
> >>> On Wed, 26 Nov 2014, Eliot Miranda wrote:
> >>>
> >>>  Hi All,
> >>>> ? ? IdentitySet>>collect: answers a Set, not an IdentitySet.? Anyone
> >>>> else
> >>>> agree this is a serious bug?? Anyone else disagree?
> >>>>
> >>>> WTF??
> >>>>
> >>>> (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] a Set(1.0
> >>>> 2 3)
> >>>> --
> >>>> best,Eliot
> >>>>
> >>>>
> >>> Sometimes it's feasable to return an IdentitySet, other times it's not,
> >>> so
> >>> there's no optimal solution - #collect: can't cover all cases.
> >>>
> >>> Set >> #collect: explicitly returns a Set, because this is the least bad
> >>> solution to handle all of its subclasses reasonably well.
> >>> In case of WeakSet, returning a WeakSet makes no sense, because some of
> >>> your objects will disappear immediately.
> >>> In case of IdentitySet, PluggableSet, KeyedSet and KeyedIdentitySet the
> >>> returned values may not behave well in the original collection.
> >>>
> >>> The best is to always be explicit:
> >>>
> >>> (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [ :e | e ] as:
> >>> IdentitySet
> >>> "==> an IdentitySet(1 2 3 1.0 2.0 3.0)"
> >>>
> >>> Levente
> >>>
> >>>
> >> In Squeak 2.8 (checked on http://bertfreudenberg.github.io/SqueakJS/run/
> >> ):
> >>
> >>  (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] ==> a
> >> Set(2.0 1 2 3 3.0 1.0)
> >>
> >>
> >> But in trunk I get this:
> >>
> >>  (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] ==> a Set(1
> >> 2 3)
> >>
> >>
> >> So answering a Set makes sense for the reasons that Levente explains, but
> >> trunk
> >> is definitely broken WRT the contents of that set.
> >>
> >> This bug needs a unit test to go along with whatever fix we agree on.
> >>
> >
> > It's not a bug either. The Floats are not there, because they are equal to
> > the integers:
> >
> > 1 = 1.0 "==> true"
> > 1.0 = 1 "==> true"
> >
> > Therefore they have the same hash:
> >
> > 1 hash "==> 1"
> > 1.0 hash "==> 1"
> >
> > The bug was in Squeak 2.8:
> > 1.0 = 1 "==> true"
> > 1 = 1.0 "==> true"
> > 1 hash "==> 1"
> > 1.0 hash "==> 61440"
> >
> > Here's the comment of Float >> hash from The Trunk:
> >         "Hash is reimplemented because = is implemented. Both words of the
> > float are used. (The bitShift:'s ensure that the intermediate results do
> > not become a large integer.) Care is taken to answer same hash as an equal
> > Integer."
> >
> 
> Exactly.

Yes. I confused myself by going too far back in the wayback machine.
It seems that in Squeak 2.8 we had this:

  Set withAll: #(1 2 3 1.0 2.0 3.0) ==>  Set(2.0 1 2 3 3.0 1.0)

Which is clearly wrong, as Levente explains above.

Dave

> 
> My hope would be something like this (Set order is arbitrary)
> 
>         ((IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e]) =>
> IdentitySet (1 2 3 1.0 2.0 3.0)
> 
> but that
>        (Set withAll: #(1 2 3 1.0 2.0 3.0)) => Set (1 2 3)
> 
> (and it always contains just the integers since these are added first).
> 
> But Levente's collect:as: solution is perfectly acceptable and as he points
> out has the advantage of being explicit.
>