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