On Fri, 15 Feb 2019, Chris Muller wrote:

>>> There was another change to earlier today that you may be interested
>> > in asking that question about too, since it changed 19-year old
>> > SequenceableCollection>>#= with a one-day old replacement and actually
>> > went into trunk.
>>
>> Please note that this has been discussed more than a month ago, and the discussion clearly converged to this consensus:
>> - having Interval and Array being = is not an important feature
>> - it seriously impacts efficiency of Interval hash
>> - it causes long standing bugs that we carry for years (19 years or so)
>
> Yes, I remember that discussion.
>
>> What's the point of having Interval = Array, when we have OrderedCollection ~= Array at the same time?
>> A less arbitrary feature would be a generalization of eachOperand isSequenceable ==> testThatElementsAreSameSequence:
>> But it's both too hard to maintain (a bug factory), aand a problem for efficient implementation of some specialized collection.
>> At the end, it's maybe not so much a desirable feature and it's practically not used in trunk images, apart for writing shorter tests.
>> I'm not a big fan of C motto when used with extremism: you don't pay for what you don't buy.
>> But I have the feeling that it makes sense here.
>
> I have no reason to critique this reasoning, I simply did not have
> time to test with it yet.  Sometimes testing after tweaks at low
> levels can "reveal" things that weren't thought about before.  But
> also, what I really dislike are continued use == on the class test.

What is the use case which doesn't work with the

 	foo class == bar class ifTrue: [ ... ]

pattern?

Levente

> I'm sorry, but these kinds of hacks are killing Squeak's dynamism by a
> thousand cuts and me along with it.  :(   Would you **Please
> Delegate** equivalence tests (#=) to the receiver, and let them
> inherit the identity implementation from Object?
>
> - Chris
>
>
> - Chris
>
>> BTW, I don't like  hasEqualElements: because I mentally associate elements to Set, and for me #(1 2 3)  hasEqualElements: #(3 1 2), just with a different order.
>> I would prefer something like isSameSequenceAs: which clearly tells about the ordering.
>>
>> Le mer. 13 févr. 2019 à 17:21, Chris Cunningham <cunningham.cb at gmail.com> a écrit :
>>>
>>> Thank you. This was an angle I hadn't thought of - but definitely opens up how to fix it correctly:
>>> >Definitively abandon SequenceableCollection equality tests based on equal species.
>>>
>>> On Tue, Feb 12, 2019 at 2:56 PM <commits at source.squeak.org> wrote:
>>>>
>>>> Nicolas Cellier uploaded a new version of Collections to project The Trunk:
>>>> http://source.squeak.org/trunk/Collections-nice.820.mcz
>>>>
>>>> ==================== Summary ====================
>>>>
>>>> Name: Collections-nice.820
>>>> Author: nice
>>>> Time: 12 February 2019, 11:56:35.262017 pm
>>>> UUID: bb383133-067c-4133-987b-c481a7de69c7
>>>> Ancestors: Collections-ul.819, Collections-cbc.813
>>>>
>>>> Definitively abandon SequenceableCollection equality tests based on equal species.
>>>>
>>>> Old behaviour can still be obtained thru hasEqualElements: but the default is to not try to support such trans-class equality feature because it is much too complex.
>>>>
>>>> Particularly Interval are no more equal to Arrays with same sequence. We can thus optimize hash a bit more and fix the old bugs of equa objects with different hashes. Merge Collections-cbc.813 for this and rehashAll in postscript.
>>>>
>>>> There are not so many classes concerned by this change, mainly RunArray, Interval and LinkedList:
>>>>
>>>> Collection withAllSubclasses select: [:e | [e basicNew species ~= e] on: Error do: [false]]
>>>> -> an OrderedCollection(WeakRegistry LinkedList Interval ByteCharacterSet CharacterSetComplement LazyCharacterSet WideCharacterSet ShortRunArray Semaphore Mutex TextLineInterval WeakArray Monitor MCVersionName ByteSymbol WideSymbol)
>>>>
>>>> We will have to change the tests that rely on such equality.
>>>>
>>>> =============== Diff against Collections-ul.819 ===============
>>>>
>>>> Item was changed:
>>>>   ----- Method: FloatArray>>= (in category 'comparing') -----
>>>>   = aFloatArray
>>>> -       | length |
>>>>         <primitive: 'primitiveEqual' module: 'FloatArrayPlugin'>
>>>> +       ^super = aFloatArray!
>>>> -       aFloatArray class = self class ifFalse: [^ false].
>>>> -       length := self size.
>>>> -       length = aFloatArray size ifFalse: [^ false].
>>>> -       1 to: self size do: [:i | (self at: i)
>>>> -                       = (aFloatArray at: i) ifFalse: [^ false]].
>>>> -       ^ true!
>>>>
>>>> Item was changed:
>>>>   ----- Method: Interval>>= (in category 'comparing') -----
>>>>   = anObject
>>>> -
>>>>         ^ self == anObject
>>>> +               or: [anObject isInterval
>>>> +                       ifFalse: [super = anObject]
>>>> +                       ifTrue:
>>>> +                               [start = anObject first
>>>> +                                and: [step = anObject increment
>>>> +                                and: [self last = anObject last]]]]!
>>>> -               ifTrue: [true]
>>>> -               ifFalse: [anObject isInterval
>>>> -                       ifTrue: [start = anObject first
>>>> -                               and: [step = anObject increment
>>>> -                                       and: [self last = anObject last]]]
>>>> -                       ifFalse: [super = anObject]]!
>>>>
>>>> Item was changed:
>>>>   ----- Method: Interval>>hash (in category 'comparing') -----
>>>>   hash
>>>>         "Hash is reimplemented because = is implemented."
>>>> +         ^((start hash hashMultiply bitXor: self last hash) hashMultiply
>>>> +                 bitXor: self size)!
>>>> -
>>>> -       ^(((start hash bitShift: 2)
>>>> -               bitOr: stop hash)
>>>> -               bitShift: 1)
>>>> -               bitOr: self size!
>>>>
>>>> Item was changed:
>>>>   ----- Method: RunArray>>= (in category 'comparing') -----
>>>>   = anObject
>>>> +       self == anObject ifTrue: [^ true].
>>>> -       "Test if all my elements are equal to those of anObject"
>>>> -
>>>>         ^anObject class == self class
>>>> +               and:
>>>> -               ifTrue: "Faster test between two RunArrays"
>>>>                         [(runs hasEqualElements: anObject runs)
>>>> +                        and: [values hasEqualElements: anObject values]]!
>>>> -                        and: [values hasEqualElements: anObject values]]
>>>> -               ifFalse:
>>>> -                       [anObject isCollection and: [self hasEqualElements: anObject]]!
>>>>
>>>> Item was changed:
>>>>   ----- Method: SequenceableCollection>>= (in category 'comparing') -----
>>>>   = otherCollection
>>>>         "Answer true if the receiver is equivalent to the otherCollection.
>>>> +       First test for identity, then rule out different class and sizes of
>>>> -       First test for identity, then rule out different species and sizes of
>>>>         collections. As a last resort, examine each element of the receiver
>>>>         and the otherCollection."
>>>>
>>>>         self == otherCollection ifTrue: [^ true].
>>>> +       self class == otherCollection class ifFalse: [^ false].
>>>> -       self species == otherCollection species ifFalse: [^ false].
>>>>         ^ self hasEqualElements: otherCollection!
>>>>
>>>> Item was changed:
>>>>   ----- Method: Symbol>>= (in category 'comparing') -----
>>>>   = aSymbol
>>>>         "Compare the receiver and aSymbol."
>>>>         self == aSymbol ifTrue: [^ true].
>>>> +       aSymbol isSymbol ifTrue: [^ false].
>>>> -       self class == aSymbol class ifTrue: [^ false].
>>>>         "Use String comparison otherwise"
>>>>         ^ super = aSymbol!
>>>>
>>>> Item was changed:
>>>> + (PackageInfo named: 'Collections') postscript: 'Character initializeClassificationTable.
>>>> + HashedCollection rehashAll.'!
>>>> - (PackageInfo named: 'Collections') postscript: 'Character initializeClassificationTable'!
>>>>
>>>>
>>>
>>