On Wed, 3 Jul 2024 19:11:34 GMT, Chen Liang <li...@openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/constant/MethodTypeDescImpl.java 
>> line 228:
>> 
>>> 226:             mtype = mt;
>>> 227:         } catch (TypeNotPresentException ex) {
>>> 228:             throw (ClassNotFoundException) ex.getCause();
>> 
>> On a side note, I wonder if it's better to re-wrap the exception here as a 
>> `ReflectiveOperationException`, instead of just getting the cause. That will 
>> retain the entire stack trace.
>
> @JornVernee Here are a few traces for comparison: 
> https://gist.github.com/5d441ab2159833e808303d1accb66ee8
> 
> In all cases, the entire stacktrace is retained; this ClassNotFoundException 
> has the `MethodTypeDescImpl::resolveConstantDesc` in its trace already.
> 
> I believe directly unwrapping the `ClassNotFoundException` is the best:
> 1. In future optimization, we can parse the individual classes more directly 
> (such as via `ClassDesc.resolveConstantDesc`) and the new code can just throw 
> the CNFE directly without extra wrapping, as user don't anticipate wrapped 
> causes.
> 2. `IllegalAccessException` throwing is done directly.
> 
> Also, would you mind to review the associated CSR as well?

Sorry, I've been out sick. Reviewed it now

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19991#discussion_r1666827723

Reply via email to