On 2/25/21 6:52 AM, Johannes Kuhn wrote:
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?
Yes, I suggest to put it under test/jdk/java/lang/Class/arrayType.
Mandy
- 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
