On Dec 4, 2017, at 4:32 PM, Dan Smith <daniel.sm...@oracle.com> wrote:
> 
> Actioned. Notes on nontrivial suggestions below:
> 
>> On Dec 1, 2017, at 8:21 PM, John Rose <john.r.r...@oracle.com> 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

Reply via email to