Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression in range equality #8738

Closed
scabug opened this issue Jul 20, 2014 · 3 comments
Closed

regression in range equality #8738

scabug opened this issue Jul 20, 2014 · 3 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Jul 20, 2014

See: https://groups.google.com/forum/#!topic/scala-internals/wDXhYFBINs4

It appear Range equality no longer works correctly in scala 2.11 (was correct in 2.10)

first the symptoms:
 Range(0,0) == Range(10,100,10)
res0: Boolean = true

and

Range(0,1) == Range(0,0)
java.util.NoSuchElementException
  at scala.collection.LinearSeqOptimized$class.last(LinearSeqOptimized.scala:134)
  at scala.collection.immutable.List.last(List.scala:83)
  at scala.collection.immutable.Range.last(Range.scala:107)
  at scala.collection.immutable.Range.equals(Range.scala:371)
  ... 32 elided


looking at the code, the following commit seems to be responsible:
https://github.com/scala/scala/commit/e152297090c26051b7e9a6a1740c4670d23d9d5d
we used to have a check (length == x.length) which was removed.
The current code, if the first Range is empty it will be equal to any Range
when the second isEmpty and the first is not empty and begins with 0 it will throw an exception when accessing last on the empty Range.

This is my suggested fix (addition is blue):

  override def equals(other: Any) = other match {
    case x: Range =>
      // Note: this must succeed for overfull ranges (length > Int.MaxValue)
      (x canEqual this) && (
        (isEmpty && x.isEmpty) || // all empty sequences are equal

        (nonEmpty && x.nonEmpty && start == x.start && {                  // Otherwise, must have same start
          val l0 = last
          (l0 == x.last && (                    // And same end
            start == l0 || step == x.step       // And either the same step, or not take any steps
          ))
        })
      )
    case _ =>
      super.equals(other)
  }
-------------------------------------------------------------------
Minor correction: for the exception to occur the empty range must have the same start as the first range. 
Evidently empty Ranges still can have a different starting position (which seems funny to me)
@scabug
Copy link
Author

@scabug scabug commented Jul 20, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8738?orig=1
Reporter: @retronym
Affected Versions: 2.11.0

@scabug
Copy link
Author

@scabug scabug commented Jul 20, 2014

@Ichoran said:
Ack, sorry--I don't know how that got through testing.

Will fix immediately.

@scabug
Copy link
Author

@scabug scabug commented Jul 20, 2014

@scabug scabug closed this Jul 20, 2014
@scabug scabug added the blocker label Apr 7, 2017
@scabug scabug added this to the 2.11.2 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.