RFR 8186961 Class.getFields() does not return fields of previously visited super interfaces/classes.

[Paul Sandoz]

Hi Caes,

As we discussed off list the post loop logic is easily missed.

However, i found another obvious issue i missed with two classes (super/sub) extending from the same interface that declares a field. See updated test case in the webrev.

I have an idea to retain Field[] arrays and then process ‘em all at the end of the method to produce the final array. That should hopefully make the logic clearer too.

Paul.


> On 29 Nov 2017, at 16:00, Claes Redestad <claes.redestad at oracle.com> wrote:
> 
> Hi Paul,
> 
> it seems you'll effectively skip processing of the last interface of c in the new code - is this intentional?
> 
> 3049         Field[] iFields = null;
> 3050         for (Class<?> c : getInterfaces()) {
> 3051             if (iFields != null && iFields.length > 0) {
> ...
> 3060             }
> 3061             iFields = c.privateGetPublicFields();
> 3062         }
> 
> ifaces is unused:
> 
> 3047         Class<?>[] ifaces = getInterfaces();
> 
> Nit: You could probably simplify code by replacing List<Field> fields with the LinkedHashSet directly, removing the need to create it conditionally.
> 
> /Claes
> 
> 
> On 2017-11-29 21:15, Paul Sandoz wrote:
>> Hi,
>> 
>> Please review:
>> 
>>   http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186961-iface-static-fields-get/webrev/ <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186961-iface-static-fields-get/webrev/>
>> 
>> There is a bug lurking, perhaps for a while, where diamond patterns for interface hierarchies can result in the incorrect reporting of fields when using reflection, see the test case.
>> 
>> Since reflection data is cached i don’t see an advantage to retaining a set of of traversed interfaces, which is the root cause of the issue.
>> 
>> The code is optimized for common cases and will for less common cases collect fields of interfaces into a temporary linked hash set (to preserve the order, perhaps not strictly necessary but i did not want to change that behaviour).
>> 
>> Thanks,
>> Paul.
>