Queries and patch for JDK-8034854: outer_class_info_index of synthetic class is not zero

[Alex Buckley]

Interesting reasons. Does reuse of an existing top-level or anonymous 
class choose from a pool of all classes (inc. user-defined ones) or only 
synthetic classes?

I'm still not clear if "one class is synthesized per top-level type" 
means "one member class" or "one anonymous class". I guess the 
ill-formed answer currently given by javac is "one member-anonymous 
class", because the non-zero outer_class_info_index implies it's a 
member class while the zero inner_name_index implies it's an anonymous 
class - yuk. I recommend making them truly anonymous.

Alex

On 2/20/2014 12:34 PM, Jan Lahoda wrote:
> As far as I was able to determine, these are the cases where and why the
> auxiliary classes are generated/used:
> -as tags for access constructors for private constructors. An existing
> anonymous innerclass is reused as a tag, if available, otherwise at most
> one class is synthesized per top-level type.
> -for target levels whose ldc instruction does not support references to
> classes, desugared code for class literals (a "getter" and cache for the
> Class objects) is placed into a "cache" class. Depending on the
> circumstances, a top-level class or an existing anonymous innerclass is
> reused if possible, otherwise at most one class is synthesized per
> top-level type.
> -for switch-over-enum, a lazy map between enum constants and ordinals is
> placed into a cache class. An existing anonymous innerclass is reused,
> if available, otherwise at most one class is synthesized per top-level
> type.
> -for interfaces, to hold their assertions enabled status. At most one
> class is synthesized per top-level type.
>
> Jan
>
> On 02/20/2014 08:14 PM, Alex Buckley wrote:
>> It would make sense to consider the full range of reasons why these
>> auxiliary classes are generated. You indicated one reason - tags for
>> accessing private ctors - and it makes sense to generate a "true"
>> anonymous class there (outer_class_info_index=0, inner_name_index=0).
>> But perhaps other reasons would justify auxiliary classes with
>> meaningful "owners" - again, your word - and there it would be sensible
>> to consider them as member classes rather than anonymous classes.
>>
>> Alex
>>
>> On 2/20/2014 3:11 AM, Jan Lahoda wrote:
>>> Hi Alex,
>>>
>>> Thanks for the comments.
>>>
>>> I was briefly considering filling some inner_name for the synthetic
>>> classes, but using zeroing outer_class_info_index seemed somewhat
>>> cleaner, safer (no risk of name clashes or misinterpretation of the
>>> name) and simpler. But if generating an inner_name for the synthetic
>>> classes would be (strongly) preferred, I can investigate it.
>>>
>>> Jan
>>>
>>> On 02/19/2014 08:13 PM, Alex Buckley wrote:
>>>> Hi Jan,
>>>>
>>>> The requirement that outer_class_info_index must agree with
>>>> inner_name_index w.r.t. an anonymous class was added in JVMS7
>>>> because we
>>>> saw class files where they disagreed and it simply made no sense. The
>>>> requirement was conditioned on 51.0 class files because we didn't want
>>>> to break pre-7 class files with insensible InnerClasses.
>>>>
>>>> The auxiliary classes generated by javac appear to have a meaningful
>>>> "owner" - your word - so it would seem appropriate to have a non-zero
>>>> outer_class_info_index. Just generate a random name for
>>>> inner_name_index. (The 4.7.6 text assumes the "original simple name"
>>>> can
>>>> be derived from source code, but that's not applicable for synthetic
>>>> classes.) This change could reasonably affect all target levels, since
>>>> no-one should be relying on the value of inner_name_index for these
>>>> auxiliary classes.
>>>>
>>>> OTOH, your proposal to represent the auxiliary classes as true
>>>> anonymous
>>>> classes in InnerClasses is attractive because it exposes even less
>>>> information than at present. This change could reasonably affect all
>>>> target levels too, since no-one should be relying on the value of
>>>> outer_class_info_index for these auxiliary classes.
>>>>
>>>> Alex
>>>>
>>>> On 2/19/2014 4:34 AM, Jan Lahoda wrote:
>>>>> Hello,
>>>>>
>>>>> I have a few questions about JDK-8034854 and a possible patch/fix for
>>>>> it. The bug URL:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8034854
>>>>>
>>>>> The problem is that while JVMS 7, 4.7.6. (The InnerClasses Attribute)
>>>>> mandates that:
>>>>>   If a class file has a version number that is greater than or
>>>>> equal to
>>>>>   51.0, and has an InnerClasses attribute in its attributes table,
>>>>> then
>>>>>   for all entries in the classes array of the InnerClasses attribute,
>>>>>   the value of the outer_class_info_index item must be zero if the
>>>>> value
>>>>>   of the inner_name_index item is zero.
>>>>> javac in some cases produces non-zero "outer_class_info_index" even if
>>>>> "inner_name_index" is zero. This happens for synthetically generated
>>>>> auxiliary classes. These classes are generated for a number of
>>>>> reasons,
>>>>> for example to be used as tags when accessing private constructors.
>>>>> The
>>>>> synthetic classes internally have an empty name, so the generated
>>>>> "inner_name_index" is zero, but their owner is a class, so they get
>>>>> the
>>>>> non-zero "outer_class_info_index".
>>>>>
>>>>> I've sketched out a simple fix for this problem, which ensures that
>>>>> "outer_class_info_index" is zero for classes that have empty name:
>>>>> http://cr.openjdk.java.net/~jlahoda/8034854/webrev.00/
>>>>>
>>>>> After this change, the generated synthetic classes look a lot like
>>>>> anonymous classes defined in an initializer of the given class
>>>>> (based on
>>>>> the InnerClasses attribute and the EnclosingMethod attribute). That
>>>>> seems reasonable to me.
>>>>>
>>>>> My questions are:
>>>>> -does the fix above make sense?
>>>>> -the change affects all target levels. It seems to me that the new
>>>>> behavior makes sense even for pre-7 classfiles, but I'll gladly limit
>>>>> the new behavior to only some minimal target level if desired.
>>>>>
>>>>> Any comments welcome.
>>>>>
>>>>> Thanks,
>>>>>     Jan