RFR: JDK-8241310 Fix warnings in jdk buildtools
The buildtools (java tools needed to be run during the build) have long been plagued by warnings, includuing deprecations and unchecked warnings, which cannot be silenced during the build. This patch fixes all buildtool warnings. Most of the warnings are fixed properly, but a few have had their warnings suppressed locally. For two tools, cldrconverter and tzdb, I gave up to get them fully fixed, and instead suppressed warnings in some places. Common for both these tools were that they used collections of some kind, with a mixed bag of data types (e.g. a Map from String to either String, HashMap, ArrayList and String[]). I'm frankly not sure how this could ever have worked :-) but I assume that the data files being parsed has a structure that "guarantees" that this works. Two files in generatecharacter were missing a proper copyright header. I noticed this when I were about to update the copyright year, and when I checked the other files I noted another one without header. While I did not need to change this file, I thought it was suitable to fix the missing header for both files. I have verified that the code generated by the build is identical with and without this patch. Bug: https://bugs.openjdk.java.net/browse/JDK-8241310 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8241310-fix-warnings-in-buildtools/webr... /Magnus
Looks good to me. I love the WrapperGenerator using Vector and Hashtable! /Erik On 2020-03-19 09:53, Magnus Ihse Bursie wrote:
The buildtools (java tools needed to be run during the build) have long been plagued by warnings, includuing deprecations and unchecked warnings, which cannot be silenced during the build.
This patch fixes all buildtool warnings. Most of the warnings are fixed properly, but a few have had their warnings suppressed locally.
For two tools, cldrconverter and tzdb, I gave up to get them fully fixed, and instead suppressed warnings in some places. Common for both these tools were that they used collections of some kind, with a mixed bag of data types (e.g. a Map from String to either String, HashMap, ArrayList and String[]). I'm frankly not sure how this could ever have worked :-) but I assume that the data files being parsed has a structure that "guarantees" that this works.
Two files in generatecharacter were missing a proper copyright header. I noticed this when I were about to update the copyright year, and when I checked the other files I noted another one without header. While I did not need to change this file, I thought it was suitable to fix the missing header for both files.
I have verified that the code generated by the build is identical with and without this patch.
Bug: https://bugs.openjdk.java.net/browse/JDK-8241310 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8241310-fix-warnings-in-buildtools/webr...
/Magnus
On 2020-03-19 18:54, Erik Joelsson wrote:
Looks good to me. Thanks.
I love the WrapperGenerator using Vector and Hashtable!
Yeah. State of the art. I'm still trying to wrap my head around this piece of beauty: assert !(currentContainer instanceof Entry); Entry<?> entry = (Entry<?>)currentContainer; ... /Magnus
/Erik
On 2020-03-19 09:53, Magnus Ihse Bursie wrote:
The buildtools (java tools needed to be run during the build) have long been plagued by warnings, includuing deprecations and unchecked warnings, which cannot be silenced during the build.
This patch fixes all buildtool warnings. Most of the warnings are fixed properly, but a few have had their warnings suppressed locally.
For two tools, cldrconverter and tzdb, I gave up to get them fully fixed, and instead suppressed warnings in some places. Common for both these tools were that they used collections of some kind, with a mixed bag of data types (e.g. a Map from String to either String, HashMap, ArrayList and String[]). I'm frankly not sure how this could ever have worked :-) but I assume that the data files being parsed has a structure that "guarantees" that this works.
Two files in generatecharacter were missing a proper copyright header. I noticed this when I were about to update the copyright year, and when I checked the other files I noted another one without header. While I did not need to change this file, I thought it was suitable to fix the missing header for both files.
I have verified that the code generated by the build is identical with and without this patch.
Bug: https://bugs.openjdk.java.net/browse/JDK-8241310 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8241310-fix-warnings-in-buildtools/webr...
/Magnus
Hi Magnus, please try not to use @SuppressWarnings("unchecked") on methods, but on local variable instead to reduce the scope, you can introduce a local variable for that. In Bundle, your patch declare @SuppressWarnings("unchecked") on the method while you already have a local variable with a @SuppressWarnings("unchecked"). Also, usually you don't need @SuppressWarnings("rawtype") apart when you use Object.getClass, otherwise using a wilcard solve the issue. By example in PluralsParseHandle, instead of Entry entry = (Entry)currentContainer; the code should be Entry<?,?> entry = (Entry<?,?>)currentContainer; BTW, in this method, you have an Arrays.stream(array).forEach() which is an antipattern, a simple loop should be used instead. regards, Rémi ----- Mail original -----
De: "Magnus Ihse Bursie" <magnus.ihse.bursie@oracle.com> À: "build-dev" <build-dev@openjdk.java.net>, "core-libs-dev" <core-libs-dev@openjdk.java.net> Envoyé: Jeudi 19 Mars 2020 17:53:09 Objet: RFR: JDK-8241310 Fix warnings in jdk buildtools
The buildtools (java tools needed to be run during the build) have long been plagued by warnings, includuing deprecations and unchecked warnings, which cannot be silenced during the build.
This patch fixes all buildtool warnings. Most of the warnings are fixed properly, but a few have had their warnings suppressed locally.
For two tools, cldrconverter and tzdb, I gave up to get them fully fixed, and instead suppressed warnings in some places. Common for both these tools were that they used collections of some kind, with a mixed bag of data types (e.g. a Map from String to either String, HashMap, ArrayList and String[]). I'm frankly not sure how this could ever have worked :-) but I assume that the data files being parsed has a structure that "guarantees" that this works.
Two files in generatecharacter were missing a proper copyright header. I noticed this when I were about to update the copyright year, and when I checked the other files I noted another one without header. While I did not need to change this file, I thought it was suitable to fix the missing header for both files.
I have verified that the code generated by the build is identical with and without this patch.
Bug: https://bugs.openjdk.java.net/browse/JDK-8241310 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8241310-fix-warnings-in-buildtools/webr...
/Magnus
On 2020-03-19 19:12, Remi Forax wrote:
Hi Magnus,
please try not to use @SuppressWarnings("unchecked") on methods, but on local variable instead to reduce the scope, you can introduce a local variable for that. Aha, I did not know that possibility existed! (My Java skills is becoming somewhat rusty after spending too much time in the build system.) That's much better, I fully agree.
In Bundle, your patch declare @SuppressWarnings("unchecked") on the method while you already have a local variable with a @SuppressWarnings("unchecked").
Also, usually you don't need @SuppressWarnings("rawtype") apart when you use Object.getClass, otherwise using a wilcard solve the issue. By example in PluralsParseHandle, instead of Entry entry = (Entry)currentContainer; the code should be Entry<?,?> entry = (Entry<?,?>)currentContainer; Thank you. I'm still struggling with the <?> capturing mechanics. BTW, in this method, you have an Arrays.stream(array).forEach() which is an antipattern, a simple loop should be used instead. I'm sure the code is full of ugliness; however it is not "my" code -- I'm merely trying to fix the compiler warnings, and I will not do any further cleanups. If I started to walk on that path, there will be no end.
Here is an updated webrev. Now I'm down to just four SuppressWarnings, all of them localized, and three of them (in Bundle.java) are modeled after similar patterns in the same function. http://cr.openjdk.java.net/~ihse/JDK-8241310-fix-warnings-in-buildtools/webr... /Magnus
regards, Rémi
----- Mail original -----
De: "Magnus Ihse Bursie" <magnus.ihse.bursie@oracle.com> À: "build-dev" <build-dev@openjdk.java.net>, "core-libs-dev" <core-libs-dev@openjdk.java.net> Envoyé: Jeudi 19 Mars 2020 17:53:09 Objet: RFR: JDK-8241310 Fix warnings in jdk buildtools The buildtools (java tools needed to be run during the build) have long been plagued by warnings, includuing deprecations and unchecked warnings, which cannot be silenced during the build.
This patch fixes all buildtool warnings. Most of the warnings are fixed properly, but a few have had their warnings suppressed locally.
For two tools, cldrconverter and tzdb, I gave up to get them fully fixed, and instead suppressed warnings in some places. Common for both these tools were that they used collections of some kind, with a mixed bag of data types (e.g. a Map from String to either String, HashMap, ArrayList and String[]). I'm frankly not sure how this could ever have worked :-) but I assume that the data files being parsed has a structure that "guarantees" that this works.
Two files in generatecharacter were missing a proper copyright header. I noticed this when I were about to update the copyright year, and when I checked the other files I noted another one without header. While I did not need to change this file, I thought it was suitable to fix the missing header for both files.
I have verified that the code generated by the build is identical with and without this patch.
Bug: https://bugs.openjdk.java.net/browse/JDK-8241310 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8241310-fix-warnings-in-buildtools/webr...
/Magnus
Looks even better. /Erik On 2020-03-20 04:42, Magnus Ihse Bursie wrote:
On 2020-03-19 19:12, Remi Forax wrote:
Hi Magnus,
please try not to use @SuppressWarnings("unchecked") on methods, but on local variable instead to reduce the scope, you can introduce a local variable for that. Aha, I did not know that possibility existed! (My Java skills is becoming somewhat rusty after spending too much time in the build system.) That's much better, I fully agree.
In Bundle, your patch declare @SuppressWarnings("unchecked") on the method while you already have a local variable with a @SuppressWarnings("unchecked").
Also, usually you don't need @SuppressWarnings("rawtype") apart when you use Object.getClass, otherwise using a wilcard solve the issue. By example in PluralsParseHandle, instead of Entry entry = (Entry)currentContainer; the code should be Entry<?,?> entry = (Entry<?,?>)currentContainer; Thank you. I'm still struggling with the <?> capturing mechanics. BTW, in this method, you have an Arrays.stream(array).forEach() which is an antipattern, a simple loop should be used instead. I'm sure the code is full of ugliness; however it is not "my" code -- I'm merely trying to fix the compiler warnings, and I will not do any further cleanups. If I started to walk on that path, there will be no end.
Here is an updated webrev. Now I'm down to just four SuppressWarnings, all of them localized, and three of them (in Bundle.java) are modeled after similar patterns in the same function.
http://cr.openjdk.java.net/~ihse/JDK-8241310-fix-warnings-in-buildtools/webr...
/Magnus
regards, Rémi
----- Mail original -----
De: "Magnus Ihse Bursie" <magnus.ihse.bursie@oracle.com> À: "build-dev" <build-dev@openjdk.java.net>, "core-libs-dev" <core-libs-dev@openjdk.java.net> Envoyé: Jeudi 19 Mars 2020 17:53:09 Objet: RFR: JDK-8241310 Fix warnings in jdk buildtools The buildtools (java tools needed to be run during the build) have long been plagued by warnings, includuing deprecations and unchecked warnings, which cannot be silenced during the build.
This patch fixes all buildtool warnings. Most of the warnings are fixed properly, but a few have had their warnings suppressed locally.
For two tools, cldrconverter and tzdb, I gave up to get them fully fixed, and instead suppressed warnings in some places. Common for both these tools were that they used collections of some kind, with a mixed bag of data types (e.g. a Map from String to either String, HashMap, ArrayList and String[]). I'm frankly not sure how this could ever have worked :-) but I assume that the data files being parsed has a structure that "guarantees" that this works.
Two files in generatecharacter were missing a proper copyright header. I noticed this when I were about to update the copyright year, and when I checked the other files I noted another one without header. While I did not need to change this file, I thought it was suitable to fix the missing header for both files.
I have verified that the code generated by the build is identical with and without this patch.
Bug: https://bugs.openjdk.java.net/browse/JDK-8241310 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8241310-fix-warnings-in-buildtools/webr...
/Magnus
participants (3)
-
Erik Joelsson
-
Magnus Ihse Bursie
-
Remi Forax