Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class.arrayType() throws UnsupportedOperationException in jdk19+ #15298

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

pshipton
Copy link
Member

@pshipton pshipton commented Jun 10, 2022

Issue #14598

Continuing to throw IllegalArgumentException as the cause for the UnsupportedOperationException is done for compatibility.

@pshipton
Copy link
Member Author

jenkins compile amac jdknext

@pshipton pshipton added the jdk19 label Jun 10, 2022
pshipton added a commit to pshipton/openjdk-tests that referenced this pull request Jun 10, 2022
Resolved via eclipse-openj9/openj9#15298

Signed-off-by: Peter Shipton <Peter_Shipton@ca.ibm.com>
@pshipton pshipton force-pushed the atype branch 2 times, most recently from bf52296 to 2f4c09f Compare June 10, 2022 21:54
@pshipton
Copy link
Member Author

jenkins compile amac jdknext

@pshipton
Copy link
Member Author

jenkins compile amac jdknext

@pshipton
Copy link
Member Author

jenkins compile amac jdknext

@pshipton
Copy link
Member Author

Tested in grinders, which passed.
https://openj9-jenkins.osuosl.org/view/Test/job/Grinder/890 - java/lang/Class/ArrayType.java
https://openj9-jenkins.osuosl.org/view/Test/job/Grinder/892 - Jep334Tests jdk19
https://openj9-jenkins.osuosl.org/view/Test/job/Grinder/893 - Jep334Tests jdk18

@pshipton
Copy link
Member Author

@tajila what do you think about this condition added to Class.arrayType(), to throw UnsupportedOperationException "if the number of dimensions of the resulting array type would exceed 255". This is a limitation that OpenJ9 doesn't have.
http://cr.openjdk.java.net/~iris/se/19/build/latest/api/java.base/java/lang/Class.html#arrayType()

The java/lang/Class/ArrayType.java test checks for this. I added a check so the test can pass, but we could also ignore it (exclude or modify the test) or challenge it.

@pshipton pshipton requested a review from tajila June 11, 2022 12:13
Copy link
Contributor

@tajila tajila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, no one relies on being able to create arrays with more than 255 dimensions. I see no harm in adhering to RI behaviour

throw new IllegalArgumentException();
/*[IF JAVA_SPEC_VERSION >= 19]*/
try {
int arrayCount = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps something like this may be clearer

        Class<?> baseType = this;
    	for (int arrayCount = 0; baseType.isArray(); arrayCount++) {
    		if (arrayCount == 255) {
    			throw new IllegalArgumentException();
    		}
               baseType = baseType.getComponentType();

    	}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason I missed this comment.
Made this change and retested, which failed. It works when I changed the check to 254. Or I suppose I could start the count from 1 and check for 255 if you prefer.

@pshipton
Copy link
Member Author

jenkins compile amac jdk19

Issue eclipse-openj9#14598

Signed-off-by: Peter Shipton <Peter_Shipton@ca.ibm.com>
@pshipton
Copy link
Member Author

jenkins compile amac jdk19

@pshipton
Copy link
Member Author

pshipton commented Jun 21, 2022

Rerun java/lang/Class/ArrayType.java with the update.
https://openj9-jenkins.osuosl.org/view/Test/job/Grinder/995 - passed

@pshipton pshipton requested a review from tajila June 21, 2022 20:59
@pshipton
Copy link
Member Author

@tajila when you get a chance to review.

@tajila tajila merged commit 48b8900 into eclipse-openj9:master Jun 23, 2022
smlambert pushed a commit to adoptium/aqa-tests that referenced this pull request Jun 24, 2022
Resolved via eclipse-openj9/openj9#15298

Signed-off-by: Peter Shipton <Peter_Shipton@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants