fix mutable.HashSet.addAll: remove redundant call to super method

[miteshaghera]

Removed redundant call to super method.

Fixes scala/bug#11601

[dwijnand]

Hey, @miteshaghera. Thanks for the PR! Do you think you could add a unit test that validates this fix and ensures it doesn't regress, please?

[miteshaghera]

@dwijnand Give me some time I'll update unit test which validate this PR.

[dwijnand]

Thanks!

[SethTisue]

could you squash this into a single commit?

this looks otherwise ready to merge.

[som-snytt]

For a test, I'd suggest something direct:

package once

import collection.IterableOnce

class OnceOnly extends IterableOnce[Int] {
  override def knownSize = 1
  private var iterated: Boolean = false
  override def iterator = {
    assert(!iterated)
    iterated = true
    new Iterator[Int] {
      private var v = Option(42)
      override def hasNext = v.nonEmpty
      override def next() = { val res = v.get ; v = None ; res }
    }
  }
}

object Test extends App {
  val hs = collection.mutable.HashSet.empty[Int]
  hs.addAll(new OnceOnly)
}

Putting it in junit form is left as an exercise for the reader.

[som-snytt]

Looking at what I wrote, I'm not sure if knownSize should be zero after the iterator is exhausted?

Ideally, all the API taking an IterableOnce would be tested that it obeys the contract. Is it a contract if it's just a handshake between two good ole boys that they promise not to call it twice?

[miteshaghera]

For a test, I'd suggest something direct:

package once

import collection.IterableOnce

class OnceOnly extends IterableOnce[Int] {
  override def knownSize = 1
  private var iterated: Boolean = false
  override def iterator = {
    assert(!iterated)
    iterated = true
    new Iterator[Int] {
      private var v = Option(42)
      override def hasNext = v.nonEmpty
      override def next() = { val res = v.get ; v = None ; res }
    }
  }
}

object Test extends App {
  val hs = collection.mutable.HashSet.empty[Int]
  hs.addAll(new OnceOnly)
}

Putting it in junit form is left as an exercise for the reader.

Thanks, I'll add this is as another test as well. Can I use your snippet?

[som-snytt]

Yes, please use it in any way if it's helpful.

[miteshaghera]

could you squash this into a single commit?

this looks otherwise ready to merge.

I squashed the commits.

[lrytz]

Thank you, @miteshaghera!