JDK-8262003: Class.arrayType should not throw IllegalArgumentException

[Johannes Kuhn]

Thanks Mandy.

Will amend the CSR to cover TypeDescriptor.OfField::arrayType as well, 
and create a patch.

Where should I put the tests?
Didn't find any tests that exercise Class::arrayType in test/jdk/java/lang/.
Any wishes for tests that I should add w.r.t. Class::arrayType?

- Johannes

On 24-Feb-21 20:54, Mandy Chung wrote:
> Hi Johannes,
> 
> Changing Class::arrayType to throw ISA makes sense to me.   I think 
> `TypeDescriptor.ofField::arrayType` spec should also be updated to throw 
> ISA to follow what JVM checks for array dimension because it's a static 
> constraint check [1] (rather than a resolution error) for `anewarray` 
> instruction to create an array no more than 255 dimensions.
> 
> The CSR looks okay.  I agree that the compatibility risk is low.
> 
> I'd like to review PR along with CSR together.
> 
> Mandy
> [1] 
> https://docs.oracle.com/javase/specs/jvms/se15/html/jvms-4.html#jvms-4.9.1
> 
> On 2/23/21 6:38 AM, Johannes Kuhn wrote:
>> I want to learn about writing a CSR, and need a sponsor teaching me 
>> the ropes.
>>
>> ----
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8262003
>>
>> Currently, Class.arrayType() will throw an IllegalArgumentException if 
>> the maximum number of dimensions will be exceeded.
>>
>> Throwing an IllegalArgumentException from a method that doesn't take 
>> an argument is, at least, strange.
>>
>> Therefore I would like to update the specification to allow this 
>> method to throw an IllegalStateException, similar to what 
>> ClassDesc.arrayType() does.
>>
>> ----
>>
>> The current plan is:
>>
>> * Create a CSR detailing the spec changes: 
>> https://bugs.openjdk.java.net/browse/JDK-8262211
>> * Surround the code with a try-catch block. Checking for the error 
>> case is hard, and somewhat rare, so I think this is appropriate.
>> * Copy the @throws javadoc line from ClassDesc.arrayType to 
>> Class.arrayType
>>
>> But there are a few questions I would love to get the answer on:
>>
>> * Both Class.arrayType() and ClassDesc.arrayType() are specified by 
>> TypeDescriptor.OfField. Should the specification of 
>> TypeDescriptor.OfField.arrayType() changed as well?
>> * Is the draft of my CSR fine? Should I add some details, or omit 
>> things? Rephrase things?
>>
>> In advance, thanks for any support and feedback on this.
>>
>> - Johannes
>