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

[Jan Lahoda]

On 02/20/2014 10:06 PM, Alex Buckley wrote:
> 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?

User-defined anonymous classes are reused for private constructor tags 
if available, but, on a closer look, user-defined anonymous classes 
don't seem to be reused in other cases. User-defined top-level classes 
(if available) appear to be reused for targets <= 1.4 to hold the 
desugared code for class literals.

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

Internally, they look somewhat like a member class with an empty name 
("anonymous member class"). But I don't think the internal 
representation should affect the content of the attributes in this case.

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

Thanks.

A webrev that updates tests according to comments received so far:
http://cr.openjdk.java.net/~jlahoda/8034854/webrev.01/

Jan

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