Re: Final nestmates spec

[John Rose]

On Dec 4, 2017, at 4:32 PM, Dan Smith  wrote:
> 
> Actioned. Notes on nontrivial suggestions below:
> 
>> On Dec 1, 2017, at 8:21 PM, John Rose  wrote:
>> 
>> ++"The NestHost and NestMembers attributes of a class D are used
>> to make symbolic references to classes and interfaces in the same
>> run-time package as D.  References to other types are ineffective under
>> the rules of 5.4.1.1 and should not be introduced. Similarly, a nest host
>> H should not list itself in its NestMembers attribute, although this
>> will have no effect under the rules of 5.4.1.1."
> 
> Good suggestion, but where to put it? I ended up with this:
> … 


That's great, much better than my suggested wording; thanks!

>> (The rules call for loading the NestHost H even if it is not in the same
>> package as the referring class M.  This is the only substantive thing
>> I feel slightly uncomfortable about in the spec., and it's not enough
>> to demand a change.  But it's there.)
> 
> Yes. Discussed this with David and Alex, and we tried a few different 
> iterations before settling on this. A key observation is that, if we eagerly 
> detect that the class can't possibly be a packagemate, we can quickly return 
> "false", but then an IAE will occur. It's important to avoid loading in the 
> fast/common path, but when you're throwing errors you're in the 
> slow/exceptional path. So the extra complexity of a special-case rule doesn't 
> seem justified.
> 
> Stepping back, I asked myself the question, "what good would an extra 
> pre-check do?", and couldn't come up with anything. It's not like the ability 
> to ask to load classes is a closely-guarded right.

Yes.  If I load a class Foo and it has a NestHost of Bar, then I might have to 
load Bar, but there are many other ways that loading Foo might trigger a load 
of Bar.  And, loading Foo and then Foo's NestHost can only be triggered in two 
circumstances:  Foo is originating a symbolic reference to a private thing 
(maybe in Bar or maybe not), or a private member in Foo is the target of a 
symbolic reference (maybe in Bar's nest or maybe not).  In both cases, both 
origin and target classes are already loaded and are fully competent to 
authorize loading of their respective NestHosts.  The loading of the NestHost 
is a new thing, but not any newer than other "on the fly" loading of related 
classes.  So I don't see any opportunity for trickery here.

> ...
>> ++"5.4.4.1 Nest Membership Checking"
> 
> I'll add a note and leave it to Alex to decide. I think he may not like 
> singleton subsections…

Either way is fine with me.
>> ...
>> If I read the logic right, the only way to select a method here is
>> if the symbolic reference already resolved to a public interface
>> method, so the can-override relation is actually true, regardless
>> of what the 5.4.3.3 process produces.  But if it's true, the proof
>> is subtle and non-local.  I would like to be assured of this fact
>> in the spec. itself.  Failing that, I think there is room for future
>> improvement at this point, by saying that the can-override
>> relation *is enforced* at step 3, and allow JVMs to prove that
>> the enforcement is a nop.
> 
> We have the following two functions:
> canOverride(m1, m2)
> maximallySpecific(C, name, desc)
> 
> The latter, by design, produces a set of methods that can override m2 (the 
> resolved method). Once you have that set, there's no need to assert 
> canOverride on its elements.
> 
> How does 'maximallySpecific' have this property? Because m2 is not private 
> (that's an earlier case in selection), meaning it's public, and we search for 
> non-private methods with the same name and descriptor.

Good, that's the way I read the logic.  (I'm relieved; I still understand the 
spec!)

> If we had protected or package interface methods, then we'd need something to 
> consider accessibility, like
> maximallySpecificOverrides(C, m2)

And that's the "non-local" part I referred to, that made me a little 
uncomfortable.
MaximallySpecific happens to include canOverride as a corollary, but only 
indirectly.
So if we make a non-local change to interface methods, then this logic may grow 
a bug.

> But, for now, it works to re-use the 'maximallySpecific' used by resolution.


IMO a non-normative observation of this subtle corollary would be helpful.
The spec. is full of such subtleties, but sometimes–as with the cross references
regarding protected access checks–it's good to shine a little light on them.

— John

Re: Final nestmates spec

[Karen Kinnear]

Dan,

Many thanks for the careful updates and working so closely with all of us with 
each
iteration.

Couple of minor comments:

1. Since we postponed this, and each release will change the ClassFile Version:
   - the new attributes will be in ClassFile Version 55
   - If we want to use the new versioning, that would be 18.9 (rather than 11)

2. 3.7 Code example update - there is an invokespecial #4 // Method 
Near.getItNear()I
I don’t think getItNear exists in the green example. I assume you meant 
Near.getIt().

3. 5.4.2. Preparation:
2nd changed paragraph - 
The loader constraint equation below compares types relative to L2 (which is 
the class loader for the selected method) vs. L3. I do not see L3 defined here.
I believe that  is intended to be the super interface which declares the 
method in the first part of the sentence.

4. 5.4.4 I like the improvements suggested in this email thread clarifying that 
a host_class_index refers to a Constant_Class. 
And thank you for the future reminder of the circularity issue if we allow 
classes to be private.

5.5.4.5 Thank you for the example and for sweating the details. As an 
implementor, I very much like having all of this information in one place in 
the JVMS.

6. 5.4.5 “the selected method of the invocation for an instance of a class or 
interface C”
For invokeinterface and invoke virtual “C” is always an object ref which must 
always be a class
not an interface.

Assuming you might be looking ahead to having this also work for invoke special 
when we get rid
of ACC_SUPER? Otherwise I think this might work better with “class C”.

thanks,
Karen


> On Dec 1, 2017, at 7:52 PM, John Rose  wrote:
> 
> On Dec 1, 2017, at 4:41 PM, Dan Smith  > wrote:
>> 
>> I've uploaded what I believe could be a final spec for nestmates, here:
>> http://cr.openjdk.java.net/~dlsmith/nestmates.html 
>> 
>> 
>> Since October, changes include:
>> 
>> - Tweaks to the explanatory material in 2.11.8 and 3.7
>> 
>> - Some restructuring of the new 5.4.4 text to be a little less disruptive 
>> and more clearly define "same nest" and "nest host". There are some subtle 
>> behavioral differences compared to the previous iteration when errors occur.
>> 
>> - Added an example to 5.4.5 to illustrate transitive overriding
>> 
>> —Dan
> 
> I suggest changing the references to 18.3 to 10, based on today's information 
> copied below.
> 
> — John
> 
> 
> http://mail.openjdk.java.net/pipermail/jdk-dev/2017-December/000301.html 
> 
> 
> Begin forwarded message:
>> 
>> From: mark.reinh...@oracle.com 
>> Subject: Re: Draft JEP: Time-Based Release Versioning
>> Date: December 1, 2017 at 7:59:47 AM PST
>> To: volker.simo...@gmail.com 
>> Cc: jdk-...@openjdk.java.net 
>> 
>> 2017/12/1 3:11:18 -0800, volker.simo...@gmail.com 
>> :
>>> thanks for publishing the draft. Overall it looks good!
>>> 
>>> I have just a few comment :)
>>> 
>>> 1. Is thisJEP (i.e. the new version scheme) intended to be targeted
>>> for Java 10?
>> 
>> It's intended to be targeted when it's ready, just like any other JEP.
>> It's obviously desirable to have it in 10, but if it doesn't make it
>> then it'll be in 11.
>> 
>>> I would appreciate to have it in ten but aren't we already quite late?
>>> This is specification relevant (i.e. has to go into JSR 383) because
>>> it changes java.lang.Runtime.Version and various standard system
>>> properties which refer to it. JSR 383 has to be renamed from "Java SE
>>> 18.3 Platform JSR" to "Java SE 10 Platform JSR" afterwards, right?
>> 
>> That should be done anyway, regardless of whether this JEP makes it into
>> JDK 10.  If we change nothing in JDK 10, as it stands today, then the
>> JDK 10 GA release will identify itself as 10, not 18.3.
>> 
>>