On Wed, 28 Oct 2020 08:35:47 GMT, Peter Levart <plev...@openjdk.org> wrote:

> Yes, but it is not really useful there. The super handler is captured by the 
> user invocation handler created in the invocation handler factory function 
> and it is only used by the user invocation handler. All we need in the proxy 
> instance is a boolean flag indicating the way the invocation handler was 
> created (which one of the two overloaded newProxyInstance methods was called).

Either way provides the distinction if this proxy class supports default method 
invocation.   I chose to store the superHandler rather than a boolean that may 
allow us to do white-box testing (an alternative to the trick in getting the 
superHandler in spinning another proxy instance).   It's a trivial change  if 
we prefer to store a boolean field but keep the constructor to take 
`superHandler` parameter.  I'm fine with it.

> To be precise, we need to pass CCE and NPE thrown by the default method 
> unchanged, but we need to wrap CCE and NPE thrown by the method handle 
> transformation chain with IllegalArgumentException. Is that what you are 
> referring to? 

Yes and it's implementation details.  For the specification side, I agree that 
`InvocationHandler::invoke` throws the exception by the method and no need to 
wrap it with `InvocationTargetException` .

> "Do we want to wrap CCE and NPE thrown by the method handle transformation 
> chain with IllegalArgumentException?".

I did consider that and propose to go with IAE for simplicity and consistency 
with the core reflection (`Method::invoke`, `Field::set` etc) when there is an 
illegal argument.   Is it worth specifying `CCE` and  `NPE` explicitly when an 
argument fails the conversion?   Maybe but average users of `java.lang.reflect` 
may not use `java.lang.invoke` and `IAE` is thrown for illegal argument to the 
method invocation.   When invoking a default method, `Method` object 
representing a default method is passed to `superHandler` and it's intuitive to 
consider the behavior as if `Method::invoke` is called (note that the 
specification does not mandate/expect to use `Lookup::findSpecial` and 
`MethodHandle::invoke*`).   While CCE and NPE provide slightly clearer reason 
of the illegal argument, as you said, they are subtle variants of the same 
thing indicating the parameters are wrong.  This only impacts the performance 
of the exception cases which should not be a big concern.    This explains why 
I a
 m in favor to throw IAE for illegal arguments which is the expected behavior 
for average core reflection users.  

> What to do about that. Is UncheckedReflectiveOperationException a suitable 
> solution? 

I do see your point of proposing `UncheckedReflectiveOperationException` which 
I thought about too and was tempted.  I'm uncertain yet if we want a  unchecked 
 exception type for any reflective operation exceptions.     Should future new 
`java.lang.reflect` and `java.lang.invoke` APIs use the unchecked exception?  
This is a broader question to answer than one specific exception type 
`java.lang.reflect.InaccessibleInvocationHandlerException`??

> I'm not particularly fond of creating another Proxy.invocationHandler method, 
> because it does not solve the problem of the old method that may also be 
> called.

Me neither.

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

PR: https://git.openjdk.java.net/jdk/pull/313

Reply via email to