IndexOutOfBoundsException detail message

[scabug]

Hi,

I've noticed that the IndexOutOfBoundsException only returns the size of the collection as detail message. I think it should be a more detailed message like "size is n, but m is expected".

The class where I found the unfavorable message was scala.collection.LinearSeqOptimized$class.apply

what do you mean, would it be a good idea to change the message?

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-9797?orig=1
Reporter: Ben

[scabug]

@SethTisue said:
Sounds like a good idea to me. There are a lot of sites in the collections code to update here, not just one, I'm afraid.

[scabug]

@ruippeixotog said:
I like the idea of having more information about the collection bounds when the exception is thrown, it would have been useful to me in the past. However, there are probably performance tradeoffs:

• Can we always know the size of the collection in constant extra time? I can't think of any situation where that's not possible, but I may be missing some contrived case;
• Even if it's not worse in big-O notation, some extra counting and other work may be needed when the collection size can't be known in constant-time (e.g. in LinearSeq). Also, a bigger string with some string appends will have to be built.

I'm willing to take on this, if you think it's a good idea. @Ichoran, what do you think? Is it worth it performance-wise?

[scabug]

@Ichoran said (edited on Nov 13, 2016 12:51:02 AM UTC):
@ruippeixotog - I don't think any extra work should be done in a success case in order to make the exception message nicer. Given that exceptions are already really expensive compared to most things, making the message nicer when you are guaranteed to have to throw it, and it's easy and O(1) or even O(log n), is great. Just be cautious that you don't turn a method that is really tiny into one which is huge, so you don't lose automatic JIT inlining. You may need to introduce an extra private method to handle the exception in some cases.

For example, in

class TinyException {
  var n,m = 0;
  def foo() { println("The real work is here.") }
  def bar(b: Boolean) {
    if (b) foo else throw new Exception("Argh.")
  }
  def baz(b: Boolean) {
    if (b) foo else throw new Exception("n is " + n + " and m is " + m)
  }
}

bar is under the automatic inlining limit but baz is not.

[scabug]

Ben said (edited on Nov 15, 2016 9:13:17 AM UTC):
Error messages are supposed to inform the developer that something critical went wrong. I think it doesn't matter how much they cost because they are not the default in a runtime. A developer should avoid exceptions as much as he can but sometimes they are explicitly needed. The message purpose is to provide as much information as possible about the exception cause.

I think it costs nothing if you just add some text like "Can't access " + n + "th element in List". Just throw n is so ambiguous that somebody is totally confused if he uses for example only exception.getMessage().

[scabug]

@ruippeixotog said:
If the improved error messages only affect negatively the error scenario, I think we all agree that it is worth it. My question was if it was worth it to do extra work even in the success scenario just to provide better error messages (e.g. to show the size of the collection), and I agree with @Ichoran on that regard.

@Ichoran, is there an easy way I can check if methods are able of being JIT-inlined? If not, is there some golden rule to that?

[scabug]

@Ichoran said:
@ruippeixotog - There is the magical 35-byte limit under which point the JIT compiler pretty much automatically inlines a method. Other than that, it's certainly not forbidden from doing so, but it's less certain. But keeping forwarders small should be easy enough: you should almost always be able to convert an exception into a call to a private reportIndexException method in fewer bytes than the original exception generation.

So if a method looks like a forwarder to another method, I'd try to keep it as such.

@ben - People sometimes catch exceptions, at which point they do become part of the normal runtime in unusual cases. Changing the execution time by several orders of magnitude is, therefore, not something to do lightly. Creating an exception is O(n) in the depth of the stack, and has a pretty high constant factor also. Adding some constant or log term is fine. Adding O(m) with m >>> n is probably not, because it could make someone's normal error-handling code a performance bottleneck.

[scabug]

Ben said:
@rex Kerr yes I understand this situation, that's why I said it would be enough to enhance the string itself instead of adding more variables.

[gourlaysama]

Fixed in scala/scala#7210.