Smalltalk › Squeak › Squeak - Dev

The Inbox: CollectionsTests-cbc.296.mcz

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

4 messages Options

commits-2

The Inbox: CollectionsTests-cbc.296.mcz

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: interval2 hash.!

Eliot Miranda-2

Re: The Inbox: CollectionsTests-cbc.296.mcz

Hi Chris,

On Sun, Oct 28, 2018 at 5:13 PM <[hidden email]> wrote:

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.

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.!

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: interval2 hash.!

_,,,^..^,,,_

best, Eliot

Nicolas Cellier

Re: The Inbox: CollectionsTests-cbc.296.mcz

Le lun. 29 oct. 2018 à 08:33, Eliot Miranda <[hidden email]> a écrit :

Hi Chris,

On Sun, Oct 28, 2018 at 5:13 PM <[hidden email]> wrote:
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.

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?

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

+ 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: interval2 hash.!

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

cbc

Re: The Inbox: CollectionsTests-cbc.296.mcz

On Mon, Oct 29, 2018 at 12:47 PM Nicolas Cellier <[hidden email]> wrote:

Le lun. 29 oct. 2018 à 08:33, Eliot Miranda <[hidden email]> a écrit :
Hi Chris,

On Sun, Oct 28, 2018 at 5:13 PM <[hidden email]> wrote:
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.

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?
+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

+ 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: interval2 hash.!

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