Mantis - Squeak
Viewing Issue Advanced Details
6937 Collections minor always 02-19-08 18:12 04-18-10 22:04
closed 3.10  
none trunk  
0006937: removeAll: aCollection doesn't do what we expect if aCollection==self
This known error has had a revival originating from a mail of Sophie in the beginnner list... and I ended up opening a bug report...

The problem is:

col := #(1 2 3) asOrderedCollection.
col removeAll: col.
^col returns an OrderedCollection(2) instead of an empty one as we would expect...

"If you start removing items from the collection, increasing the
index will cause you to skip elements." from Ron

Ron suggested:
"I agree it is an error, but it's one that you hit pretty early in your
Smalltalk career, I'm not sure anything really needs to change. I would
support just using a copy on the parameter and adding #removeAll: Is this
necessary? It wouldn't hurt."

col := #(1 2 3) asOrderedCollection.
col removeAll: col **copy**.
^col returns an OrderedCollection()
"Because modifying the collection while iterating over the same is not
safe generally. The above idiom is "known" - but of course it sucks. :)

I advocated to add a #removeAll message in the protocol of Collection
and then implement it in subclasses using efficient variants a looooong
while back but it never got accepted. The climate today for such a
change is much more... "pragmatic" I think and hope. So we should simply
add it!"

Concerning using copy...
"Thumbs down, I do not want anybody copy a collection behind my back
(however small or large it is) just because remove* has nothing to do with

I was suggesting raising an error as, it's quite "strange" (circular) to have an expression like: aCol removeAll: aCol ...

But Klaus promised a fix so ;)

related to 0007177closed andreas removeAll fails for Dictionaries [^] (562 bytes) 02-19-08 18:51

02-19-08 18:51 tested with the 441 cases in the CollectionTests category, all green.
nicolas cellier   
02-19-08 20:28   
"As posted in beginners list, self == aCollection does not handle case when aCollection is just a wrapper on self...
So the change won't removeAll problems...
in a 3.9 image try this:"

| collec1 collec2 |
collec1 := OrderedCollection with: 'a' with: 'b' with: 'c'.
collec2 := MappedCollection collection: collec1 map: (3 to: 1 by: -1).
collec1 removeAll: collec2.
collec1 inspect

"Agree, yet, the problem lies in super...
Note: MappedCollection was removed from 3.10, but in spirit there can be other wrapper collections (i used some myself)."
nicolas cellier   
02-19-08 21:09   
"Oops, i copy/pasted the wrong test.
The one that fails is more obvious:"

| collec1 collec2 |
collec1 := OrderedCollection with: 'a' with: 'b' with: 'c'.
collec2 := MappedCollection collection: collec1 map: (1 to: 3).
collec1 removeAll: collec2.
02-19-08 21:41   
as mentioned in, since 3.10 MappedCollection no longer exists. Do you have a counter example which fails indeed in Squeak 3.10; can you break the attached fix in 3.10?
nicolas cellier   
02-19-08 22:43   
Proposed change is OK to me. I vote for inclusion in 3.11.
It does not fail per se. super fails...

Simply, i wanted to point that it won't solve all the tricky cases.
So these cases will have to be handled at super level.
And adding a self == aCollection at super level will still have flaws.

MappedCollection is not mainstream anymore, but other wrappers could become, see LazyLists and LazyCollections in Squeak source...
Collection wrapper is a classical pattern.
As long as they are used under your control, no problem.
02-22-08 23:35   
There are perhaps other collections (besides OrderedCollection) which would benefit from such fix.
05-29-08 17:35   
"fix begin"
Installer mantis bug:6937 fix: ''.
"fix end"
12-18-08 05:21   
sometimes I do no understand what "status" this Mantis thing wants to have changed, there seemingly are 3 of them :(

I hope that this post influences one of the stati towards the positive, the fix was attached long time ago.
nicolas cellier   
09-14-09 20:03   
I tried to generalize the case in 0007177
There, an alternate #removeAll: implementation exploits the #removeAll message.
#removeAll message can be found in
- Keith's Kernel-Extensions at [^]
- 0007177
- in recent squeak trunk [^]
nicolas cellier   
10-04-09 20:37   
This was resolved by 0007177