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:

Under `NestHost`:

**`host_class_index`**

: **The value of the `host_class_index` item must be a valid index into the 
`constant_pool` table. The `constant_pool` entry at that index must be a 
`CONSTANT_Class_info` structure ([4.4.1]) representing a class or interface.**

    > **If the referenced class cannot be loaded, does not belong to the same 
run-time package, or does not authorize nest membership, an error may occur 
during access checking ([5.4.4]).**

And under `NestMembers`:

**`classes[]`**

: **Each value in the `classes` array must be a valid index into the 
`constant_pool` table. The `constant_pool` entry at that index must be a 
`CONSTANT_Class_info` structure ([4.4.1]) representing a class or interface.**

    > **This array is consulted during access checking ([5.4.4]). It should 
consist of references to other classes and interfaces that belong to the same 
run-time package and have `NestHost` attributes referencing this class or 
interface. Items that do not meet this description are discouraged and will be 
ignored by access checking.**

> (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.

> "• R is protected and is declared in a class C, and D is either a subclass of 
> C or C itself. Furthermore, if R is not static, then the symbolic reference 
> to R must contain a symbolic reference to a class T, such that T is either a 
> subclass of D, a superclass of D, or D itself."
> + "During verification, it was also required that, even if T is a superclass 
> of D, the target reference of a protectedinstance field access or method 
> invocation must be an instance of D or a subclass of D (4.10.1.8)."
> 
> I'm glad to see this addition, even if it's non-normative.  The web of
> checks that collectively enforce protected access is not well explained
> in the JVMS, because they are scattered in several places.
> (See also my separate message on stricter symbolic references.)

It's not new, but I rearranged and rephrased it in a way that is hopefully 
clearer. :-)

> ++"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...

> Or is there a place elsewhere that makes it really clear that a symbolic
> reference in a CP of M is always resolved relative to M?  (Such questions
> might become more urgent when we have disembodied symbolic
> references created by the Constable API.)

It is always the case, yes, and I don't think the spec does anything to suggest 
that this is even a well-formed question. That is, symbolic references can only 
be resolved one way, and the class in which they occur seems to be an implicit 
part of them. For example, 'checkcast' invokes resolve(ref), which immediately 
starts talking about 'D', the class in which the reference occurs (5.4.3.1).

> ++"5.4.5.1 Method Selection"

Noted.

> +"3. Otherwise, if there is exactly one maximally-specific method (5.4.3.3) 
> in the superinterfaces of C that matches the resolved method's name and 
> descriptor and is not abstract, then it is the selected method."
> There's no use of the concept "can override" in point 3, which is the only 
> place where selection
> of super-interface methods occurs.  I assume that this is because the 
> can-override
> relation is not relevant at this point, but it feels like a narrative loose 
> end.
> 
> 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.

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

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

—Dan

Reply via email to