8310849: Pattern matching for instanceof and arrayType cleanup in j.l.invoke and j.l.reflect #14642

liach · 2023-06-25T09:18:34Z

This patch touches java.lang.reflect and java.lang.invoke packages. It replaces instanceof + cast with pattern matching and updates Array.newInstance().getClass() patterns with arrayType() for obtaining array types of a class.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8310849: Pattern matching for instanceof and arrayType cleanup in j.l.invoke and j.l.reflect (Enhancement - P4)

Reviewers

Mandy Chung (@mlchung - Reviewer) ⚠️ Review applies to 7879c150
Joe Darcy (@jddarcy - Reviewer) ⚠️ Review applies to 7879c150

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14642/head:pull/14642
$ git checkout pull/14642

Update a local copy of the PR:
$ git checkout pull/14642
$ git pull https://git.openjdk.org/jdk.git pull/14642/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 14642

View PR using the GUI difftool:
$ git pr show -t 14642

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14642.diff

Webrev

Link to Webrev Comment

….invoke and j.l.reflect

bridgekeeper · 2023-06-25T09:20:03Z

👋 Welcome back liach! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-06-25T09:22:39Z

@liach The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2023-06-25T09:26:22Z

Webrevs

src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java

src/java.base/share/classes/sun/invoke/util/ValueConversions.java

Glavo · 2023-06-26T13:00:45Z

src/java.base/share/classes/sun/reflect/generics/reflectiveObjects/GenericArrayTypeImpl.java

-        if (o instanceof GenericArrayType) {
-            GenericArrayType that = (GenericArrayType) o;
-
+        if (o instanceof GenericArrayType that) {


return o instanceof GenericArrayType that && Objects.equals(genericComponentType, that.getGenericComponentType());

Glavo · 2023-06-26T13:01:54Z

src/java.base/share/classes/sun/reflect/generics/reflectiveObjects/WildcardTypeImpl.java

    public boolean equals(Object o) {
-        if (o instanceof WildcardType) {
-            WildcardType that = (WildcardType) o;
+        if (o instanceof WildcardType that) {


return o instanceof WildcardType that && Arrays.equals(this.getLowerBounds(), that.getLowerBounds()) && Arrays.equals(this.getUpperBounds(), that.getUpperBounds());

Glavo · 2023-06-26T13:04:02Z

src/java.base/share/classes/sun/reflect/annotation/AnnotatedTypeFactory.java

        @Override
        public boolean equals(Object o) {
-            if (o instanceof AnnotatedType &&
+            if (o instanceof AnnotatedType that &&


return o instanceof AnnotatedType that && !(o instanceof AnnotatedArrayType) && !(o instanceof AnnotatedTypeVariable) && !(o instanceof AnnotatedParameterizedType) && !(o instanceof AnnotatedWildcardType) && equalsTypeAndAnnotations(that);

src/java.base/share/classes/sun/invoke/util/Wrapper.java

mlchung

Looks okay to me.

mlchung · 2023-06-26T18:28:06Z

src/java.base/share/classes/java/lang/invoke/MethodType.java

-            if (o instanceof MethodType) {
-                return equals((MethodType)o);
-            }
+        if (x instanceof ConcurrentWeakInternSet.WeakEntry<?> e && e.get() instanceof MethodType mt) {


Nit: break this long line into 2.

Done. Thanks for the review and suggestion.

mlchung · 2023-06-26T18:31:15Z

src/java.base/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java

-                boolean typeArgEquality = Arrays.equals(actualTypeArguments, // avoid clone
-                                                        that.getActualTypeArguments());
-                for (Type t : actualTypeArguments) {
-                    System.out.printf("\t\t%s%s%n", t, t.getClass());


Surprised that these debugging statements are unnoticed for so many years.

openjdk · 2023-06-26T20:07:35Z

@liach This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8310849: Pattern matching for instanceof and arrayType cleanup in j.l.invoke and j.l.reflect

Reviewed-by: mchung, darcy

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 21 new commits pushed to the master branch:

79c056e: 8006421: GraphicsConfiguration of a frame is changed when the frame is moved to another screen
45b581b: 8309583: AArch64: Optimize firstTrue() when amount of elements < 8
87e6fab: 8310873: Re-enable locked_create_entry symbol check in runtime/NMT/CheckForProperDetailStackTrace.java for RISC-V
39fa4e6: 8310489: New test runtime/ClassInitErrors/TestStackOverflowDuringInit.java failed
46add3f: 8310909: java.io.InvalidObjectException has redundant @since tag
8f5b677: 8310908: Non-standard @since tag in com.sun.java.accessibility.util.package-info
a197ee7: 8310838: Correct range notations in MethodTypeDesc specification
7c6a28f: 8310922: java/lang/Class/forName/ForNameNames.java fails after being added by JDK-8310242
7db2f08: 8310242: Clarify the name parameter to Class::forName
297c799: 8301492: Modernize equals() method of ResourceBundle.CacheKey and Bundles.CacheKey
... and 11 more: https://git.openjdk.org/jdk/compare/a420ff48da7d8861a874ea6f7278249c20113215...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@mlchung, @jddarcy) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

src/java.base/share/classes/java/lang/invoke/MemberName.java

src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java

Co-authored-by: Andrey Turbanov <turbanoff@gmail.com>

liach · 2023-06-27T04:55:14Z

Tests pass, integrating. Thank you for your reviews!

/integrate

openjdk · 2023-06-27T04:56:45Z

@liach
Your change (at version 9ea36a5) is now ready to be sponsored by a Committer.

mlchung · 2023-06-27T16:09:17Z

/sponsor

openjdk · 2023-06-27T16:10:55Z

Going to push as commit 2bd4136.
Since your change was applied there have been 28 commits pushed to the master branch:

7ce967a: 8281455: Change JVM options with small ranges from 64 to 32 bits, for gc_globals.hpp
f6133ed: 8310890: Normalize identifier names
58bb655: 8222329: Readable read(CharBuffer) does not specify that 0 is returned when there is no remaining space in buffer
05e9c41: 8310731: Configure a javax.net.ssl.SNIMatcher for the HTTP/1.1 test servers in java/net/httpclient tests
1587836: 8310425: [JVMCI] compiler/runtime/TestConstantDynamic: lookupConstant returned an object of incorrect type: null
73d7aa1: 8301489: C1: ShortLoopOptimizer might lift instructions before their inputs
990e3a7: 8308899: Introduce Classfile context and improve Classfile options
79c056e: 8006421: GraphicsConfiguration of a frame is changed when the frame is moved to another screen
45b581b: 8309583: AArch64: Optimize firstTrue() when amount of elements < 8
87e6fab: 8310873: Re-enable locked_create_entry symbol check in runtime/NMT/CheckForProperDetailStackTrace.java for RISC-V
... and 18 more: https://git.openjdk.org/jdk/compare/a420ff48da7d8861a874ea6f7278249c20113215...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-06-27T16:11:13Z

@mlchung @liach Pushed as commit 2bd4136.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8310849: Pattern matching for instanceof and arrayType cleanup in j.l…

86808e3

….invoke and j.l.reflect

openjdk bot added the rfr Pull request is ready for review label Jun 25, 2023

openjdk bot added the core-libs core-libs-dev@openjdk.org label Jun 25, 2023

Merge branch 'master' into cleanup/invoke-instanceof

4f5829e

Glavo suggested changes Jun 26, 2023

View reviewed changes

simplify equals by replacing if with and operators

7879c15

mlchung approved these changes Jun 26, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 26, 2023

jddarcy approved these changes Jun 26, 2023

View reviewed changes

turbanoff reviewed Jun 26, 2023

View reviewed changes

src/java.base/share/classes/java/lang/invoke/MemberName.java Outdated Show resolved Hide resolved

turbanoff reviewed Jun 26, 2023

View reviewed changes

src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java Outdated Show resolved Hide resolved

liach and others added 2 commits June 27, 2023 05:55

Use single space for single-line if statements

2625943

Co-authored-by: Andrey Turbanov <turbanoff@gmail.com>

Wrap line that is too long

9ea36a5

openjdk bot added the sponsor Pull request is ready to be sponsored label Jun 27, 2023

openjdk bot added the integrated Pull request has been integrated label Jun 27, 2023

openjdk bot closed this Jun 27, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Jun 27, 2023

liach mentioned this pull request Aug 23, 2025

8366028: MethodType::fromMethodDescriptorString should not throw UnsupportedOperationException for invalid descriptors #26909

Closed

3 tasks

8310849: Pattern matching for instanceof and arrayType cleanup in j.l.invoke and j.l.reflect #14642

8310849: Pattern matching for instanceof and arrayType cleanup in j.l.invoke and j.l.reflect #14642

Uh oh!

Conversation

liach commented Jun 25, 2023 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Jun 25, 2023

Uh oh!

openjdk bot commented Jun 25, 2023

Uh oh!

mlbridge bot commented Jun 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Glavo Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

Glavo Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

Glavo Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mlchung left a comment

Choose a reason for hiding this comment

Uh oh!

mlchung Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

liach Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

mlchung Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liach commented Jun 27, 2023

Uh oh!

openjdk bot commented Jun 27, 2023

Uh oh!

mlchung commented Jun 27, 2023

Uh oh!

openjdk bot commented Jun 27, 2023

Uh oh!

openjdk bot commented Jun 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

liach commented Jun 25, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Jun 25, 2023 •

edited

Loading

openjdk bot commented Jun 26, 2023 •

edited

Loading