On Oct 5, 2009, at 3:19 53PM, Nicolas Cellier wrote:
> 2009/10/5 Henrik Johansen <[email protected]>:
>> Good to know I'm not the only one who found it strange :)
>>
>> BTW, I thought the default for Collections was to do a shallowCopy
>> with a copy operation?
>>
>
> I removed this default behaviour SequenceableCollection>>shallowCopy
> which used to use (self copyFrom: 1 to: self size).
> This beautifull hack was using self species, leading to this mess...
> And it forced me defining a #basicShallowCopy to work the hack around
> (which was the shortest but uggliest solution).
>
> The copy was deep enough so that modifying the copy would not modify
> the original, and this is what postCopy now does.
>
>> ll := LinkedList new.
>> ll add: Link new.
>> ll2 := ll copy.
>> ll add: Link new.
>>
>> inspecting ll2, it's firstLink does not have a nextLink, which
>> implies
>> to me a deepCopy has occured.
>>
>
> I did not take time to write all tests yet, I will give it a try in
> trunk.
>
>> Of course, it's arguable that that's better behaviour for
>> LinkedLists,
>> seeing as otherwise ll2's lastLink would be invalid when you added to
>> ll.
>> At a glance, SparseLargeTable, Stack (Through LinkedList
>> implementation) and WideCharacterSet in your changeset all seem to do
>> a deep copy.
>>
>
> just deep enough, not more, not less.
To my understanding, that's too deep.
You're copying elements in the collection as well, not just creating a
copy of the collection with the same elements.
That's what I'd define as a deepCopy, after a copy, I'd expect the
elements in them to be the same.
Maybe it's just what I've gotten used to in VW, and Squeak does
something else historically, but mixing and matching which to do based
on what feels best at any given time can lead to some nasty bugs :)
>
>> Also, how does part in SharedQueue2
>> monitor critical: [
>> monitor := Monitor new.
>> items := items copy
>> ] work? Seems a bit fishy to me to have a critical section on a
>> monitor, then replacing it as part of it...
>>
>
> He he, it should just work, because we are talking to a Monitor object
> (the Smalltalk way), not to a pointer *monitor somewhere in memory
> (the C/C++ view).
Oh, absolutely, it will not crash due to deallocated pointers or such.
What I meant was a concurrency issue, when some other object requests
something that requires a monitor lock, after Monitor has been
replaced, but before the items have been copied.
i.e.
while the intent of the critical section in that piece of code as far
as I can tell, is that the two operations should happen without items
being accessible before they have been copied.
so you'd actually have to write something like:
monitor critical: [
monitor := Monitor new.
monitor critical:
[items := items copy]]
to ensure that does not happen.
Cheers,
Henry
_______________________________________________
Pharo-project mailing list
[email protected]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
