gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.53k stars 378 forks source link

Array out of bounds exception not thrown? #3348

Closed dankurka closed 9 years ago

dankurka commented 9 years ago

Originally reported on Google Code with ID 3343

Found in GWT Release:
1.5

Detailed description:

The following tests fail:

  public void testArray_empty() {
    String[] array = new String[0];
    try {
      Object value = array[0];
      fail("exception expected, but got: " + value);
    } catch (Exception expected) {
    }
  }

  public void testArray_nonEmpty() {
    String[] array = new String[10];
    try {
      Object value = array[10];
      fail("exception expected, but got: " + value);
    } catch (Exception expected) {
    }
  }

In both cases, the failure message says:
"exception expected, but got: undefined"

Reported by hayward.chan on 2009-02-07 10:36:19

dankurka commented 9 years ago
We don't catch unchecked exceptions in production code - there is a risk that the
overhead of checking this in production javascript could dramatically hurt
performance. I say leave it as is!

Reported by troy.jezewski on 2009-02-08 02:22:03

dankurka commented 9 years ago
Just a thought: couldn't ArrayIndexOutOfBoundsException be removed from the runtime

emulation? With compile-time errors, there's a chance that people won't use it; and

if it's documented in the JRE page, that they understand the underlying reason and

don't report bugs?

Reported by t.broyer on 2009-02-08 19:07:45

dankurka commented 9 years ago
I am fine with that exception thrown being a JavaScriptException instead of ArrayIndexOutOfBoundsException
(like other implicit excepitons such as NPE), but 
shouldn't there at least be an exception thrown (no matter what it is), instead of

letting an undefined value propagate?

Reported by hayward.chan on 2009-02-10 00:11:01

dankurka commented 9 years ago
No, the "problem" is about throwing an exception, whichever its type, because each 
time you'd code "Object value = array[10]" you'd end up with "if (10 >= array.length)

{ throw some_exception } else { Object value = array[10]; }".
It's much better if you do not rely on ArrayIndexOutOfBoundsException and code it 
like that in your java code (the test against array.length); as this won't impact 
other array accesses where you know for sure that you won't go beyond the array 
length (such as in a for loop).

Reported by t.broyer on 2009-02-10 09:04:29

dankurka commented 9 years ago
The Java Language Specification is fairly unambiguous about this one. 

http://java.sun.com/docs/books/jls/second_edition/html/arrays.doc.html

"10.4 Array Access
...
All array accesses are checked at run time; an attempt to use an index that is less
than zero or greater than or equal to the length of the array causes an
ArrayIndexOutOfBoundsException to be thrown."

To contrast the options:

The case for "throw ArrayIndexOutOfBoundsException": 
- Code Maintainability: Early detection of failures, and no "secondary effect"
failures masking this failure (good reason!).
- Compliance with the Java Language Spec, consistency between hosted and script mode.
- Crazy people who write code which relies on catching ArrayIndexOutOfBoundsException
can keep writing crazy code.

The case for "return undefined":
- Faster performance (good reason!)
- A few less bytes in the code.

So it's maintainability vs performance.

Sounds like there should be a "compatability" compiler option, and/or method
annotation. That way you can run your automated tests in the "compatibility mode" to
catch those bugs, and override the setting for performance where you need it.

Reported by troy.jezewski on 2009-02-10 11:17:08

dankurka commented 9 years ago
Similar issue with strings: issue 6942

IMO, those two should be closed as KnownQuirks; I'll leave it to someone else to decide
though.

Reported by t.broyer on 2011-10-31 19:25:55

dankurka commented 9 years ago

Reported by rdayal@google.com on 2011-11-04 19:37:52