[squeak-dev] The Inbox: CollectionsTests-cbc.296.mcz

Discussion:

c***@source.squeak.org

0000-10-30 06:03:47 UTC

Chris Cunningham uploaded a new version of CollectionsTests to project The Inbox:
http://source.squeak.org/inbox/CollectionsTests-cbc.296.mcz

==================== Summary ====================

Name: CollectionsTests-cbc.296
Author: cbc
Time: 28 October 2018, 5:12:30.875512 pm
UUID: 12a1d6bf-85c7-8d40-aa1c-022556c1cb18
Ancestors: CollectionsTests-topa.295

Test for #hash and #= bugs. In anticipation of fixing these.

=============== Diff against CollectionsTests-topa.295 ===============

Item was added:
+ ----- Method: IntervalTest>>testHashBug3380 (in category 'tests') -----
+ testHashBug3380
+ "Array and Interval equate, but their hashes didn't. Test that this is fixed.
+ It is about mantis bug http://bugs.squeak.org/view.php?id=6455"
+ | interval array |
+ interval := (1 to: 3).
+ array:= #(1 2 3).
+ self assert: interval equals: array.
+ self assert: interval hash equals: array hash.!

Item was added:
+ ----- Method: IntervalTest>>testHashEqualIfIntervalEqual (in category 'tests') -----
+ testHashEqualIfIntervalEqual
+ | interval1 interval2 |
+ interval1 := 0 to: 1.
+ interval2 := 0 to: 5/3. "Taken from an actual issue in an image"
+ self assert: interval1 equals: interval2.
+ self assert: interval1 hash equals: inte

Eliot Miranda

2018-10-29 07:32:45 UTC

Permalink

Hi Chris,

Post by c***@source.squeak.org
http://source.squeak.org/inbox/CollectionsTests-cbc.296.mcz
==================== Summary ====================
Name: CollectionsTests-cbc.296
Author: cbc
Time: 28 October 2018, 5:12:30.875512 pm
UUID: 12a1d6bf-85c7-8d40-aa1c-022556c1cb18
Ancestors: CollectionsTests-topa.295
Test for #hash and #= bugs. In anticipation of fixing these.
=============== Diff against CollectionsTests-topa.295 ===============
+ ----- Method: IntervalTest>>testHashBug3380 (in category 'tests') -----
+ testHashBug3380
+ "Array and Interval equate, but their hashes didn't. Test that
this is fixed.
+ It is about mantis bug http://bugs.squeak.org/view.php?id=6455"
+ | interval array |
+ interval := (1 to: 3).
+ array:= #(1 2 3).
+ self assert: interval equals: array.

At least for me I don't see why this should be true. We have
hasEqualElements: for this, so

SequenceableCollection allSubclasses select:
[:c|
[(#(1 2 3) as: c) = #(1 2 3)] on: Error do: [:ex| false]]
an OrderedCollection(LinkedList Interval RunArray Array Mutex WeakArray
Monitor)

SequenceableCollection allSubclasses reject:
[:c|
[(#(1 2 3) as: c) = #(1 2 3)] on: Error do: [:ex| true]] an
OrderedCollection(OrderedCollection SourceFileArray SoundBuffer WordArray
ShortIntegerArray Bitmap FloatArray ShortRunArray ByteArray IntegerArray
SparseLargeTable PredicatedArray DoubleByteArray DoubleWordArray Semaphore
SortedCollection GraphicSymbol UrlArgumentList TraitComposition
ObjectFinalizerCollection WeakOrderedCollection StandardSourceFileArray
ExpandedSourceFileArray WordArrayForSegment ActionSequence
WeakActionSequence Cubic KedamaFloatArray SocketAddress SparseLargeArray
FloatCollection WeakActionSequenceTrappingErrors)

SequenceableCollection allSubclasses reject:
[:c|
[(#(1 2 3) as: c) hasEqualElements: #(1 2 3)] on: Error do: [:ex| true]] an
OrderedCollection(SoundBuffer ShortIntegerArray)

i.e. there are many SequenceableCollection subclasses, including
OrderedCollection, and all the integer arrays (ByteArray through
DoubleWordArray) that are not #= to an equivalent integer array.

I would throw out the assumption that intervals and arrays are equal; as
long as hasEqualElements: answers true things are good, no?

+ self assert: interval hash equals: array hash.!

Post by c***@source.squeak.org
+ ----- Method: IntervalTest>>testHashEqualIfIntervalEqual (in category 'tests') -----
+ testHashEqualIfIntervalEqual
+ | interval1 interval2 |
+ interval1 := 0 to: 1.
+ interval2 := 0 to: 5/3. "Taken from an actual issue in an image"
+ self assert: interval1 equals: interval2.
+ self assert: interval1 hash equals: interval2 hash.!

--
_,,,^..^,,,_
best, Eliot

Nicolas Cellier

2018-10-29 19:47:21 UTC

Permalink

Post by Eliot Miranda
Hi Chris,

At least for me I don't see why this should be true. We have
hasEqualElements: for this, so
[:c|
[(#(1 2 3) as: c) = #(1 2 3)] on: Error do: [:ex| false]]
an OrderedCollection(LinkedList Interval RunArray Array Mutex WeakArray
Monitor)
[:c|
[(#(1 2 3) as: c) = #(1 2 3)] on: Error do: [:ex| true]] an
OrderedCollection(OrderedCollection SourceFileArray SoundBuffer WordArray
ShortIntegerArray Bitmap FloatArray ShortRunArray ByteArray IntegerArray
SparseLargeTable PredicatedArray DoubleByteArray DoubleWordArray Semaphore
SortedCollection GraphicSymbol UrlArgumentList TraitComposition
ObjectFinalizerCollection WeakOrderedCollection StandardSourceFileArray
ExpandedSourceFileArray WordArrayForSegment ActionSequence
WeakActionSequence Cubic KedamaFloatArray SocketAddress SparseLargeArray
FloatCollection WeakActionSequenceTrappingErrors)
[:c|
[(#(1 2 3) as: c) hasEqualElements: #(1 2 3)] on: Error do: [:ex| true]]
an OrderedCollection(SoundBuffer ShortIntegerArray)
i.e. there are many SequenceableCollection subclasses, including
OrderedCollection, and all the integer arrays (ByteArray through
DoubleWordArray) that are not #= to an equivalent integer array.
I would throw out the assumption that intervals and arrays are equal; as
long as hasEqualElements: answers true things are good, no?

+1

If = is based on species, we might have to revise =...

Post by Eliot Miranda
+ self assert: interval hash equals: array hash.!

--
_,,,^..^,,,_
best, Eliot

Chris Cunningham

2018-10-29 20:05:38 UTC

Permalink

On Mon, Oct 29, 2018 at 12:47 PM Nicolas Cellier <

Post by Nicolas Cellier

Post by Eliot Miranda
Hi Chris,

+1
If = is based on species, we might have to revise =...

So, I was doing a straight forward reading of bug 3380 (
http://bugs.squeak.org/view.php?id=3380 ) where it was talking about
falling back to the super slow hashing.

Based on your comments and Eliot's comments, I think I should backtrack
from that position - especially now that Interval is no longer the same
species as Array.

Still, that problem exists as noted in 3380; it is still the fact that (1
to: 3) = #( 1 2 3 ), but the hashes are different. This is because (among
other things) the hash for collections uses species while #= doesn't.

Using Eliot's formula:

(SequenceableCollection allSubclasses select:
[:c|
[(#(1 2 3) as: c) = #(1 2 3)] on: Error do: [:ex| true]] )
difference:
(SequenceableCollection allSubclasses select:
[:c|
[(#(1 2 3) as: c) hash = #(1 2 3) hash] on: Error do: [:ex| true]] )
an OrderedCollection(Interval RunArray TextLineInterval)

So, Interval, RunArray, and TextLineInterval have collection issues with
Array.
But, given that 26 subclasses of SequenceableCollection pass this test, we
don't want to change #= to be species aware. So, how to fix without
breaking other goodness? Or, just leave 3380 open for later inspection?

In any case, I'll revert to a much simpler Interval>>hash (called
#hashBetter in the other thread)

-cbc

Post by Nicolas Cellier

Post by Eliot Miranda
+ self assert: interval hash equals: array hash.!

--
_,,,^..^,,,_
best, Eliot