Hi Karen,

On 21/06/2018 7:42 AM, Karen Kinnear wrote:
Looks good.

Thanks. Unfortunately there were a few more minor edits "overnight". Everything updated in place (apologies as I wanted to include a simple patch but overwrote things before realising I now had no means to do so.)

Summary:

- Class-level nest definition: use nestmates instead of members to avoid potential confusion/conflict with reference to "private members"

"A _nest_ is a set of classes and interfaces... . The classes and interfaces are known as _nestmates_. One nestmate acts as the _nest host_, and enumerates the other nestmates which belong to the nest; each of them in turn records it as the nest host. ...

- getNestHost
- replace record withenumerate when referring to nest members (members record their nest host; nest hosts enumerate their members)

-getNestMembers
  - replace @jvms ref with @see getNestHost


Can you send the updates to the valhalla spec experts please? We told them this 
was coming, and minor changes for
clarification.

I already did:

http://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2018-June/000710.html

Thanks,
David


thanks,
Karen

On Jun 19, 2018, at 12:41 AM, David Holmes <david.hol...@oracle.com> wrote:

Discussions on the CSR request have led to further changes to the documentation 
involving nests and the new nest-related method. There are no semantic changes 
here just clearer explanations.

Incremental webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v7-incr/

(don't worry if you don't see a v6, it didn't really exist).

Full webrev: http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v7/

Specdiffs updated in place at: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/specs/

Summary of changes:

- The definition of a nest etc is moved to the class-level javadoc of 
java.lang.Class, along with some other edits provided by Alex Buckley to pave 
the way for future updates
- The nest-related methods are written in a more clear and consistent way

Thanks,
David
-----

On 12/06/2018 3:16 PM, David Holmes wrote:
Here is one further minor update from the CSR discussions:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v5-incr/src/java.base/share/classes/java/lang/Class.java.cdiff.html
 Thanks,
David
On 25/05/2018 3:52 PM, David Holmes wrote:
Here are the further minor updates so far in response to all the review 
comments.

Incremental corelibs webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v3-incr/

Full corelibs webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v3/

Change summary:

src/java.base/share/classes/jdk/internal/reflect/Reflection.java
- remove inaccurate pseudo-assertion comment

test/jdk/java/lang/reflect/Nestmates/SampleNest.java
- code cleanup: <> operator

test/jdk/java/lang/reflect/Nestmates/TestReflectionAPI.java
- code cleanup: streamify duplicate removals

test/jdk/java/lang/invoke/PrivateInterfaceCall.java
- use consistent @bug number

Thanks,
David

On 22/05/2018 8:15 PM, David Holmes wrote:
Here are the updates so far in response to all the review comments.

Incremental webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v2-incr/

Full webrev: 
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v2/

Change summary:

src/java.base/share/classes/java/lang/Class.java
- getNesthost:
    - change "any error" -> "any linkage error" as runtime errors will 
propagate. [This needs ratifying by EG]
    - add clarification that primitive and array classes are not explicitly 
members of any nest and so form singleton nests
    - add clarification that all nestmates are in the same package
    - re-word @return text to exclude the "royal 'we'"
- fix javadoc cross references

---

Moved reflection API tests from 
test/hotspot/jtreg/runtime/Nestmates/reflectionAPI/ to 
test/jdk/java/lang/reflect/Nestmates/

---

java/lang/reflect/Nestmates/TestReflectionAPI.java

Run tests twice to show that failure reasons remain the same.

---

test/jdk/jdk/lambda/vm/InterfaceAccessFlagsTest.java

Disable test via annotation rather than commenting out.

---

src/java.base/share/classes/jdk/internal/reflect/Reflection.java

- Fix indent for nestmate access check.
- Remove unnecessary local variable

---

src/java.base/share/classes/sun/invoke/util/VerifyAccess.java

- Replace myassert with proper assert

---

Thanks,
David

On 15/05/2018 10:52 AM, David Holmes wrote:
This review is being spread across four groups: langtools, core-libs, hotspot 
and serviceability. This is the specific review thread for core-libs - webrev:

http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.corelibs.v1/

See below for full details - including annotated full webrev guiding the review.

The intent is to have JEP-181 targeted and integrated by the end of this month.

Thanks,
David
-----

The nestmates project (JEP-181) introduces new classfile attributes to identify 
classes and interfaces in the same nest, so that the VM can perform access 
control based on those attributes and so allow direct private access between 
nestmates without requiring javac to generate synthetic accessor methods. These 
access control changes also extend to core reflection and the 
MethodHandle.Lookup contexts.

Direct private calls between nestmates requires a more general calling context 
than is permitted by invokespecial, and so the JVMS is updated to allow, and 
javac updated to use, invokevirtual and invokeinterface for private class and 
interface method calls respectively. These changed semantics also extend to 
MethodHandle findXXX operations.

At this time we are only concerned with static nest definitions, which map to a 
top-level class/interface as the nest-host and all its nested types as 
nest-members.

Please see the JEP for further details.

JEP: https://bugs.openjdk.java.net/browse/JDK-8046171
Bug: https://bugs.openjdk.java.net/browse/JDK-8010319
CSR: https://bugs.openjdk.java.net/browse/JDK-8197445

All of the specification changes have been previously been worked out by the 
Valhalla Project Expert Group, and the implementation reviewed by the various 
contributors and discussed on the valhalla-dev mailing list.

Acknowledgments and contributions: Alex Buckley, Maurizio Cimadamore, Mandy 
Chung, Tobias Hartmann, Vladimir Ivanov, Karen Kinnear, Vladimir Kozlov, John 
Rose, Dan Smith, Serguei Spitsyn, Kumar Srinivasan

Master webrev of all changes:

http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v1/

Annotated master webrev index:

http://cr.openjdk.java.net/~dholmes/8010319-JEP181/jep181-webrev.html

Performance: this is expected to be performance neutral in a general sense. 
Benchmarking and performance runs are about to start.

Testing Discussion:
------------------

The testing for nestmates can be broken into four main groups:

-  New tests specifically related to nestmates and currently in the 
runtime/Nestmates directory

- New tests to complement existing tests by adding in testcases not previously 
expressible.
    -  For example java/lang/invoke/SpecialInterfaceCall.java tests use of 
invokespecial for private interface methods and performing receiver typechecks, 
so we add java/lang/invoke/PrivateInterfaceCall.java to do similar tests for 
invokeinterface.

-  New JVM TI tests to verify the spec changes related to nest attributes.

-  Existing tests significantly affected by the nestmates changes, primarily:
     -  runtime/SelectionResolution

     In most cases the nestmate changes makes certain invocations that were 
illegal, legal (e.g. not requiring invokespecial to invoke private interface 
methods; allowing access to private members via reflection/Methodhandles that 
were previously not allowed).

- Existing tests incidentally affected by the nestmate changes

    This includes tests of things utilising class redefinition/retransformation 
to alter nested types but which unintentionally alter nest relationships (which 
is not permitted).

There are still a number of tests problem-listed with issues filed against them 
to have them adapted to work with nestmates. Some of these are intended to be 
addressed in the short-term, while some (such as the 
runtime/SelectionResolution test changes) may not eventuate.

- https://bugs.openjdk.java.net/browse/JDK-8203033
- https://bugs.openjdk.java.net/browse/JDK-8199450
- https://bugs.openjdk.java.net/browse/JDK-8196855
- https://bugs.openjdk.java.net/browse/JDK-8194857
- https://bugs.openjdk.java.net/browse/JDK-8187655

There is also further test work still to be completed (the JNI and JDI 
invocation tests):
- https://bugs.openjdk.java.net/browse/JDK-8191117
which will continue in parallel with the main RFR.

Pre-integration Testing:
   - General:
      - Mach5: hs/jdk tier1,2
      - Mach5: hs-nightly (tiers 1 -3)
   - Targetted
     - nashorn (for asm changes)
     - hotspot: runtime/*
                serviceability/*
                compiler/*
                vmTestbase/*
     - jdk: java/lang/invoke/*
            java/lang/reflect/*
            java/lang/instrument/*
            java/lang/Class/*
            java/lang/management/*
    - langtools: tools/javac
                 tools/javap


Reply via email to