[9] RFR of 8065556: (bf) Buffer.position and other methods should include detail in IAE

[John Rose]

On Mar 23, 2015, at 3:12 PM, Martin Buchholz <martinrb at google.com> wrote:
> 
> Because this is a critical low-level core library:
> I would not have a checkPositionBounds method at all and save a branch by using | instead of || and optimize for the current hotspot implementation:
> 
> if ((newPosition > limit) | (newPosition < 0)) 
>   throw new IllegalArgumentException(positionOutOfBoundsMsg(newPosition));

I will confirm that it's best, at present, to separate hot code from cold code.

Eventually I think we'll have good enough library functions for range checks that we can avoid working with explicit range check expressions.  Even experts get them wrong when subexpressions can overflow (for subsequence range checks).

For now, I would add one more tweak to Martin's:

> if ((newPosition > limit) | (newPosition < 0)) 
>   throw positionOutOfBoundsException(newPosition);

The instance creation expression and the string formatting are both cold code which deserves to be segregated into a factory method.

The throw itself is convenient to put into the caller code, though, because (a) it doesn't increase caller bytecode complexity, and (b) it doesn't confuse javac or the code maintainer with a nonexistent fall-through of control after the error.

— John