On Wed, 18 Nov 2020 16:51:47 GMT, Paul Sandoz <[email protected]> wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix the name passed to condy calling classData
>
> src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 342:
>
>> 340: } catch (ClassCastException e) {
>> 341: throw e;
>> 342: } catch (Throwable e) {
>
> The following might be more appropriate so, in general, errors and runtime
> exceptions are not explicitly wrapped:
>
> try {
> return BootstrapMethodInvoker.widenAndCast(classdata, type);
> }
> catch (RuntimeException | Error e) {
> throw e;
> }
> catch (Throwable e) {
> throw new InternalError("Unexpected exception", e);
> }
>
> same applies to `classDataAt` and `ConstantBootstraps.explicitCast`.
> Refinement of the runtime exceptions is also possible, but i think the key
> thing here is to let errors pass through and any possibly expected runtime
> exceptions will get wrapped in `BootstrapMethodError`.
Yes a good refinement.
diff --git
a/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
b/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
index 71cae83e160..27d74284dc6 100644
--- a/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
+++ b/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
@@ -413,8 +413,8 @@ public final class ConstantBootstraps {
MethodHandle conv = MethodHandles.explicitCastArguments(id, mt);
try {
return conv.invoke(value);
- } catch (ClassCastException e) {
- throw e; // specified, let CCE through
+ } catch (RuntimeException|Error e) {
+ throw e; // let specified CCE and other runtime exceptions/errors
through
} catch (Throwable throwable) {
throw new InternalError(throwable); // Not specified, throw
InternalError
}
diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
index cd9bdbaf5a3..368948ab5a8 100644
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
@@ -337,8 +342,8 @@ public class MethodHandles {
try {
return BootstrapMethodInvoker.widenAndCast(classdata, type);
- } catch (ClassCastException e) {
- throw e;
+ } catch (RuntimeException|Error e) {
+ throw e; // let CCE and other runtime exceptions through
} catch (Throwable e) {
throw new InternalError(e);
}
@@ -409,8 +414,8 @@ public class MethodHandles {
try {
Object element = classdata.get(index);
return BootstrapMethodInvoker.widenAndCast(element, type);
- } catch
(ClassCastException|NullPointerException|IndexOutOfBoundsException e) {
- throw e;
+ } catch (RuntimeException|Error e) {
+ throw e; // let specified exceptions and other runtime
exceptions/errors through
} catch (Throwable e) {
throw new InternalError(e);
}
> test/jdk/java/lang/invoke/MethodHandles/classData/ClassDataTest.java line 77:
>
>> 75: */
>> 76: @Test
>> 77: public void noClassData() throws Throwable {
>
> `throws Throwable` needed on this and other method declarations?
`noClassData` only needs `IllegalACcessException`. `assertClassData` throws
Throwable because it unwraps from `InvocationTargetException`. I can take a
pass to clean this up further.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1171
