Translate TypeNotPresentException to NoClassDefFoundError #19
Conversation
| try { | ||
| type = MethodType.fromMethodDescriptorString(methodDescriptor, VM.getVMLangAccess().getClassloader(clazz)); | ||
| } catch (TypeNotPresentException e) { | ||
| throwNoClassDefFoundError(e); |
There was a problem hiding this comment.
One way to make it easier to reason about this code is to write the call as:
throw throwNoClassDefFoundError(e);
so that readers know that an exception is guaranteed to be thrown here and there is no path to continue to the code below.
| try { | ||
| cpEntry = getCPMethodTypeAt(clazz, index); | ||
| } catch (TypeNotPresentException e) { | ||
| throwNoClassDefFoundError(e); |
There was a problem hiding this comment.
Same here throw throwNoClassDefFoundError(e);
| * Throw NoClassDefFoundError if the cause of TypeNotPresentException is | ||
| * ClassNotFoundException. Otherwise, re-throw TypeNotPresentException. | ||
| */ | ||
| private static void throwNoClassDefFoundError(TypeNotPresentException e) { |
There was a problem hiding this comment.
And this should be declared as:
private static Throwable throwNoClassDefFoundError(TypeNotPresentException e) {
Even though its declared to return the Throwable, it should still throw the exception itself. This is just a pattern to make it easier for the caller to make it explicit that there is no fall through case
| NoClassDefFoundError noClassDefFoundError = new NoClassDefFoundError(cause.getMessage()); | ||
| noClassDefFoundError.initCause(cause); | ||
| throw noClassDefFoundError; | ||
| } else { |
There was a problem hiding this comment.
Since the if block unconditionally throws, the else isn't necessary. The throw e can follow the end of the if block. This makes again makes it clearer that there is no fall through path here.
babsingh
force-pushed
the
jcl_tnpe_to_ncdfe
branch
2 times, most recently
from
05a3122 to
2cf2c88
Compare
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
babsingh
force-pushed
the
jcl_tnpe_to_ncdfe
branch
from
2cf2c88 to
5da4014
Compare
babsingh
changed the title
| * Throw NoClassDefFoundError if the cause of TypeNotPresentException is | ||
| * ClassNotFoundException. Otherwise, re-throw TypeNotPresentException. | ||
| */ | ||
| private static Throwable noClassDefFoundError(TypeNotPresentException e) { |
There was a problem hiding this comment.
Can you add proper javadoc for this method? Also, to match the naming convention for this kind of method, can you prepend it with throw aka throwNoClassDefFoundError(...)?
babsingh
force-pushed
the
jcl_tnpe_to_ncdfe
branch
3 times, most recently
from
b707b10 to
8473390
Compare
babsingh
changed the title
While resolving bootstrap method handles, TypeNotPresentException is currently thrown if a class is not found since we rely upon the MethodType API. But, NoClassDefFoundError should be thrown if a class is not found while resolving bootstrap method handles. This change translates TypeNotPresentException to NoClassDefFoundError while resolving bootstrap method handles. Signed-off-by: Babneet Singh <[email protected]>
babsingh
force-pushed
the
jcl_tnpe_to_ncdfe
branch
from
8473390 to
70a939e
Compare
|
Jenkins test sanity |
Allow Object methods in invokeinterface
- segment memory allocation and free - no support for compressed refs - bug fixes for jvmimageport functions (imem) - increased heap size Signed-off-by: akshayben <[email protected]>
* Register Class into Table (eclipse-openj9#23) - register performed in createramclass - added register functions into internalfuncs - changed naming scheme Signed-off-by: akshayben <[email protected]>
…type-queries Switch to value type query methods from ClassEnv
While resolving bootstrap method handles, TypeNotPresentException is
currently thrown if a class is not found since we rely upon the
MethodType API. But, NoClassDefFoundError should be thrown if a class
is not found while resolving bootstrap method handles. This change
translates TypeNotPresentException to NoClassDefFoundError while
resolving bootstrap method handles.
Signed-off-by: Babneet Singh [email protected]