Final nestmates spec
john.r.rose at oracle.com
Sat Dec 2 03:21:58 UTC 2017
I read the whole thing with care. It's great; ship it.
I do have some editorial comments and nitpicks.
There's one substantive change, suggested tentatively.
+"It authorizes an enumerated set classes and interfaces to claim membership"
s/ set / set of /
I notice that we have minimized the structural requirements on nestmate
attributes are minimized. That's good, but it means there is a certain
practical uncertainty about what can be in them. For example, the NestHost
attribute can point to a crazy class like java.lang.Object, and this only gets
diagnosed during access control (5.4.4). We've agreed this is the right
thing to do. Likewise, there can be garbage in the NestMembers list,
and nothing bad will happen, at least until reflection tries to reify it.
I suggest a non-normative comment guiding users about the intended
contents of these attributes:
++"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 220.127.116.11 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 18.104.22.168."
(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.)
"• 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 (22.214.171.124)."
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.)
If new subsections are desirable, one could be introduced here, for ease of cross referencing:
+"If a referenced field or method is not accessible, access checking throws an IllegalAccessError. If an exception is thrown while attempting to determine the nest host of a class or interface, access checking fails for the same reason."
++"126.96.36.199 Nest Membership Checking"
+"To determine whether a class or interface C belongs to the same nest as D, the following steps are performed:"
This feels slightly ambiguous:
"2. Otherwise, where i is the host_class_index item of M's NestHost attribute, the symbolic reference at index i of M's run-time constant pool is resolved to a class or interface H"
Often when describing resolution it is obvious from context what is the
resolving class, but sometimes it needs to be explicitly stated. Is this
a place that needs to be more explicit? Suggest:
s/ is resolved / is resolved from M /
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.)
At this point comes the only substantive change I am tempted to suggest:
+"2. • Otherwise, where i is the host_class_index item of M's NestHost attribute, the symbolic reference at index i of M's run-time constant pool is resolved to a class or interface H (188.8.131.52). Any of the exceptions pertaining to class or interface resolution can be thrown."
s/H/H in the same runtime package as M/
s/Any/Resolution is suppressed if the symbolic reference is to a package other than that of M. Any/
+"3. • If resolution of H succeeds, but H is not declared in the same run-time package as M, an IncompatibleClassChangeErroris thrown."
s/as M/as M, or if resolution was suppressed in step 2/
I'm pretty sure we've discussed this before, and although I am
not in favor of deep checks on the new attributes, this one would
have the most benefit, IMO, since it localizes the side effects
from a bad class file to resolutions on that class file itself.
Here's another possible subsection could be introduced for cross references:
++"184.108.40.206 Method Selection"
+"Where an invokevirtual or an invokeinterface invocation refers to a resolved method mR, the selected method of the invocation for an instance of a class or interface C is determined as follows:"
(Alternatively, we could just remember, or even point out, that overriding
prepares for selection, and is useless without it.)
Later on, there are several references to "step 3 of the lookup procedure"
which might read better in the context of a sharper reference (220.127.116.11)
to that lookup procedure.
+"3. Otherwise, if there is exactly one maximally-specific method (18.104.22.168) 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 22.214.171.124 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.
Thanks for the outstanding spec. work.
More information about the valhalla-spec-observers