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.

Sign up for GitHub

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

Jump to bottom

Switch from using <init> to <new> for Value Types #14959

Closed
ehrenjulzert opened this issue Apr 22, 2022 · 3 comments · Fixed by #16357
Closed

Switch from using <init> to <new> for Value Types #14959

ehrenjulzert opened this issue Apr 22, 2022 · 3 comments · Fixed by #16357
Labels
comp:jit project:valhalla Used to track Project Valhalla related work

Comments

@ehrenjulzert
Copy link

Currently JavaC compiles value classes with an <init> method instead of a <new> method. Because of this, class verification also assumes that value types will always use <init> and not <new>. When JavaC switches to using <new> instead of <init>, the verification code will have to switch as well.

#14816 has code to verify value types that have both <init> and <new> methods, but the code to verify the <new> methods is currently disabled. To enable it we just need to get rid of the #if defined(J9VM_OPT_VALHALLA_NEW_FACTORY_METHOD) checks, and then get rid of the code for verifying <init> (since it will no longer be relevant).
@ehrenjulzert ehrenjulzert mentioned this issue Apr 22, 2022
Merged
@hangshao0 hangshao0 added comp:vm project:valhalla Used to track Project Valhalla related work labels Apr 22, 2022
@ehrenjulzert ehrenjulzert mentioned this issue Jun 21, 2022
Merged
@hangshao0 hangshao0 mentioned this issue Nov 9, 2022
Closed
5 tasks
@hangshao0 hangshao0 mentioned this issue Nov 22, 2022
Merged
@hzongaro
Copy link
Member

hzongaro commented Nov 22, 2022 •
edited
Loading

The JIT compiler also has some special handling to allow for the possibility that a method named <init> might not be a constructor if value types are enabled. It will no longer be relevant once <vnew> is introduced. I'll start on a pull request for that.

Opened draft pull request #16357

ehrenjulzert pushed a commit to ehrenjulzert/openj9 that referenced this issue Nov 29, 2022
@hangshao0
Update the verifier to recognize new constructor <vnew> of value type
c3fd072 
1. The VT constructors are compiled into <vnew>. Update all places using
<new> to <vnew>.
1. Enable all the code inside J9VM_OPT_VALHALLA_NEW_FACTORY_METHOD.
2. Remove obsolete code that checks <init> as VT constructor.

Closes eclipse-openj9#14959

Signed-off-by: Hang Shao <[email protected]>
@tajila tajila closed this as completed in fc4662c Nov 30, 2022
@hangshao0
Copy link
Contributor

The VM PR is merged.

@hangshao0 hangshao0 removed the comp:vm label Nov 30, 2022
@hangshao0
Copy link
Contributor

Re-open this issue as the JIT PR is still open.

@hangshao0 hangshao0 reopened this Nov 30, 2022
@hzongaro hzongaro mentioned this issue Dec 6, 2022
Merged
@hangshao0 hangshao0 mentioned this issue Aug 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
comp:jit project:valhalla Used to track Project Valhalla related work
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove special treatment of <init> for value types hzongaro/openj9
3 participants
@hangshao0 @hzongaro @ehrenjulzert