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.

Dave