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

ListBuffer's insert, insertAll, update and updated accept negative positions #6632

Closed
scabug opened this issue Nov 9, 2012 · 11 comments
Closed

Comments

@scabug
Copy link

scabug commented Nov 9, 2012

None of the following operations should succeed:

scala> import collection.mutable.ListBuffer
import collection.mutable.ListBuffer

scala> val lb = ListBuffer('a, 'b, 'c, 'd, 'e)
lb: scala.collection.mutable.ListBuffer[Symbol] = ListBuffer('a, 'b, 'c, 'd, 'e)

scala> lb.insert(-1, 'x)

scala> lb
res19: scala.collection.mutable.ListBuffer[Symbol] = ListBuffer('a, 'x, 'b, 'c, 'd, 'e)

scala> lb.insertAll(-2, Array('y, 'z))

scala> lb
res21: scala.collection.mutable.ListBuffer[Symbol] = ListBuffer('a, 'y, 'z, 'x, 'b, 'c, 'd, 'e)

scala> lb.update(-3, 'u)

scala> lb
res23: scala.collection.mutable.ListBuffer[Symbol] = ListBuffer('a, 'u, 'z, 'x, 'b, 'c, 'd, 'e)

scala> lb.updated(-4, 'd)
res24: scala.collection.mutable.ListBuffer[Symbol] = ListBuffer('d, 'u, 'z, 'x, 'b, 'c, 'd, 'e)

The documentation even specifies:

IndexOutOfBoundsException
if the index n is not in the valid range 0 <= n <= length. 
@scabug
Copy link
Author

scabug commented Nov 9, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6632?orig=1
Reporter: @soc
Affected Versions: 2.9.2, 2.10.0-RC1, 2.10.0-RC2, 2.10.0, 2.11.0-M1

@scabug
Copy link
Author

scabug commented Nov 24, 2012

@soc said:
Merged scala/scala@522ef79 which fixes this issue for insert, insertAll and update.
updated is not fixed, discussion needed where exactly the fix needs to be added.

@scabug
Copy link
Author

scabug commented Feb 2, 2013

@JamesIry said:
Making this change could break user code that "works" with negative indexes. I'm kicking to 2.11.0

@scabug
Copy link
Author

scabug commented Mar 15, 2013

@adriaanm said:
Un-assigning to foster work stealing, as announced in https://groups.google.com/forum/?fromgroups=#!topic/scala-internals/o8WG4plpNkw

@scabug
Copy link
Author

scabug commented Oct 15, 2013

@gkossakowski said:
Unassigning and rescheduling to M7 as previous deadline was missed.

@scabug
Copy link
Author

scabug commented Dec 23, 2013

@Ichoran said:
Updated still not fixed. I'm happy to do so, if we agree it should be fixed rather than documenting the admittedly weird behavior.

@scabug
Copy link
Author

scabug commented Dec 24, 2013

@SethTisue said:
+1 on fixing for 2.11

@scabug
Copy link
Author

scabug commented Dec 31, 2013

@soc said:
Yes, we should fix it. That it hasn't been fixed yet was just due to my limited time available, not the lack of desire.

Imho a good way to start would be to survey whether there is some kind of implicit consensus in the other implementations on how to behave and it's just this one which behaves differently, or whether implementations' behavior in error cases is all over the place.
Additionally, it would make sense to see how other languages/libraries deal with those kinds of input and try to learn whether their approach works better than maybe the straightforward approach we might gather while looking at Scala's collections solely.

@scabug
Copy link
Author

scabug commented Dec 31, 2013

@soc said:
This is kind of fishy, too:

scala> import collection.mutable.{ArrayBuffer => ListBuffer}
import collection.mutable.{ArrayBuffer=>ListBuffer}

scala> val lb = ListBuffer('a, 'b, 'c, 'd, 'e)
lb: scala.collection.mutable.ArrayBuffer[Symbol] = ArrayBuffer('a, 'b, 'c, 'd, 'e)

scala> lb.updated(-4, 'd)
res7: scala.collection.mutable.ArrayBuffer[Symbol] = ArrayBuffer('d, 'b, 'c, 'd, 'e)

scala> lb.updated(12, 'd)
java.lang.UnsupportedOperationException: empty.tail
  at scala.collection.TraversableLike$class.tail(TraversableLike.scala:449)
  at scala.collection.mutable.IndexedSeqLike$$anon$1.scala$collection$IndexedSeqOptimized$$super$tail(IndexedSeqLike.scala:52)
  at scala.collection.IndexedSeqOptimized$class.tail(IndexedSeqOptimized.scala:129)
  at scala.collection.mutable.IndexedSeqLike$$anon$1.tail(IndexedSeqLike.scala:52)
  at scala.collection.SeqLike$class.updated(SeqLike.scala:516)
  at scala.collection.AbstractSeq.updated(Seq.scala:41)
  ... 32 elided

@scabug
Copy link
Author

scabug commented Feb 10, 2014

@adriaanm said:
Since 2.11.0-RC1 is one week away, pushing all non-blockers without PR to 2.11.1-RC1. Please undo the change if I missed work in progress.

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@Ichoran said:
Fixed updated. Waiting for Jenkins to tell me the fallout, if any.

scala/scala#3530

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants