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

Code cleanup related to exception related helpers #2359

Merged
merged 1 commit into from
Jul 13, 2018

Conversation

0dvictor
Copy link
Contributor

@0dvictor 0dvictor commented Jul 9, 2018

  1. Remove following three helpers that is not in-use:
    TR_IncompatibleClassChangeError
    TR_AbstractMethodError
    TR_IllegalAccessError
  2. Consolidate TR_throwCurrentException on X86, aligning with other platforms.

Signed-off-by: Victor Ding dvictor@ca.ibm.com

1. Remove following three helpers that is not in-use:
   TR_IncompatibleClassChangeError
   TR_AbstractMethodError
   TR_IllegalAccessError
2. Consolidate TR_throwCurrentException on X86, aligning with other platforms.

Signed-off-by: Victor Ding <dvictor@ca.ibm.com>
@andrewcraik
Copy link
Contributor

is this reducing the flexibility of the codegen or implementation to be able to throw these exceptions from the JIT'd code? if so why would we want to get rid of that ability?

@0dvictor
Copy link
Contributor Author

0dvictor commented Jul 9, 2018

The 3 exception throwing helpers are never used and hence have not been tested for awhile. Currently there is no infrastructure to test any of them so that the future usability is not guaranteed.
In addition, I do not see such a need to throw any of these exceptions in the foreseeable future. Should any of them is needed in the future, it can be easily re-introduced. One should ensure the correctness when any of the helpers is re-introduced.

@andrewcraik
Copy link
Contributor

Jenkins test sanity xlinux

@andrewcraik
Copy link
Contributor

Jenkins test sanity win

@andrewcraik
Copy link
Contributor

Jenkins test sanity win32

@0dvictor
Copy link
Contributor Author

Windows failures due to #2129

@andrewcraik
Copy link
Contributor

this actually affects all platforms so I'm going to relaunch a full sanity

@andrewcraik
Copy link
Contributor

Jenkins test sanity

@andrewcraik
Copy link
Contributor

@fjeremic I think this is ok - at least for x86 - could you review for z @gita-omr FYI

@fjeremic fjeremic self-assigned this Jul 13, 2018
@fjeremic fjeremic merged commit 298d324 into eclipse-openj9:master Jul 13, 2018
@0dvictor 0dvictor deleted the exception branch July 13, 2018 14:49
@gita-omr
Copy link
Contributor

@0dvictor did you check that those helpers are not use on any platform? I just don't want to do all the search if you already done it. I think in the item like that it would be good to indicate what (if anything) other codegens should do.

@0dvictor
Copy link
Contributor Author

did you check that those helpers are not use on any platform? I just don't want to do all the search if you already done it. I think in the item like that it would be good to indicate what (if anything) other codegens should do.

Yes, I searched the three helpers from the whole OpenJ9 and OMR code, none of the them is currently in-use.

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.

4 participants