Here is the updated webrev:
http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186961-iface-static-fields-get/webrev/
I opted for the simple solution using a LinkedHashSet.
It’s possible to heavily optimize (avoiding the production of a linked hash set
until required [*]) but the resulting code is harder to understand.
Paul.
[*] when there are two or more super interface with fields, or one or more
super interfaces and a super classes with fields.
> On 30 Nov 2017, at 08:41, Paul Sandoz <paul.san...@oracle.com> wrote:
>
> 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.
>>
>
