HI Chris,

The changes will be limited to Interval, and will be changes to #= and hash
(and the interval test so this doesn't show up again).

I'll push the changes to inbox soon; and to trunk tomorrow/early next
week.  The test will go to Trunk with the changes to inbox (the test will
be what I've pushed to the inbox minus the 3380 part).

And, yes, I'll rebase if off of the current trunk version - there has been
significant changes since my last proposal.

Interestingly:

= anObject
    ^ self == anObject or:
        [ (anObject isInterval
            and:
                [ start = anObject first and:
                    [ step = anObject increment and: [ self last =
anObject last ] ] ])
            or: [ super = anObject ] ]

This is actually wrong - if the two items to compare are intervals but they
don't match based on interval hash (first/last/increment), then it will
check if super #= returns true - that is not desirable.  But, I understand
the desire you mention here - which I believe is what Eliot was driving for
as well.

-cbc

On Thu, Nov 15, 2018 at 2:21 PM Chris Muller <asqueaker at gmail.com> wrote:

> > Nice.  It seems like we have consensus on what to change.
> >
> > I'll push these changes (with the tests) to trunk soon.
>
> Hey, "seems" carries enough uncertainty to give us one final look
> before trunk.  By "these changes" are you referring to just the
> Interval>>#hash or some Array changes, too?  All we've seen so far are
> Collections-cbc.810.mcz, could we get one look at your final draft
> proposal before trunk?
>
> On a less important note, I personally find a pure conditional
> nomenclature more attractive than the embedded ifTrue:ifFalse:, like:
>
> = anObject
>     ^ self == anObject or:
>         [ (anObject isInterval
>             and:
>                 [ start = anObject first and:
>                     [ step = anObject increment and: [ self last =
> anObject last ] ] ])
>             or: [ super = anObject ] ]
>
> For whatever and whenever you push, I'm sure you already were but,
> just in case, I would be grateful if you would please base it solely
> off the current top trunk version with no intermediate versions in the
> ancestry.   :-)
>
> Thanks a lot finding this and helping get it fixed!
>
> Best Regards,
>   Chris
>
>  - Chris
>
>
> > The fix I have for #hash was exactly what Elliot suggested.
> > I'll make sure to include the rehash as well (thanks for the code
> snippit Bert!)
> > If no one objects strenuously, I'll also include Eliot's slight rewrite
> of #= has well - it is marginally cleaner and equally fast, so now is a
> reasonable time to include it.
> >
> > I'll delay working on bug #3380 for now - to fix this, we'd have to also
> add in a check on class in #= to make sure we aren't comparing an interval
> to an array.  Unless someone has been bitten by this recently, I'd rather
> wait.
> >
> > -cbc
> >
> >
> > On Thu, Nov 15, 2018 at 1:32 PM Chris Muller <asqueaker at gmail.com>
> wrote:
> >>
> >> > Isn't this extremely simple to fix?
> >> >
> >> > #= is implemented in terms of start, step, and last.
> >> >
> >> > So why not implement #hash as
> >> >
> >> > ^(start hash bitXor: step hash) bitXor: self last hash
> >> >
> >> > Then in the postscript do a
> >> >
> >> > HashedCollection rehashAll
> >> >
> >> > and that should be it, right?
> >> >
> >> > I did a quick check using
> >> >
> >> > Dictionary allInstances select: [ :dict | dict keys anySatisfy:
> #isInterval]
> >> >
> >> > and the system seems fine after that.
> >>
> >> You forgot about the universe that lies beyond the fringes of the
> >> image.  There be persistent data files out there.  And users.
> >> Possibly with systems that rely on Interval>>#hash.
> >>
> >> But since most applications don't use non-Integer based Intervals
> >> (since Interval doesn't really support them, by design, I guess),
> >> there's no reason whatsoever to decimate the universe when Eliot's
> >> simply fixes the corner case that no one is using anyway.
> >>
> >> +1 to Eliots suggestion to fix Interval>>#hash.
> >>
> >>  - Chris
> >>
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20181115/130e679d/attachment.html>