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

Fix lambda inference on openj9 VM #63

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

TwilightFlower
Copy link
Contributor

Currently, lambda generic inference (not really sure what to call it) fails on the OpenJ9 VM since Class.getConstantPool() does not exist. This PR switches to using JavaLangAccess.getConstantPool(Class), which exists on both OpenJ9 and the standard Hotspot VM, and an instance of it can be acquired via sun.misc.SharedSecrets on Java 8 and lower or jdk.internal.misc.SharedSecrets on Java 9 and higher, both of which exist on both OpenJ9 and Hotspot.

@codecov-io
Copy link

Codecov Report

Merging #63 into master will decrease coverage by 16.42%.
The diff coverage is 70%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master      #63       +/-   ##
=============================================
- Coverage     76.71%   60.29%   -16.43%     
+ Complexity      105       71       -34     
=============================================
  Files             2        2               
  Lines           335      340        +5     
  Branches         98       99        +1     
=============================================
- Hits            257      205       -52     
- Misses           34      102       +68     
+ Partials         44       33       -11
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/net/jodah/typetools/TypeResolver.java 61.27% <70%> (-18.86%) 63 <0> (-34)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0af77fe...dfb9dfb. Read the comment docs.

@jhalterman jhalterman merged commit 113a7ae into jhalterman:master Feb 26, 2020
@jhalterman jhalterman mentioned this pull request Feb 26, 2020
@jhalterman
Copy link
Owner

jhalterman commented Feb 26, 2020

@Nuclearfarts Apologies - I merged this one without realizing it caused test failures locally (though CI passed). Could you have a look again and possibly submit another PR? It's worth noting a previous, similar PR was submitted in #49 which might be a useful reference.

@TwilightFlower
Copy link
Contributor Author

Ah, I see the comments on #49 mention that SharedSecrets moved again in JDK 12. I only tested on JDK 8 and JDK 11.

@jhalterman
Copy link
Owner

I'm actually running on JDK 8 locally FWIW - I think the test failures may have to do with some of the other PRs that just merged before this (but haven't had a chance to look closely).

@TwilightFlower
Copy link
Contributor Author

Tests are all passing locally for me, on both JDK 8 and JDK 11, both HotSpot and OpenJ9. A quick check of JDK 13 showed that AccessibleObject.override is now in the reflection blacklist, so it crashes there anyway.

@jhalterman
Copy link
Owner

Were you testing with this commit rebased onto the current master?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants