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

Properly initalize superclass of IlGeneratorMethodDetails #16

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

mgaudet
Copy link
Contributor

@mgaudet mgaudet commented Sep 14, 2017

Currently we don't properly IlGeneratorMethodDetails, however that
hasn't harmed us because it has no state. However, I'm trying to add a
field to the OMR::IlGeneratorMethodDetails which will require it to be
initalized, so this commit adds the required call to the parent
constructor.

Signed-off-by: Matthew Gaudet [email protected]

@lmaisons
Copy link
Contributor

lmaisons commented Sep 14, 2017

I take it you will eventually need it to do something other than default construct its parent? Because it should be doing that automatically anyway.

@mgaudet
Copy link
Contributor Author

mgaudet commented Sep 14, 2017

Hmm. If that's true, I could withdraw this. I wasn't 100% sure that was the case, and was leaning towards Explicit > Implicit.

I'm OK either way then

@lmaisons
Copy link
Contributor

I'm all for changing magic into prescription

@genie-openj9
Copy link

Can one of the admins verify this patch?

2 similar comments
@genie-openj9
Copy link

Can one of the admins verify this patch?

@genie-openj9
Copy link

Can one of the admins verify this patch?

@mstoodle
Copy link
Contributor

mstoodle commented Oct 4, 2017

Jenkins test sanity

@mstoodle
Copy link
Contributor

mstoodle commented Oct 4, 2017

I prefer to make it explicit since this little corner of class hierarchy is not really straight-forward.

Copy link
Contributor

@mstoodle mstoodle left a comment

Choose a reason for hiding this comment

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

Please update the copyright date to 2017 and then this can be merged.

Currently we don't properly IlGeneratorMethodDetails, however that
hasn't harmed us because it has no state. However, I'm trying to add a
field to the OMR::IlGeneratorMethodDetails which will require it to be
initalized, so this commit adds the required call to the parent
constructor.

Signed-off-by: Matthew Gaudet <[email protected]>
@mgaudet mgaudet force-pushed the InitializeMethodDetails branch from b6f344b to e5da320 Compare October 4, 2017 17:19
@mgaudet
Copy link
Contributor Author

mgaudet commented Oct 4, 2017

Rebased, corrected copyright.

Copy link
Contributor

@mstoodle mstoodle left a comment

Choose a reason for hiding this comment

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

Ok build on Travis shows failure but it's a bogus failure and it passed earlier (before the change to the copyright). I'm just going to merge.

Thanks @mgaudet

@mstoodle
Copy link
Contributor

mstoodle commented Oct 5, 2017

Ok I cannot merge from a phone due to the failure (thanks github) so if any other committer is willing to push the button please do.

@mstoodle mstoodle merged commit 9b0546a into eclipse-openj9:master Oct 5, 2017
tajila pushed a commit to tajila/openj9 that referenced this pull request Jul 11, 2019
* Place JVMImage instance into JVMImagePortLibrary
- created wrapper around omr port library and added jvmimage
- placed pointer to wrapper in J9JavaVM
- changed all functions to rely on vm reference or jvmimageportlibrary

Signed-off-by: akshayben [email protected]
hzongaro pushed a commit to hzongaro/openj9 that referenced this pull request Mar 26, 2020
…sandbox

Implment monitor enter/exit for valuetype
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants