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

Fri Feb 21 09:34:40 PST 2014

On 21/02/14 16:29, Jan Lahoda wrote:
> 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/

Hi Jan,

I would modify the proposed test by including all the cases for which 
javac generates synthetic classes, also it would be preferable if the 
test is included in just one file.

Thanks,
Vicente

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