Hi Mandy,
On 22/05/2018 1:23 PM, mandy chung wrote:
Hi David,
I reviewed:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v1/
Looks good in general. I have a couple other comments. I will review
the new version that includes new tests in test/jdk/java/lang/reflect
when it's ready.
Thanks! That should be out later today.
Class.java
3988 Class<?>[] members = getNestMembers0();
If the above fails with any LinkageError, what is the expected behavior
if Class::getNestMembers() is called the second time? will it throw the
same LinkageError?
Based on the implementation it should throw the same error. We use
constant-pool resolution to locate the member classes so if resolution
fails then it must continue to fail for the same reason per the JVMS rules.
Are you thinking that getNestMembers() should say something about this?
On 5/20/18 10:57 PM, David Holmes wrote:
I think it's okay in VerifyAccess too but the "FIX ME" suggests there
are doubts so I assume this needs more eyes to triple check.
The "FIX ME" is not about the access check but the form of the
assertion. The assert is verification that resolution of a private
method always leads to selection of that private method: refc == defc.
I used "myassert" so that this check was always enabled during
testing. The "FIX ME" was to either convert to language assert
statement or else remove it (having validated that it never fires).
Obviously "myassert" has not fired in all the testing that I have done
so either choice seems fine. Do you have a preference?
Since you have the assert, I would convert it to language assert that
would help the readers to understand the result of such resolution.
Will do.
Regarding Alan's and Peter's comment on the spec clarification on
primitive type and array type, it is reasonable to take a pass on all
reflection APIs and make the text explicit consistently if this class is
a primitive type and array type. I will file an issue to track this.
Class::getModifiers does return the modifier of the component type
whereas Class::getEnclosingClass returns null if this class is an array
type even if the component type has an enclosing class. Clarification
would help developers.
Thanks. I will add the small clarification that Alan requested on
getNestHost().
Thanks,
David
Mandy
