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

[Jan Lahoda]

Hi Vicente,

Updated patch:
http://cr.openjdk.java.net/~jlahoda/8034854/webrev.02/

Thanks for the comments,
     Jan

On 02/21/2014 06:34 PM, Vicente-Arturo Romero-Zaldivar wrote:
> 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
>