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.
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?
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.
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.
Mandy
