On Wed, 13 Mar 2024 13:51:27 GMT, Claes Redestad <redes...@openjdk.org> wrote:
> Relying on the upper bounds check of `charAt` doesn't work well with the
> `CharArraySequence` whose `charAt` deliberately does not throw an IIOBE if
> the array is longer than the provided length, ie, it'll look at chars beyond
> the provided range. The examples I tested still end up as a NFE, but it's
> clear from the cause that we're running past the length:
>
> ```
> jshell> new BigDecimal(new char[] { '-', '1', 'e'}, 0, 1);
> | Exception java.lang.NumberFormatException
> | at BigDecimal.<init> (BigDecimal.java:754)
> | at BigDecimal.<init> (BigDecimal.java:543)
> | at BigDecimal.<init> (BigDecimal.java:518)
> | at (#4:1)
> | Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds
> for length 3
> | at BigDecimal$CharArraySequence.charAt (BigDecimal.java:559)
> | at BigDecimal.parseExp (BigDecimal.java:772)
> | at BigDecimal.<init> (BigDecimal.java:619)
> | ...
> ```
>
> Baseline/expected:
>
> ```
> jshell> new BigDecimal(new char[] { '-', '1', 'e'}, 0, 1);
> | Exception java.lang.NumberFormatException: No digits found.
> | at BigDecimal.<init> (BigDecimal.java:635)
> | at BigDecimal.<init> (BigDecimal.java:518)
> | at (#1:1)
> ```
>
> Having a check on `len > 0` is more robust - and I'd be surprised if avoiding
> a redundant check on the loop entry is affecting performance?
If the likely error/boundary conditions change, those changed conditions should
be added to the regression tests.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18177#discussion_r1523509168
