additional JSR 292 review resources

2013-09-12 Thread John Rose
P.S.  To see the proposed JSR 292 spec changes only (no code, rendered 
javadoc), see:

http://cr.openjdk.java.net/~jrose/pres/201309-jsr292/1-specdiff-meth-info-8008688
http://cr.openjdk.java.net/~jrose/pres/201309-jsr292/2-specdiff-meth-arity-8019417
http://cr.openjdk.java.net/~jrose/pres/201309-jsr292/3-specdiff-meth-spr-8001109
http://cr.openjdk.java.net/~jrose/pres/201309-jsr292/4-specdiff-meth-nsme-8001108
http://cr.openjdk.java.net/~jrose/pres/201309-jsr292/5-specdiff-meth-clinit-8024599
http://cr.openjdk.java.net/~jrose/pres/201309-jsr292/6-specdiff-meth-coll-8001110
http://cr.openjdk.java.net/~jrose/pres/201309-jsr292/7-specdiff-meth-mr-8024438

(The code changes for the last one, 8024438, are not yet ready to review.)

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

2013-09-12 Thread John Rose
On Sep 12, 2013, at 5:44 PM, David Chase  wrote:

> Do these sibling bugs have numbers?

Yes, 8022701.  I just updated the bug to explain their common genesis.

> And I take it you would like to see
> 
> 1) a test enhanced with ASM to do all 3 variants of this

Yes, please.  The case of "no such field" does not have a direct cause from 
lambda expressions AFAIK, but it can occur with "raw" CONSTANT_MethodHandles.  
(It would be reasonable for javac to emit CONSTANT_MH's for fields in some very 
limited circumstances, but I'll bet it doesn't.)

> 2) DO attach the underlying cause

Yes.  Read the javadoc for ExceptionInInitializerError for an example of why 
users want the underlying cause for linkage errors.

(Golly, I wonder what happens if resolution of a CONSTANT_MethodHandle tries to 
touch a class with a booby-trapped .  I hope it's the case that the 
ExceptionInInitializerError just passes straight up the stack.  But if it were 
to get wrapped in a ROE of some sort, it would probably bubble up as an IAE.  
Could be a charming exploration.  Actually, no, I don't want to go in there.  
Getting all possible linkage errors correct is fine, but we have more important 
things to do.)

— John

> David
> 
> On 2013-09-12, at 5:35 PM, John Rose  wrote:
> 
>> It looks good.  I have three requests.
>> 
>> Regarding this comment:
>> + * See MethodSupplier.java to see how to produce these bytes.
>> + * They encode that class, except that method m is private, not public.
>> 
>> The recipe is incomplete, since it does not say which bits to tweak to make 
>> m private.  Please add that step to the comments more explicitly, or if 
>> possible to the code (so bogusMethodSupplier is a clean copy of the od 
>> output).  I suppose you could ask sed to change the byte for you.  That will 
>> make this test a better example for future tests, and make it easier to 
>> maintain if javac output changes.  The high road to use would be asm, 
>> although for a single byte tweak od works OK.
>> 
>> Also, this bug has two twins.  The same thing will happen with 
>> NoSuchMethodE* and NoSuchFieldE*.  Can you please make fixes for those guys 
>> also?
>> 
>> FTR, see MemberName.makeAccessException() for logic which maps the other 
>> way, from *Error to *Exception.  I don't see a direct way to avoid the 
>> double mapping (Error=>Exception=>Error) when it occurs.
>> 
>> For the initCause bit we should do this:
>> 
>>   } catch (IllegalAccessException ex) {
>>   Error err = new IllegalAccessError(ex.getMessage());
>>   err.initCause(ex.getCause());  // reuse underlying cause of ex
>>   throw err;
>>   } ... and for NSME and NSFE...
>> 
>> That way users can avoid the duplicate exception but can see any underlying 
>> causes that the JVM may include.
>> 
>> Thanks for untangling this!
>> 
>> — John
>> 
>> On Sep 12, 2013, at 12:17 PM, David Chase  wrote:
>> 
>>> The test is adapted from the test in the bug report.
>>> The headline on the bug report is wrong -- because it uses reflection in 
>>> the test to do the invocation,
>>> the thrown exception is wrapped with InvocationTargetException, which is 
>>> completely correct.
>>> HOWEVER, the exception inside the wrapper is the wrong one.
>>> 
>>> The new test checks the exception in both the reflective-invocation and 
>>> plain-invocation cases,
>>> and also checks both a method handle call (which threw the wrong exception) 
>>> and a plain
>>> call (which threw, and throws, the right exception).
>>> 
>>> The test also uses a tweaked classloader to substitute the borked bytecodes 
>>> necessary to get
>>> the error, so it is not only shell-free, it can also be run outside jtreg.
>>> 
>>> On 2013-09-12, at 2:34 PM, Christian Thalinger 
>>>  wrote:
>>> 
 
 On Sep 12, 2013, at 11:28 AM, David Chase  wrote:
 
> New webrev, commented line removed:
> http://cr.openjdk.java.net/~drchase/8022701/webrev.03/
 
 I think the change is good given that the tests work now.  Is your test 
 derived from the test in the bug report?
 
 And it would be good if John could also have a quick look (just be to be 
 sure).
 
 -- Chris
 
> 
> On 2013-09-12, at 1:53 PM, David Chase  wrote:
> 
>> I believe it produced extraneous output -- it was not clear to me that 
>> it was either necessary or useful to fully describe all the converted 
>> exceptions that lead to the defined exception being thrown.  The 
>> commented line should have just been removed (I think).
>> 
>> On 2013-09-12, at 1:24 PM, Christian Thalinger 
>>  wrote:
>> 
>>> + // err.initCause(ex);
>>> 
>>> Why is this commented?
>>> 
>>> -- Chris
> 
 
>>> 
>>> ___
>>> mlvm-dev mailing list
>>> mlvm-dev@openjdk.java.net
>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>> 
> 

__

RFR (M) 8001110: method handles should have a collectArguments transform, generalizing asCollector

2013-09-12 Thread John Rose
Please review this change for a change to the JSR 292 implementation:

http://cr.openjdk.java.net/~jrose/8001110/webrev.00/

Summary: promote an existing private method; make unit tests on all argument 
positions to arity 10 with mixed types

The change is to javadoc and unit tests, documenting and testing some corner 
cases of JSR 292 APIs.

Bug Description:  Currently, a method handle can be transformed so that 
multiple arguments are collected and passed to the original method handle. 
However, the two routes to doing this are both limited to special purposes. 

(They are asCollector, which collects only trailing arguments, and only into an 
array; and foldArguments, which collects only leading arguments into filter 
function, and passes both the filtered result *and* the original arguments to 
the original.)

MethodHandles.collectArguments(mh, pos, collector) should produce a method 
handle which acts like lambda(a*, b*, c*) { x = collector(b*); return mh(a*, x, 
c*) }, where the span of arguments b* is located by pos and the arity of the 
collector.

There is internal machinery in any JSR 292 implementation to do this. It should 
be made available to users.

This is a missing part of the JSR 292 spec.

Thanks,
— John

P.S. Since this is a change which oriented toward JSR 292 functionality, the 
review request is to mlvm-dev and core-libs-dev.
Changes which are oriented toward performance will go to mlvm-dev and 
hotspot-compiler-dev.
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


RFR (S) 8024599: JSR 292 direct method handles need to respect initialization rules for static members

2013-09-12 Thread John Rose
Please review this change for a change to the JSR 292 implementation:

http://cr.openjdk.java.net/~jrose/8024599/webrev.00/

Summary: Align MH semantic with bytecode behavior of constructor and static 
member accesses, regarding  invocation.

The change is to javadoc and unit tests, documenting and testing some corner 
cases of JSR 292 APIs.

Bug Description:  When using lookupStatic to create a direct method handle for 
a static method, class initialization needs to be specified to align with the 
underly bytecode behavior of the invokestatic instruction. 

The JDK8 and 7u40 implementations do this, but the specification claims that 
the call to lookupStatic itself performs class initialization. Although this 
may have been the behavior of early versions of JDK 7, it is an error. 

Detail: 

The javadoc headers to Lookup and MethodHandle state that method handles 
emulate bytecode behaviors. The most precise specification of this emulation is 
in the JVMS, 5.4.3.5: 

> Calling this method handle on a valid set of arguments has exactly the same 
> effect and returns the same result (if any) as the corresponding bytecode 
> behavior. 

Also the doc for the "invokevirtual" instruction says: 

> ...if the method handle to be invoked has bytecode behavior, the Java Virtual 
> Machine invokes the method handle as if by execution of the bytecode behavior 
> associated with the method handle's kind. 

And the doc for the "invokestatic" instruction describes the placement of the 
lazy class initialization: 

> On successful resolution of the method, the class that declared the resolved 
> method is initialized (§5.5) if that class has not already been initialized. 

This puts the class initialization action near the border between resolution 
(link time) and execution (run time). The JDK 7 behavior acts as if the 
initialization action were part of resolution (link time), in that it claims to 
perform class initialization before the method handle is returned from 
findStatic (etc.). 

But on a closer reading of the JVMS, that is wrong. Note that errors (and 
presumably side effects) from class initialization are classified as runtime 
effects, not linktime effects: 

> Run-time Exceptions Otherwise, if execution of this invokestatic instruction 
> causes initialization of the referenced class, invokestatic may throw an 
> Error as detailed in §5.5. 

In other words, the original JDK 7 specification of eager initalization poorly 
aligns with (poorly emulates) the bytecode behavior, contrary to the overall 
pronouncements of the JSR 292 specification. 

Compatibility risk for better alignment is small, since most users of JSR 292 
create method handles lazily during BSM (i.e., invokedynamic bootstrap method) 
execution. It will not matter to them (except to reduce surprise due to 
semantic mislignment) if the initialization action is moved from the BSM to the 
first execution of the MH produced by the BSM. 

It appears that the at least one other implementation, the "JSR 292 Backport", 
performs static member class initialization on first execution of bytecode 
behavior, because it uses reflection and weaving of "*static" instructions. 
This is no accident: The coherence and usefulness of JSR 292 relies on close 
alignment of member access semantics among the various modes, direct bytecode 
execution and various old and new reflective APIs. 

Besides spec. coherence, a key problem with the "eager" class initialization 
(in findStatic) is usability. Eager class initialization is impossible to undo. 
It is also difficult to predict and avoid. Users who want it and don't have it 
can call Class.forName with an argument of 'true'. Users who don't want it and 
get it (in early JDK 7 and perhaps other systems) are out of luck.

Thanks,
— John

P.S. Since this is a change which oriented toward JSR 292 functionality, the 
review request is to mlvm-dev and core-libs-dev.
Changes which are oriented toward performance will go to mlvm-dev and 
hotspot-compiler-dev.
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


RFR (S) 8001108: an attempt to use "" as a method name should elicit NoSuchMethodException

2013-09-12 Thread John Rose
Please review this change for a change to the JSR 292 implementation:

http://cr.openjdk.java.net/~jrose/8001108/webrev.00

Summary: add an explicit check for leading "<", upgrade the unit tests

The change is mostly to javadoc and unit tests, documenting and testing some 
corner cases of JSR 292 APIs.

In the RI, there is an explicit error check.

Thanks,
— John

P.S. Since this is a change which oriented toward JSR 292 functionality, the 
review request is to mlvm-dev and core-libs-dev.
Changes which are oriented toward performance will go to mlvm-dev and 
hotspot-compiler-dev.
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


RFR (L) 8024761: JSR 292 improve performance of generic invocation

2013-09-12 Thread John Rose
Please review this change for a change to the JSR 292 implementation:

http://cr.openjdk.java.net/~jrose/8024761/webrev.00/

Bug description:  The performance of MethodHandle.invoke is very slow when the 
call site type differs from the method handle type. When the types differ, the 
invocation is defined to proceed as if two steps were taken: 

1. the target method handle is first adjusted to the call site type, by 
MethodHandles.asType 

2. the type-adjusted method handle is invoked directly, by 
MethodHandles.invokeExact 

The existing code (from JDK 7) awkwardly performs the type adjustment on every 
call. But performing the type analysis and adapter creation on every call is 
inherently slow. A good fix is to cache the result of step 1 
(MethodHandles.asType), since step 2 is already reasonably fast. 

For most applications, a one-element cache on each individual method handle is 
a reasonable choice. It has the particular advantage of speeding up invocations 
of non-varargs bootstrap methods. To benefit from this, the bootstrap methods 
themselves need to be uniquified across multiple class files, so this work will 
also include a cache to benefit commonly-used bootstrap methods, such as JDK 
8's LambdaMetafactory.metafactory. 

Additional caches could be based on the call site, the call site type, the 
target type, or the target's MH.form. 

Thanks,
— John

P.S. Since this is an implementation change oriented toward performance, the 
review request is to mlvm-dev and hotspot-compiler-dev.
Changes which are oriented toward functionality will go to mlvm-dev and 
core-libs-dev.
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


RFR (S) 8001109: arity mismatch on a call to spreader method handle should elicit IllegalArgumentException

2013-09-12 Thread John Rose
Please review this change for a change to the JSR 292 implementation:

http://cr.openjdk.java.net/~jrose/8001109/webrev.00/

The change is mostly to javadoc and unit tests, documenting and testing some 
corner cases of JSR 292 APIs.

In the RI, the exception-raising code is simplified to throw just IAE.

Thanks,
— John
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

2013-09-12 Thread David Chase
Do these sibling bugs have numbers?

And I take it you would like to see

1) a test enhanced with ASM to do all 3 variants of this
2) DO attach the underlying cause

David

On 2013-09-12, at 5:35 PM, John Rose  wrote:

> It looks good.  I have three requests.
> 
> Regarding this comment:
> + * See MethodSupplier.java to see how to produce these bytes.
> + * They encode that class, except that method m is private, not public.
> 
> The recipe is incomplete, since it does not say which bits to tweak to make m 
> private.  Please add that step to the comments more explicitly, or if 
> possible to the code (so bogusMethodSupplier is a clean copy of the od 
> output).  I suppose you could ask sed to change the byte for you.  That will 
> make this test a better example for future tests, and make it easier to 
> maintain if javac output changes.  The high road to use would be asm, 
> although for a single byte tweak od works OK.
> 
> Also, this bug has two twins.  The same thing will happen with NoSuchMethodE* 
> and NoSuchFieldE*.  Can you please make fixes for those guys also?
> 
> FTR, see MemberName.makeAccessException() for logic which maps the other way, 
> from *Error to *Exception.  I don't see a direct way to avoid the double 
> mapping (Error=>Exception=>Error) when it occurs.
> 
> For the initCause bit we should do this:
> 
>} catch (IllegalAccessException ex) {
>Error err = new IllegalAccessError(ex.getMessage());
>err.initCause(ex.getCause());  // reuse underlying cause of ex
>throw err;
>} ... and for NSME and NSFE...
> 
> That way users can avoid the duplicate exception but can see any underlying 
> causes that the JVM may include.
> 
> Thanks for untangling this!
> 
> — John
> 
> On Sep 12, 2013, at 12:17 PM, David Chase  wrote:
> 
>> The test is adapted from the test in the bug report.
>> The headline on the bug report is wrong -- because it uses reflection in the 
>> test to do the invocation,
>> the thrown exception is wrapped with InvocationTargetException, which is 
>> completely correct.
>> HOWEVER, the exception inside the wrapper is the wrong one.
>> 
>> The new test checks the exception in both the reflective-invocation and 
>> plain-invocation cases,
>> and also checks both a method handle call (which threw the wrong exception) 
>> and a plain
>> call (which threw, and throws, the right exception).
>> 
>> The test also uses a tweaked classloader to substitute the borked bytecodes 
>> necessary to get
>> the error, so it is not only shell-free, it can also be run outside jtreg.
>> 
>> On 2013-09-12, at 2:34 PM, Christian Thalinger 
>>  wrote:
>> 
>>> 
>>> On Sep 12, 2013, at 11:28 AM, David Chase  wrote:
>>> 
 New webrev, commented line removed:
 http://cr.openjdk.java.net/~drchase/8022701/webrev.03/
>>> 
>>> I think the change is good given that the tests work now.  Is your test 
>>> derived from the test in the bug report?
>>> 
>>> And it would be good if John could also have a quick look (just be to be 
>>> sure).
>>> 
>>> -- Chris
>>> 
 
 On 2013-09-12, at 1:53 PM, David Chase  wrote:
 
> I believe it produced extraneous output -- it was not clear to me that it 
> was either necessary or useful to fully describe all the converted 
> exceptions that lead to the defined exception being thrown.  The 
> commented line should have just been removed (I think).
> 
> On 2013-09-12, at 1:24 PM, Christian Thalinger 
>  wrote:
> 
>> + // err.initCause(ex);
>> 
>> Why is this commented?
>> 
>> -- Chris
 
>>> 
>> 
>> ___
>> mlvm-dev mailing list
>> mlvm-dev@openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
> 

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR (S) 8019417: JSR 292 javadoc should clarify method handle arity limits

2013-09-12 Thread Christian Thalinger

On Sep 12, 2013, at 4:34 PM, John Rose  wrote:

> Please review this change for a change to the JSR 292 implementation:
> 
> http://cr.openjdk.java.net/~jrose/8019417/webrev.00
> 
> The change is to javadoc and unit tests, documenting and testing some corner 
> cases of JSR 292 APIs.
> 
> Since the RI is already correct, there are no implementation code changes.

This looks good.  The only thing that sounds odd is "very large arity":

+ * very large arity,

-- Chris

> 
> Thanks,
> — John
> ___
> mlvm-dev mailing list
> mlvm-dev@openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


RFR (S) 8019417: JSR 292 javadoc should clarify method handle arity limits

2013-09-12 Thread John Rose
Please review this change for a change to the JSR 292 implementation:

http://cr.openjdk.java.net/~jrose/8019417/webrev.00

The change is to javadoc and unit tests, documenting and testing some corner 
cases of JSR 292 APIs.

Since the RI is already correct, there are no implementation code changes.

Thanks,
— John
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

2013-09-12 Thread John Rose
It looks good.  I have three requests.

Regarding this comment:
+ * See MethodSupplier.java to see how to produce these bytes.
+ * They encode that class, except that method m is private, not public.

The recipe is incomplete, since it does not say which bits to tweak to make m 
private.  Please add that step to the comments more explicitly, or if possible 
to the code (so bogusMethodSupplier is a clean copy of the od output).  I 
suppose you could ask sed to change the byte for you.  That will make this test 
a better example for future tests, and make it easier to maintain if javac 
output changes.  The high road to use would be asm, although for a single byte 
tweak od works OK.

Also, this bug has two twins.  The same thing will happen with NoSuchMethodE* 
and NoSuchFieldE*.  Can you please make fixes for those guys also?

FTR, see MemberName.makeAccessException() for logic which maps the other way, 
from *Error to *Exception.  I don't see a direct way to avoid the double 
mapping (Error=>Exception=>Error) when it occurs.

For the initCause bit we should do this:

} catch (IllegalAccessException ex) {
Error err = new IllegalAccessError(ex.getMessage());
err.initCause(ex.getCause());  // reuse underlying cause of ex
throw err;
} ... and for NSME and NSFE...

That way users can avoid the duplicate exception but can see any underlying 
causes that the JVM may include.

Thanks for untangling this!

— John

On Sep 12, 2013, at 12:17 PM, David Chase  wrote:

> The test is adapted from the test in the bug report.
> The headline on the bug report is wrong -- because it uses reflection in the 
> test to do the invocation,
> the thrown exception is wrapped with InvocationTargetException, which is 
> completely correct.
> HOWEVER, the exception inside the wrapper is the wrong one.
> 
> The new test checks the exception in both the reflective-invocation and 
> plain-invocation cases,
> and also checks both a method handle call (which threw the wrong exception) 
> and a plain
> call (which threw, and throws, the right exception).
> 
> The test also uses a tweaked classloader to substitute the borked bytecodes 
> necessary to get
> the error, so it is not only shell-free, it can also be run outside jtreg.
> 
> On 2013-09-12, at 2:34 PM, Christian Thalinger 
>  wrote:
> 
>> 
>> On Sep 12, 2013, at 11:28 AM, David Chase  wrote:
>> 
>>> New webrev, commented line removed:
>>> http://cr.openjdk.java.net/~drchase/8022701/webrev.03/
>> 
>> I think the change is good given that the tests work now.  Is your test 
>> derived from the test in the bug report?
>> 
>> And it would be good if John could also have a quick look (just be to be 
>> sure).
>> 
>> -- Chris
>> 
>>> 
>>> On 2013-09-12, at 1:53 PM, David Chase  wrote:
>>> 
 I believe it produced extraneous output -- it was not clear to me that it 
 was either necessary or useful to fully describe all the converted 
 exceptions that lead to the defined exception being thrown.  The commented 
 line should have just been removed (I think).
 
 On 2013-09-12, at 1:24 PM, Christian Thalinger 
  wrote:
 
> + // err.initCause(ex);
> 
> Why is this commented?
> 
> -- Chris
>>> 
>> 
> 
> ___
> mlvm-dev mailing list
> mlvm-dev@openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

2013-09-12 Thread David Chase
The test is adapted from the test in the bug report.
The headline on the bug report is wrong -- because it uses reflection in the 
test to do the invocation,
the thrown exception is wrapped with InvocationTargetException, which is 
completely correct.
HOWEVER, the exception inside the wrapper is the wrong one.

The new test checks the exception in both the reflective-invocation and 
plain-invocation cases,
and also checks both a method handle call (which threw the wrong exception) and 
a plain
call (which threw, and throws, the right exception).

The test also uses a tweaked classloader to substitute the borked bytecodes 
necessary to get
the error, so it is not only shell-free, it can also be run outside jtreg.

On 2013-09-12, at 2:34 PM, Christian Thalinger  
wrote:

> 
> On Sep 12, 2013, at 11:28 AM, David Chase  wrote:
> 
>> New webrev, commented line removed:
>> http://cr.openjdk.java.net/~drchase/8022701/webrev.03/
> 
> I think the change is good given that the tests work now.  Is your test 
> derived from the test in the bug report?
> 
> And it would be good if John could also have a quick look (just be to be 
> sure).
> 
> -- Chris
> 
>> 
>> On 2013-09-12, at 1:53 PM, David Chase  wrote:
>> 
>>> I believe it produced extraneous output -- it was not clear to me that it 
>>> was either necessary or useful to fully describe all the converted 
>>> exceptions that lead to the defined exception being thrown.  The commented 
>>> line should have just been removed (I think).
>>> 
>>> On 2013-09-12, at 1:24 PM, Christian Thalinger 
>>>  wrote:
>>> 
 + // err.initCause(ex);
 
 Why is this commented?
 
 -- Chris
>> 
> 



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

2013-09-12 Thread Christian Thalinger

On Sep 12, 2013, at 11:28 AM, David Chase  wrote:

> New webrev, commented line removed:
> http://cr.openjdk.java.net/~drchase/8022701/webrev.03/

I think the change is good given that the tests work now.  Is your test derived 
from the test in the bug report?

And it would be good if John could also have a quick look (just be to be sure).

-- Chris

> 
> On 2013-09-12, at 1:53 PM, David Chase  wrote:
> 
>> I believe it produced extraneous output -- it was not clear to me that it 
>> was either necessary or useful to fully describe all the converted 
>> exceptions that lead to the defined exception being thrown.  The commented 
>> line should have just been removed (I think).
>> 
>> On 2013-09-12, at 1:24 PM, Christian Thalinger 
>>  wrote:
>> 
>>> + // err.initCause(ex);
>>> 
>>> Why is this commented?
>>> 
>>> -- Chris
> 

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

2013-09-12 Thread Christian Thalinger
+ // err.initCause(ex);

Why is this commented?

-- Chris

On Sep 6, 2013, at 4:59 PM, David Chase  wrote:

> new, improved webrev: http://cr.openjdk.java.net/~drchase/8022701/webrev.02/
> Same small changes to the sources, plus a test.
> 
> bug: wrong exception was thrown in call of methodhandle to private method
> 
> fix: detect special case of IllegalAccessException, convert to 
> IllegalAccessError
> 
> testing:
> verified that the test would pass with modified library
> verified that the test would fail with old library
> verified that the test would fail if the class substitution (for some reason) 
> did not occur
> jtreg of jdk/test/java/lang/invoke -- note that the new exception thrown is a 
> subtype of the old one, so this is unlikely to create a new surprise
> 
> 
> ___
> mlvm-dev mailing list
> mlvm-dev@openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

2013-09-12 Thread David Chase
I believe it produced extraneous output -- it was not clear to me that it was 
either necessary or useful to fully describe all the converted exceptions that 
lead to the defined exception being thrown.  The commented line should have 
just been removed (I think).

On 2013-09-12, at 1:24 PM, Christian Thalinger  
wrote:

> + // err.initCause(ex);
> 
> Why is this commented?
> 
> -- Chris
> 
> On Sep 6, 2013, at 4:59 PM, David Chase  wrote:
> 
>> new, improved webrev: http://cr.openjdk.java.net/~drchase/8022701/webrev.02/
>> Same small changes to the sources, plus a test.
>> 
>> bug: wrong exception was thrown in call of methodhandle to private method
>> 
>> fix: detect special case of IllegalAccessException, convert to 
>> IllegalAccessError
>> 
>> testing:
>> verified that the test would pass with modified library
>> verified that the test would fail with old library
>> verified that the test would fail if the class substitution (for some 
>> reason) did not occur
>> jtreg of jdk/test/java/lang/invoke -- note that the new exception thrown is 
>> a subtype of the old one, so this is unlikely to create a new surprise
>> 
>> 
>> ___
>> mlvm-dev mailing list
>> mlvm-dev@openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
> 

___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


Re: RFR(S+M) / 8022701 Accessibility checking: InvocationTargetException is thrown instead of IllegalAccessError

2013-09-12 Thread David Chase
New webrev, commented line removed:
http://cr.openjdk.java.net/~drchase/8022701/webrev.03/

On 2013-09-12, at 1:53 PM, David Chase  wrote:

> I believe it produced extraneous output -- it was not clear to me that it was 
> either necessary or useful to fully describe all the converted exceptions 
> that lead to the defined exception being thrown.  The commented line should 
> have just been removed (I think).
> 
> On 2013-09-12, at 1:24 PM, Christian Thalinger 
>  wrote:
> 
>> + // err.initCause(ex);
>> 
>> Why is this commented?
>> 
>> -- Chris



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev