On Dec 11, 2017, at 11:52 PM, David Holmes <david.hol...@oracle.com> wrote: > > I have one query with regards to the processing of the NestMembers attribute. > Originally when I wrote the draft I copied the InnerClasses text and that > included prohibiting duplicate entries in NestMembers. You dropped that > restriction from the spec (though I haven't yet removed it from the VM). How > does that interact with the Class.getNestMembers() API? Should it return the > raw NestMembers contents (which may include duplicates - including additional > references to "self") or should it return the set of members (ie no > duplicates, including self)?
My take: We have landed on laissez-faire for this kind of format check at class load time. The access checking rules are cleverly written so that garbage in the list is ignored. This is noted by the non-normative language in 4.7.29: > 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. There are four kinds of garbage: 1. [self] the nest host class itself (the one containing NestMs) 2. [outsider] valid classes whose package prefix differs form that of the nest host class 3. [duplicate] duplicate entries (either the same CONSTANT_Class or a homonym) 4. [not-found] references to classes which will not exist when resolved The spec. says these are *allowed* and *ignored*. (A fifth kind of garbage, syntax errors in class names, is caught by routine processing of CONSTANT_Class items. Maybe this is worth a unit test. The other four cases are worth a unit test, for that matter.) Each of these four cases poses a question for reflection: Do we hide it or expose it as an anomalous entry in the reflected list? In an earlier conversation, I think we agree that case 4 should just throw an error when reflection is performed, following a similar precedent with Class.getDefinedClasses. I don't remember our resolution of what to do about 1/2/3. The overall choice is to sanitize on reflection or just pass through the garbage. Given the action on 4 (just let it hit the fan), I suggest that we should also let self, outsider, and duplicate entries show through on reflection. Counterargument: We could sanitize the reflected data, and also sort or randomize its order for good measure, to prevent buggy reflection users from accidentally depending on irrelevant specifics of classfile input. Remember how we got into trouble with reflected method lists, which when they were reordered caused some buggy reflectors to break. See also iteration order of maps and ImmutableCollections.SALT. My recommendation: Just let the bad data through reflection, and rely on javac to avoid putting bad data there in the first place. The javadoc could be adorned with weasel words saying that response to bad data (of forms 1..4 above) and sequence order are undefined and may change without notice. — John