RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
> Overall, introducing a local variable is more-or-less reasonable even if it's > used only once. One point is that somebody might come along and "clean > up" the > code and inline local variables and reintroduce the problem. Another point is > that, hypothetically, if in the future Eclipse is changed to match javac's > behavior, these changes should be reverted. > > The bug database is a good place to capture actions that need to take place in > the future. Unfortunately, it's pretty far from these locations, so the > existence of such a bug wouldn't prevent the accidental cleanup from > happening. > That would indicate having a comment in the code at these locations. > > On the other hand, if Eclipse is "fixed" and these locations don't get cleaned > up for a long time, it doesn't seem like that big a deal. > > I'd suggest to put a comment at these 3 locations, something like: > > // local variable required here; see JDK-8223553 > > and not bother with filing another bug report to track this. Ok, good idea. I'll add the comment before I push. > I'll defer to Martin as to how he wants to handle the > ConcurrentSkipListMap.java > change. As Martin has taken this to the jsr166 integration 2019-06, I'll push the change without ConcurrentSkipListMap.java tomorrow. Thanks to all involved in this review! /Christoph
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
On 5/23/19 6:39 AM, Langer, Christoph wrote: big thanks for your great updates here. This all looks a lot cleaner: http://cr.openjdk.java.net/~clanger/webrevs/8223553.3/ Great, glad to help. While I'm still unsure about the underlying reasons for this disagreement between the compilers, that might take a while to resolve. Putting in these fixes isn't very intrusive and it seems to make people's lives easier, so I don't have any objection to them going in. 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? Overall, introducing a local variable is more-or-less reasonable even if it's used only once. One point is that somebody might come along and "clean up" the code and inline local variables and reintroduce the problem. Another point is that, hypothetically, if in the future Eclipse is changed to match javac's behavior, these changes should be reverted. The bug database is a good place to capture actions that need to take place in the future. Unfortunately, it's pretty far from these locations, so the existence of such a bug wouldn't prevent the accidental cleanup from happening. That would indicate having a comment in the code at these locations. On the other hand, if Eclipse is "fixed" and these locations don't get cleaned up for a long time, it doesn't seem like that big a deal. I'd suggest to put a comment at these 3 locations, something like: // local variable required here; see JDK-8223553 and not bother with filing another bug report to track this. I'll defer to Martin as to how he wants to handle the ConcurrentSkipListMap.java change. s'marks
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
On 23/05/2019 14:39, Langer, Christoph wrote: big thanks for your great updates here. This all looks a lot cleaner:http://cr.openjdk.java.net/~clanger/webrevs/8223553.3/ I concur. Thanks Stuart! best regards, -- daniel
RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
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
> Sent: Samstag, 18. Mai 2019 00:56
> To: Langer, Christoph
> Cc: David Holmes ; Daniel Fuchs
> ; core-libs-dev d...@openjdk.java.net>; net-dev ; compiler-
> d...@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 cmp = comparator;
> -@SuppressWarnings("unchecked")
> -Iterator> it =
> -(Iterator>)m.entrySet().iterator();
> +Iterator> it = m.entrySet().iterator();
> if (m instanceof SortedMap &&
> ((SortedMap)m).comparator() == cmp) {
> Node 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>
> getPlatformManagementInterfaces()
> {
> -return platformComponents()
> +Stream> 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 =
>
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.java
---
a/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
Mon May 13 17:15:56 2019 -0700
+++
b/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
Fri May 17 15:46:14 2019 -0700
@@ -1712,9 +1712,7 @@
Map m = (Map) o;
try {
Comparator cmp = comparator;
-@SuppressWarnings("unchecked")
-Iterator> it =
-(Iterator>)m.entrySet().iterator();
+Iterator> it = m.entrySet().iterator();
if (m instanceof SortedMap &&
((SortedMap)m).comparator() == cmp) {
Node b, n;
diff -r 006dadb903ab
src/java.management/share/classes/java/lang/management/ManagementFactory.java
---
a/src/java.management/share/classes/java/lang/management/ManagementFactory.java
Mon May 13 17:15:56 2019 -0700
+++
b/src/java.management/share/classes/java/lang/management/ManagementFactory.java
Fri May 17 15:46:14 2019 -0700
@@ -872,12 +872,12 @@
public static Set>
getPlatformManagementInterfaces()
{
-return platformComponents()
+Stream> 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 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>>
fxi =
+c2f.handle((h2c, t) -> createExchangeImpl(h2c, t, exchange,
connection));
+return fxi.thenCompose(x -> x);
}
}
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Stuart's cool type system hack is hard for me to grok, but it seems alright
to put into ConcurrentSkipListMap.java. We could add it to the current
jsr166 integration.
*From: *Stuart Marks
*Date: *Thu, May 16, 2019 at 3:11 PM
*To: *Martin Buchholz, David Holmes, Doug Lea, Langer, Christoph
*Cc: *compiler-...@openjdk.java.net, core-libs-dev, net-dev
On 5/14/19 9:16 PM, Martin Buchholz wrote:
> >>
> src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
>
> Regarding the change in this particular file, I'd suggest the following:
>
>
> diff -r 006dadb903ab -r 92e1fdce45e0
> src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
>
> Mon May 13 17:15:56 2019 -0700
> +++
> b/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
>
> Thu May 16 15:04:30 2019 -0700
> @@ -1712,9 +1712,7 @@
> Map m = (Map) o;
> try {
> Comparator cmp = comparator;
> -@SuppressWarnings("unchecked")
> -Iterator> it =
> -(Iterator>)m.entrySet().iterator();
> +Iterator> it =
> m.entrySet().iterator();
> if (m instanceof SortedMap &&
> ((SortedMap)m).comparator() == cmp) {
> Node b, n;
>
>
>
> I have no idea if it will be accepted by the Eclipse IDE, but this seems
> like a
> cleaner change to me than introducing a raw type.
>
> ***
>
> Christoph,
>
> On the general issue of compilation differences between javac and the
> Eclipse
> IDE, have you raised a bug with them?
>
> s'marks
>
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
On 5/14/19 9:16 PM, Martin Buchholz wrote:
src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
Regarding the change in this particular file, I'd suggest the following:
diff -r 006dadb903ab -r 92e1fdce45e0
src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
---
a/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
Mon May 13 17:15:56 2019 -0700
+++
b/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
Thu May 16 15:04:30 2019 -0700
@@ -1712,9 +1712,7 @@
Map m = (Map) o;
try {
Comparator cmp = comparator;
-@SuppressWarnings("unchecked")
-Iterator> it =
-(Iterator>)m.entrySet().iterator();
+Iterator> it = m.entrySet().iterator();
if (m instanceof SortedMap &&
((SortedMap)m).comparator() == cmp) {
Node b, n;
I have no idea if it will be accepted by the Eclipse IDE, but this seems like a
cleaner change to me than introducing a raw type.
***
Christoph,
On the general issue of compilation differences between javac and the Eclipse
IDE, have you raised a bug with them?
s'marks
RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi David, Martin, thanks for looking into this. Generally I share your view on this. It's not nice at all. However, it's the only way I can see currently to get rid of the Errors in the Eclipse IDE. Maybe an idea would be to get this in but at the same time open a ticket to evaluate this code again from a compiler/spec perspective and make the according modifications? Thanks Christoph > -Original Message- > From: David Holmes > Sent: Mittwoch, 15. Mai 2019 00:05 > To: Langer, Christoph ; Daniel Fuchs > ; core-libs-dev d...@openjdk.java.net>; net-dev > Cc: compiler-...@openjdk.java.net; Martin Buchholz > > Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the > Eclipse Java Compiler > > Hi Christoph, > > I'm very reluctant to see changes like this that the compiler folk have > not determined are actually incorrect. That said ... > > On 15/05/2019 7:03 am, Langer, Christoph wrote: > > Thanks Daniel. > > > > Can anybody help reviewing the changes to: > > src/java.base/share/classes/java/lang/invoke/MethodHandles.java > > The introduction of the intermediate local variable seems harmless > (though why it should be necessary is another matter). > > > > src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.jav > a > > As you note this should be ok'd by jsr166 folk so I've cc'd Martin > Buccholz. I dislike seeing a raw type introduced here though. > > > and > > > src/java.management/share/classes/java/lang/management/ManagementF > actory.java ? > > Introducing an unchecked cast seems very crude. I'd want the core-libs > stream experts to comment on this. > > Cheers, > David > > > > Thanks > > Christoph > > > >> -Original Message----- > >> From: Daniel Fuchs > >> Sent: Dienstag, 14. Mai 2019 18:04 > >> To: Langer, Christoph ; core-libs-dev libs- > >> d...@openjdk.java.net>; net-dev > >> Cc: compiler-...@openjdk.java.net > >> Subject: Re: RFR: 8223553: Fix code constructs that do not compile with > the > >> Eclipse Java Compiler > >> > >> Hi Christoph, > >> > >> That looks much better, thanks! > >> (but still not commenting on the other changes ;-)) > >> > >> best regards, > >> > >> -- daniel > >> > >> On 14/05/2019 13:57, Langer, Christoph wrote: > >>> Hi Daniel, > >>> > >>>>> unfortunately, your proposed solution does not work with javac. I get > >> this > >>>> in the build: > >>>> > >>>> Oh darn. I should have double checked. > >>>> Can we at least reduce the scope of the @SuppressedWarnings by > >>>> introducing a private method that just has the return call? > >>> > >>> Sure, what about this one: > >> http://cr.openjdk.java.net/~clanger/webrevs/8223553.2/ ? > >>> > >>> Thanks > >>> Christoph > >>> > >
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
I feel the same as David - reluctant to change anything. Introducing a raw type makes an ugly cast uglier. *From: *David Holmes > > > src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java > > As you note this should be ok'd by jsr166 folk so I've cc'd Martin > Buccholz. I dislike seeing a raw type introduced here though. > >
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi Christoph, I'm very reluctant to see changes like this that the compiler folk have not determined are actually incorrect. That said ... On 15/05/2019 7:03 am, Langer, Christoph wrote: Thanks Daniel. Can anybody help reviewing the changes to: src/java.base/share/classes/java/lang/invoke/MethodHandles.java The introduction of the intermediate local variable seems harmless (though why it should be necessary is another matter). src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java As you note this should be ok'd by jsr166 folk so I've cc'd Martin Buccholz. I dislike seeing a raw type introduced here though. and src/java.management/share/classes/java/lang/management/ManagementFactory.java ? Introducing an unchecked cast seems very crude. I'd want the core-libs stream experts to comment on this. Cheers, David Thanks Christoph -Original Message- From: Daniel Fuchs Sent: Dienstag, 14. Mai 2019 18:04 To: Langer, Christoph ; core-libs-dev ; net-dev Cc: compiler-...@openjdk.java.net Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler Hi Christoph, That looks much better, thanks! (but still not commenting on the other changes ;-)) best regards, -- daniel On 14/05/2019 13:57, Langer, Christoph wrote: Hi Daniel, unfortunately, your proposed solution does not work with javac. I get this in the build: Oh darn. I should have double checked. Can we at least reduce the scope of the @SuppressedWarnings by introducing a private method that just has the return call? Sure, what about this one: http://cr.openjdk.java.net/~clanger/webrevs/8223553.2/ ? Thanks Christoph
RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Thanks Daniel. Can anybody help reviewing the changes to: src/java.base/share/classes/java/lang/invoke/MethodHandles.java src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java and src/java.management/share/classes/java/lang/management/ManagementFactory.java ? Thanks Christoph > -Original Message- > From: Daniel Fuchs > Sent: Dienstag, 14. Mai 2019 18:04 > To: Langer, Christoph ; core-libs-dev d...@openjdk.java.net>; net-dev > Cc: compiler-...@openjdk.java.net > Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the > Eclipse Java Compiler > > Hi Christoph, > > That looks much better, thanks! > (but still not commenting on the other changes ;-)) > > best regards, > > -- daniel > > On 14/05/2019 13:57, Langer, Christoph wrote: > > Hi Daniel, > > > >>> unfortunately, your proposed solution does not work with javac. I get > this > >> in the build: > >> > >> Oh darn. I should have double checked. > >> Can we at least reduce the scope of the @SuppressedWarnings by > >> introducing a private method that just has the return call? > > > > Sure, what about this one: > http://cr.openjdk.java.net/~clanger/webrevs/8223553.2/ ? > > > > Thanks > > Christoph > >
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi Christoph, That looks much better, thanks! (but still not commenting on the other changes ;-)) best regards, -- daniel On 14/05/2019 13:57, Langer, Christoph wrote: Hi Daniel, unfortunately, your proposed solution does not work with javac. I get this in the build: Oh darn. I should have double checked. Can we at least reduce the scope of the @SuppressedWarnings by introducing a private method that just has the return call? Sure, what about this one: http://cr.openjdk.java.net/~clanger/webrevs/8223553.2/ ? Thanks Christoph
RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi Daniel, > > unfortunately, your proposed solution does not work with javac. I get this > in the build: > > Oh darn. I should have double checked. > Can we at least reduce the scope of the @SuppressedWarnings by > introducing a private method that just has the return call? Sure, what about this one: http://cr.openjdk.java.net/~clanger/webrevs/8223553.2/ ? Thanks Christoph
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi Christoph, On 13/05/2019 08:29, Langer, Christoph wrote: Hi Daniel, unfortunately, your proposed solution does not work with javac. I get this in the build: Oh darn. I should have double checked. Can we at least reduce the scope of the @SuppressedWarnings by introducing a private method that just has the return call? best regards, -- daniel ...\mercurial\jdk\src\java.net.http\share\classes\jdk\internal\net\http\ExchangeImpl.java:103: error: method thenCompose in class CompletableFuture cannot be applied to given types; return c2f.handle(factory).thenCompose(identity); ^ required: Function>,? extends CompletionStage> found:Function>,CompletableFuture>> reason: cannot infer type-variable(s) U#2 (argument mismatch; Function>,CompletableFuture>> cannot be converted to Function>,? extends CompletionStage>) where U#1,U#2,T are type-variables: U#1 extends Object declared in method get(Exchange,HttpConnection) U#2 extends Object declared in method thenCompose(Function>) T extends Object declared in class CompletableFuture 1 error So I think we need to go back to my initial proposal which works for both, IDE and javac:http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/ Thanks Christoph
RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi Daniel, unfortunately, your proposed solution does not work with javac. I get this in the build: ...\mercurial\jdk\src\java.net.http\share\classes\jdk\internal\net\http\ExchangeImpl.java:103: error: method thenCompose in class CompletableFuture cannot be applied to given types; return c2f.handle(factory).thenCompose(identity); ^ required: Function>,? extends CompletionStage> found:Function>,CompletableFuture>> reason: cannot infer type-variable(s) U#2 (argument mismatch; Function>,CompletableFuture>> cannot be converted to Function>,? extends CompletionStage>) where U#1,U#2,T are type-variables: U#1 extends Object declared in method get(Exchange,HttpConnection) U#2 extends Object declared in method thenCompose(Function>) T extends Object declared in class CompletableFuture 1 error So I think we need to go back to my initial proposal which works for both, IDE and javac: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/ Thanks Christoph > -Original Message- > From: Langer, Christoph > Sent: Freitag, 10. Mai 2019 11:17 > To: Daniel Fuchs ; core-libs-dev d...@openjdk.java.net>; net-dev > Cc: compiler-...@openjdk.java.net > Subject: RE: RFR: 8223553: Fix code constructs that do not compile with the > Eclipse Java Compiler > > Hi Daniel, > > I fully agree, @SuppressWarnings should be avoided wherever possible. > > So, thanks for coming up with this alternative suggestion. It works and so I > updated my webrev: > http://cr.openjdk.java.net/~clanger/webrevs/8223553.1/ > > Any reviews for the other 3 files? > > Thanks > Christoph > > > -Original Message- > > From: Daniel Fuchs > > Sent: Donnerstag, 9. Mai 2019 17:24 > > To: Langer, Christoph ; core-libs-dev libs- > > d...@openjdk.java.net>; net-dev > > Cc: compiler-...@openjdk.java.net > > Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the > > Eclipse Java Compiler > > > > Hi Christoph, > > > > I'm only commenting on the HttpClient changes, I'll let > > others comment on the other changes... > > > > > http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/src/java.net.http/s > > hare/classes/jdk/internal/net/http/ExchangeImpl.java.udiff.html > > > > I have a profound dislike for using @SuppressedWarnings, unless > > it's the only alternative. My preference would be to introduce > > local variables, if that can make the compiler happy, until such time > > where the issue is actually resolved... > > > > For instance, it seems that the following would work: > > > > // Use local variable assignments to keep other java compilers > > // happy and avoid unchecked casts warnings > > BiFunction > ExchangeImpl>> > > factory = (h2c, t) -> createExchangeImpl(h2c, t, exchange, > > connection); > > Function>, > > CompletableFuture>> > > identity = (cf) -> cf; > > return c2f.handle(factory).thenCompose(identity); > > > > > > best regards, > > > > -- daniel > > > > On 08/05/2019 09:02, Langer, Christoph wrote: > > > Hi, > > > > > > please review a small change that I'd like to see in OpenJDK to get rid > > > of compilation errors in the Eclipse IDE. > > > > > > It seems the root cause for the compilation errors is that javac would > > > sometimes widen capture variables and is hence able to compile the > > > places that I touch here. The EC4J compiler claims that it's following > > > the spec more strictly and bails out at these places. I had posted about > > > this on compiler-dev but got no reaction [0]. > > > > > > So, as this seems to be some field of open work for the compiler/spec > > > folks, I'd like to get EC4J quiesced by introducing some slight > > > modifications to the original places which makes the code appeal a bit > > > less elegant and fluent but will get rid of the compile errors. > > > > > > Please review: > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8223553 > > > > > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/ > > > > > > The modification to > > > > > > src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.jav > > a > > > belongs to JSR-166, so I don't know if it needs some special handling. > > > > > > Thanks & Best regards > > > > > > Christoph > > > > > > [0] > > > https://mail.openjdk.java.net/pipermail/compiler-dev/2019- > > March/013054.html > > >
RE: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi Daniel, I fully agree, @SuppressWarnings should be avoided wherever possible. So, thanks for coming up with this alternative suggestion. It works and so I updated my webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.1/ Any reviews for the other 3 files? Thanks Christoph > -Original Message- > From: Daniel Fuchs > Sent: Donnerstag, 9. Mai 2019 17:24 > To: Langer, Christoph ; core-libs-dev d...@openjdk.java.net>; net-dev > Cc: compiler-...@openjdk.java.net > Subject: Re: RFR: 8223553: Fix code constructs that do not compile with the > Eclipse Java Compiler > > Hi Christoph, > > I'm only commenting on the HttpClient changes, I'll let > others comment on the other changes... > > http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/src/java.net.http/s > hare/classes/jdk/internal/net/http/ExchangeImpl.java.udiff.html > > I have a profound dislike for using @SuppressedWarnings, unless > it's the only alternative. My preference would be to introduce > local variables, if that can make the compiler happy, until such time > where the issue is actually resolved... > > For instance, it seems that the following would work: > > // Use local variable assignments to keep other java compilers > // happy and avoid unchecked casts warnings > BiFunction ExchangeImpl>> > factory = (h2c, t) -> createExchangeImpl(h2c, t, exchange, > connection); > Function>, > CompletableFuture>> > identity = (cf) -> cf; > return c2f.handle(factory).thenCompose(identity); > > > best regards, > > -- daniel > > On 08/05/2019 09:02, Langer, Christoph wrote: > > Hi, > > > > please review a small change that I'd like to see in OpenJDK to get rid > > of compilation errors in the Eclipse IDE. > > > > It seems the root cause for the compilation errors is that javac would > > sometimes widen capture variables and is hence able to compile the > > places that I touch here. The EC4J compiler claims that it's following > > the spec more strictly and bails out at these places. I had posted about > > this on compiler-dev but got no reaction [0]. > > > > So, as this seems to be some field of open work for the compiler/spec > > folks, I'd like to get EC4J quiesced by introducing some slight > > modifications to the original places which makes the code appeal a bit > > less elegant and fluent but will get rid of the compile errors. > > > > Please review: > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8223553 > > > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/ > > > > The modification to > > > src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.jav > a > > belongs to JSR-166, so I don't know if it needs some special handling. > > > > Thanks & Best regards > > > > Christoph > > > > [0] > > https://mail.openjdk.java.net/pipermail/compiler-dev/2019- > March/013054.html > >
Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi Christoph, I'm only commenting on the HttpClient changes, I'll let others comment on the other changes... http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java.udiff.html I have a profound dislike for using @SuppressedWarnings, unless it's the only alternative. My preference would be to introduce local variables, if that can make the compiler happy, until such time where the issue is actually resolved... For instance, it seems that the following would work: // Use local variable assignments to keep other java compilers // happy and avoid unchecked casts warnings BiFunctionExchangeImpl>> factory = (h2c, t) -> createExchangeImpl(h2c, t, exchange, connection); Function>, CompletableFuture>> identity = (cf) -> cf; return c2f.handle(factory).thenCompose(identity); best regards, -- daniel On 08/05/2019 09:02, Langer, Christoph wrote: Hi, please review a small change that I’d like to see in OpenJDK to get rid of compilation errors in the Eclipse IDE. It seems the root cause for the compilation errors is that javac would sometimes widen capture variables and is hence able to compile the places that I touch here. The EC4J compiler claims that it’s following the spec more strictly and bails out at these places. I had posted about this on compiler-dev but got no reaction [0]. So, as this seems to be some field of open work for the compiler/spec folks, I’d like to get EC4J quiesced by introducing some slight modifications to the original places which makes the code appeal a bit less elegant and fluent but will get rid of the compile errors. Please review: Bug: https://bugs.openjdk.java.net/browse/JDK-8223553 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/ The modification to src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java belongs to JSR-166, so I don’t know if it needs some special handling. Thanks & Best regards Christoph [0] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-March/013054.html
RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler
Hi, please review a small change that I'd like to see in OpenJDK to get rid of compilation errors in the Eclipse IDE. It seems the root cause for the compilation errors is that javac would sometimes widen capture variables and is hence able to compile the places that I touch here. The EC4J compiler claims that it's following the spec more strictly and bails out at these places. I had posted about this on compiler-dev but got no reaction [0]. So, as this seems to be some field of open work for the compiler/spec folks, I'd like to get EC4J quiesced by introducing some slight modifications to the original places which makes the code appeal a bit less elegant and fluent but will get rid of the compile errors. Please review: Bug: https://bugs.openjdk.java.net/browse/JDK-8223553 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/ The modification to src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java belongs to JSR-166, so I don't know if it needs some special handling. Thanks & Best regards Christoph [0] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-March/013054.html
