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.

Reply via email to