RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

[Langer, Christoph]

Hi Stuart,

big thanks for your great updates here. This all looks a lot cleaner: http://cr.openjdk.java.net/~clanger/webrevs/8223553.3/

So, for ConcurrentSkipListMap.java, the new coding is a real improvement, removing a @SuppressWarnings("unchecked") and a cast which are both unnecessary then.
@Martin Buchholz: What do I have to do to get this into the jsr166 integration 2019-06? Shall I open a separate bug? Or shall it be committed with the fix to JDK-8223553? Please guide me.

In the other 3 locations, we're able to eliminate the EC4J issues by introducing a local variable with the right type declaration. Sounds like a viable workaround.

At Eclipse we have the following bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=530236. It refers to the OpenJDK bug https://bugs.openjdk.java.net/browse/JDK-8016207. 

I'm wondering whether this should be submitted and I should at the same time submit another bug to evaluate these code places at a time when the final resolution for JDK-8016207 was provided?

Thank you
Christoph


> -----Original Message-----
> From: Stuart Marks <stuart.marks at oracle.com>
> Sent: Samstag, 18. Mai 2019 00:56
> To: Langer, Christoph <christoph.langer at sap.com>
> Cc: David Holmes <david.holmes at oracle.com>; Daniel Fuchs
> <daniel.fuchs at oracle.com>; core-libs-dev <core-libs-
> dev at openjdk.java.net>; net-dev <net-dev at openjdk.java.net>; compiler-
> dev at openjdk.java.net
> Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the
> Eclipse Java Compiler
> 
> Hi Christoph,
> 
> I'm still not entirely sure why this is so, but the introduction of a local
> variable in MethodHandles.java seems to make things work for Eclipse.
> Addition
> of a local variable seems to be minimally invasive, so it makes sense to see if
> this technique can be applied to other cases.
> 
> (In ConcurrentSkipListMap the issue seems to be solved by addition of
> wildcards,
> as I had suggested previously, and it cleans up the unchecked cast that was
> there in the first place. So I think that one is ok already.)
> 
> In ManagementFactory.java, an unchecked cast and warnings suppression is
> introduced, and in ExchangeImpl.java a helper method was introduced. I've
> found
> ways to introduce local variables that make Eclipse happy for these cases.
> (Well, on localized test cases; I haven't built the whole JDK with Eclipse.) See
> diffs below.
> 
> The type of the local variable in ExchangeImpl.java is a mouthful.
> Interestingly
> I had to change Function.identity() to the lambda x -> x. I think this is
> because Function.identity() returns a function that doesn't allow its return
> type to vary from its argument, whereas x -> x allows a widening conversion.
> (This might provide a clue as to the differences between javac and Eclipse
> here,
> but a full understanding eludes me.)
> 
> s'marks
> 
> 
> 
> diff -r 006dadb903ab
> src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.jav
> a
> ---
> a/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.j
> ava
> Mon May 13 17:15:56 2019 -0700
> +++
> b/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.j
> ava
> Fri May 17 15:46:14 2019 -0700
> @@ -1712,9 +1712,7 @@
>           Map<?,?> m = (Map<?,?>) o;
>           try {
>               Comparator<? super K> cmp = comparator;
> -            @SuppressWarnings("unchecked")
> -            Iterator<Map.Entry<?,?>> it =
> -                (Iterator<Map.Entry<?,?>>)m.entrySet().iterator();
> +            Iterator<? extends Map.Entry<?,?>> it = m.entrySet().iterator();
>               if (m instanceof SortedMap &&
>                   ((SortedMap<?,?>)m).comparator() == cmp) {
>                   Node<K,V> b, n;
> diff -r 006dadb903ab
> src/java.management/share/classes/java/lang/management/ManagementF
> actory.java
> ---
> a/src/java.management/share/classes/java/lang/management/Managemen
> tFactory.java
> Mon May 13 17:15:56 2019 -0700
> +++
> b/src/java.management/share/classes/java/lang/management/Managemen
> tFactory.java
> Fri May 17 15:46:14 2019 -0700
> @@ -872,12 +872,12 @@
>       public static Set<Class<? extends PlatformManagedObject>>
>              getPlatformManagementInterfaces()
>       {
> -        return platformComponents()
> +        Stream<Class<? extends PlatformManagedObject>> pmos =
> platformComponents()
>                   .stream()
>                   .flatMap(pc -> pc.mbeanInterfaces().stream())
>                   .filter(clazz ->
> PlatformManagedObject.class.isAssignableFrom(clazz))
> -                .map(clazz -> clazz.asSubclass(PlatformManagedObject.class))
> -                .collect(Collectors.toSet());
> +                .map(clazz -> clazz.asSubclass(PlatformManagedObject.class));
> +         return pmos.collect(Collectors.toSet());
>       }
> 
>       private static final String NOTIF_EMITTER =
> diff -r 006dadb903ab
> src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java
> ---
> a/src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java
> Mon May 13 17:15:56 2019 -0700
> +++
> b/src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java
> Fri May 17 15:46:14 2019 -0700
> @@ -92,8 +92,9 @@
>               CompletableFuture<Http2Connection> c2f =
> c2.getConnectionFor(request, exchange);
>               if (debug.on())
>                   debug.log("get: Trying to get HTTP/2 connection");
> -            return c2f.handle((h2c, t) -> createExchangeImpl(h2c, t, exchange,
> connection))
> -                    .thenCompose(Function.identity());
> +            CompletableFuture<CompletableFuture<? extends
> ExchangeImpl<U>>> fxi =
> +                c2f.handle((h2c, t) -> createExchangeImpl(h2c, t, exchange,
> connection));
> +            return fxi.thenCompose(x -> x);
>           }
>       }
>