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.
s/18.3/10/ +"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 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." (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 (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.) 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." ++"5.4.4.1 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 (5.4.3.1). 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: ++"5.4.5.1 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 (5.4.5.1) to that lookup procedure. /Adresses JDK-8098577/s/Ad/Add/ +"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. Thanks for the outstanding spec. work. — John
