[OpenJDK 2D-Dev] JDK 9: RFR: 8033716: Fix raw and unchecked lint warnings in com.sun.imageio
Hi, Please review the webrev to clean up raw and unchecked warnings in com.sun.imageio packag at, http://cr.openjdk.java.net/~henryjen/jdk9/8033716/0/webrev/ The more significant change in this webrev is that I have changed the clone() method of MarkerSegment-derived classes to return exact type rather than Object. Otherwise, it's basically add type information and eliminate cast no longer needed. Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK 9: RFR: 8033716: Fix raw and unchecked lint warnings in com.sun.imageio
Thanks Joe for reviewing. I would like to get 2d developer review as well before pushing this, let me know if that's not necessary. Also there was a discussion ealier on whether such change should go to client or jdk9/dev repo, do we have a conclusion? Cheers, Henry On 02/05/2014 06:01 PM, Joe Darcy wrote: Hi Henry, On 02/05/2014 12:19 PM, Henry Jen wrote: Hi, Please review the webrev to clean up raw and unchecked warnings in com.sun.imageio packag at, http://cr.openjdk.java.net/~henryjen/jdk9/8033716/0/webrev/ The more significant change in this webrev is that I have changed the clone() method of MarkerSegment-derived classes to return exact type rather than Object. Otherwise, it's basically add type information and eliminate cast no longer needed. Cheers, Henry I looked over the changes and they generally look good. I've taken a closer look at the clone-related changes. The types SOSMarkerSegment, SOFMarkerSegment, MarkerSegment, etc. are call package-private and all have had their Object-returning clone methods replaced with a self-type returning clone method, a covariant override. Since there are no other potential subclasses of these com.sun.* type that would already have had a covariant override of clone, I believe the changes to clone on these three methods is fine. (This avoid the hazards outlined in JDK-7140820: (coll) Add covariant overrides to Collections clone methods.) Thanks, -Joe
Re: [OpenJDK 2D-Dev] JDK 9: RFR: 8033716: Fix raw and unchecked lint warnings in com.sun.imageio
On 02/07/2014 03:00 PM, Phil Race wrote: BMPMetadata.java 94 // Fields from CommentExtension 95 // List of byte[] 96 public List comments = null; // new ArrayList(); hmm .. how did you decide this was correct, other than trusting the comment? For this one, I took it from the comment, after verified similar types in GIF. The thing is I can't actually see where this field is used and I'm inclined to think this was a copy/paste from the GIF code. It would seem the right thing to do is delete these lines. It is public, so I don't dare to remove it, although it seems like nothing in com.sun.imageio ever make use of this field. Also, from the BMPMetadataFormat, CommentExtension is a string type, so I am not really sure what's the best to do here. If you think it is safe to remove it, I am more than happy to remove it. In the case of GIFImageMetadata they are used but you left the comments saying // new ArrayList(); since it looks like you use the diamond operator now that should not be completely true. Either remove the comment or, since it seems it was intended to be informative update it to say // new ArrayList<>(); Agree, I'll remove those comments. I will have to look at all the subsequent files later .. Thanks for reviewing it. Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK 9: RFR: 8033716: Fix raw and unchecked lint warnings in com.sun.imageio
As there is no other comments so far, I posted updated version adapted comments from Phil. - removed public field comments from BMPMetadata - removed not needed comments from GIFImageMetadata http://cr.openjdk.java.net/~henryjen/jdk9/8033716/1/webrev/ Cheers, Henry On 02/07/2014 04:27 PM, Henry Jen wrote: On 02/07/2014 03:00 PM, Phil Race wrote: BMPMetadata.java 94 // Fields from CommentExtension 95 // List of byte[] 96 public List comments = null; // new ArrayList(); hmm .. how did you decide this was correct, other than trusting the comment? For this one, I took it from the comment, after verified similar types in GIF. The thing is I can't actually see where this field is used and I'm inclined to think this was a copy/paste from the GIF code. It would seem the right thing to do is delete these lines. It is public, so I don't dare to remove it, although it seems like nothing in com.sun.imageio ever make use of this field. Also, from the BMPMetadataFormat, CommentExtension is a string type, so I am not really sure what's the best to do here. If you think it is safe to remove it, I am more than happy to remove it. In the case of GIFImageMetadata they are used but you left the comments saying // new ArrayList(); since it looks like you use the diamond operator now that should not be completely true. Either remove the comment or, since it seems it was intended to be informative update it to say // new ArrayList<>(); Agree, I'll remove those comments. I will have to look at all the subsequent files later .. Thanks for reviewing it. Cheers, Henry
[OpenJDK 2D-Dev] JDK9: RFR: 8034998: Fix raw and unchecked lint warnings in javax.imageio
Hi, Please review the webrev to clean up raw and unchecked warnings in javax.imageio packag at, http://cr.openjdk.java.net/~henryjen/jdk9/8034998/0/webrev/ The webrev does not cover javax.imageio.spi, that will come in anoter webrev. Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8034998: Fix raw and unchecked lint warnings in javax.imageio
On 02/15/2014 01:15 PM, Joe Darcy wrote: On 02/14/2014 06:43 PM, Henry Jen wrote: Hi, Please review the webrev to clean up raw and unchecked warnings in javax.imageio packag at, http://cr.openjdk.java.net/~henryjen/jdk9/8034998/0/webrev/ The webrev does not cover javax.imageio.spi, that will come in anoter webrev. Cheers, Henry Hi Henry, I skimmed over the changeset and didn't see any issues. All the signature changes to methods seem to be on private methods or on public methods in package-private classes. If there are any change to public or protected methods in public or protected classes, whose would need a ccc request. Thanks, as far as I know, all changes to public/protected fields/methods are within private classes, but don't trust me, that's the point of code review. It seems to me, there was a round of generic type clean up for public APIs, only methods in two classes(javax.imageio.spi.Image[Reader|Writer].spi) returning Class[], which I am wondering why they are not Class[] that I would like to file CCC with javax.imageio.spi package clean up. Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK 9: RFR: 8033716: Fix raw and unchecked lint warnings in com.sun.imageio
On 02/19/2014 01:46 PM, Phil Race wrote:
.
http://cr.openjdk.java.net/~henryjen/jdk9/8033716/1/webrev/src/share/classes/com/sun/imageio/plugins/gif/GIFImageReader.java.sdiff.html
230 public Iterator getImageTypes(int
imageIndex) throws IIOException {
Nitpicky perhaps, but this looks > 80 chars. If so please split it.
Will do a round to split lone lines to keep them under 80.
-
W.r.t the following change ...
http://cr.openjdk.java.net/~henryjen/jdk9/8033716/1/webrev/src/share/classes/com/sun/imageio/plugins/jpeg/DHTMarkerSegment.java.sdiff.html
145 class Htable implements Cloneable {
...
<208 protected Object clone() {
208 protected Htable clone()
-
exactly what warning is this suppressing ?
This eliminate "unchecked" cast warning when calling this method to get
an instance with correct class type.
Granted the language now allows returning a sub-class in over-riding,
but I don't
know why its a problem to return the declared type of the over-ridden
method?
The tool must have special knowledge of the semantics of the Cloneable
interface
to even call this out !
Same comment/question applies to DQTMarkerSegment.clone,
JFIFMarkerSegment.clone() and any other like that you might have changed ..
inc. the superclassMarkerSegment which I just spotted ... plus
SOFMarkerSegment,S
SOMarkerSegment
Maybe these will be OK but I need an explanation.
for (Iterator iter =
extSegments.iterator(); iter.hasNext();) {232 for
(Iterator iter = extSegments.iterator();
iter.hasNext();) {
635 for (Iterator iter =
extSegments.iterator(); iter.hasNext();)
More very long lines ...
I see yet more in JPEGMetadata : 663, 573, 688, 698, 710 .. too many
more to call out !
I will change those if we wanted to stick to 80 column limit. I seems to
see it loosed to 132 column in many places. Personally I stick to 80 but
tolerate to 100 when I feel it's more readable.
Finally, again note this change when approved should go into jdk9/client.
I'll rebase the patch.
Cheers,
Henry
Re: [OpenJDK 2D-Dev] JDK 9: RFR: 8033716: Fix raw and unchecked lint warnings in com.sun.imageio
On 02/19/2014 02:59 PM, Henry Jen wrote:
On 02/19/2014 01:46 PM, Phil Race wrote:
W.r.t the following change ...
http://cr.openjdk.java.net/~henryjen/jdk9/8033716/1/webrev/src/share/classes/com/sun/imageio/plugins/jpeg/DHTMarkerSegment.java.sdiff.html
145 class Htable implements Cloneable {
...
<208 protected Object clone() {
208 protected Htable clone()
-
exactly what warning is this suppressing ?
This eliminate "unchecked" cast warning when calling this method to get
an instance with correct class type.
That's not exactly correct, it just eliminate the need to cast, not warning.
Would you advice to revert them or keep it?
Also, I noted there are a couple fallthrough warnings, the
BMPImageReader one seems valid and should be fixed while the others
looks correct, but I am not exactly sure about the image format, so I
think I'll just list it here and perhaps someone with expertise can
review them?
/home/hjen/ws/9-client/jdk/src/share/classes/com/sun/imageio/plugins/bmp/BMPImageReader.java:916:
warning: [fallthrough] possible fall-through into case
case VERSION_4_8_BIT:
^
/home/hjen/ws/9-client/jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java:429:
warning: [fallthrough] possible fall-through into case
case 0: // not a marker, just a data 0xff
^
/home/hjen/ws/9-client/jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageWriter.java:1554:
warning: [fallthrough] possible fall-through into case
case ColorSpace.TYPE_CMYK:
^
/home/hjen/ws/9-client/jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageWriter.java:1593:
warning: [fallthrough] possible fall-through into case
case ColorSpace.TYPE_CMYK:
^
/home/hjen/ws/9-client/jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageWriter.java:1639:
warning: [fallthrough] possible fall-through into case
case ColorSpace.TYPE_CMYK:
Cheers,
Henry
[OpenJDK 2D-Dev] JDK 9: RFR[2]: 8033716: Fix raw and unchecked lint warnings in com.sun.imageio
Hi,
Please review the latest update, I think this should address the issues
raised,
http://cr.openjdk.java.net/~henryjen/jdk9/8033716/2/webrev/
- revert the clone method changes so that return type remains Object
- break long lines
- fix fallthrough warnings as Andrew suggested.
Cheers,
Henry
On 02/20/2014 10:09 AM, Phil Race wrote:
On 2/20/2014 12:05 AM, Henry Jen wrote:
On 02/19/2014 02:59 PM, Henry Jen wrote:
On 02/19/2014 01:46 PM, Phil Race wrote:
W.r.t the following change ...
http://cr.openjdk.java.net/~henryjen/jdk9/8033716/1/webrev/src/share/classes/com/sun/imageio/plugins/jpeg/DHTMarkerSegment.java.sdiff.html
145 class Htable implements Cloneable {
...
<208 protected Object clone() {
208 protected Htable clone()
-
exactly what warning is this suppressing ?
This eliminate "unchecked" cast warning when calling this method to get
an instance with correct class type.
That's not exactly correct, it just eliminate the need to cast, not
warning.
Would you advice to revert them or keep it?
In that case, I'd suggest to revert them
-phil.
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8034998: Fix raw and unchecked lint warnings in javax.imageio
Updated webrev at http://cr.openjdk.java.net/~henryjen/jdk9/8034998/1/webrev/ - change those two Iterator from public to private - rebase to jdk9/client Who will be pushing those changes once approved? Cheers, Henry On 02/19/2014 01:12 PM, Phil Race wrote: ImageIO.java 515 public Iterator iter; 829 public Iterator iter; I can't see why these are public. I think they should be private Since you are touching these lines anyway public->private I was on the look out for changes to public interfaces but I didn't see any so that's Ok So if you can 1) make the two changes 2) re-generate against the client forest then it should be ready for approval. -phil. On 2/14/2014 6:43 PM, Henry Jen wrote: Hi, Please review the webrev to clean up raw and unchecked warnings in javax.imageio packag at, http://cr.openjdk.java.net/~henryjen/jdk9/8034998/0/webrev/ The webrev does not cover javax.imageio.spi, that will come in anoter webrev. Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8034998: Fix raw and unchecked lint warnings in javax.imageio
On 02/20/2014 03:51 PM, Phil Race wrote: Re-checked only the changed part. Looks good. Do you have two reviewers yet? Thanks for reviewing, Phil. Joe Darcy reviewed the first version, and I listed both you and Joe as reviewer. I can push this if you like but you have commit rights so you can push yourself if you so wish .. Either would work, I don't know what's the standard practice for client repo though. I learned hotspot requires to push via jprt the hard way. So, why don't you push for me on this one, and let me know what's the correct process and I can do the right thing in the future. Cheers, Henry
[OpenJDK 2D-Dev] JDK9: RFR: 8035487: Fix raw and unchecked lint warnings in javax.imageio.spi
Hi, Please review the webrev to clean up raw and unchecked warnings in javax.imageio.spi packag at, http://cr.openjdk.java.net/~henryjen/jdk9/8035487/0/webrev/ Cheers, Henry
[OpenJDK 2D-Dev] JDK9: RFR: 8038644: Fix raw and unchecked warnings in sun.java2d.*
Hi, Please review the webrev to clean up raw and unchecked warnings in sun.java2d packag at, http://cr.openjdk.java.net/~henryjen/jdk9/8038644/0/webrev/ Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8038644: Fix raw and unchecked warnings in sun.java2d.*
A update is needed, which can be reviewed at
http://cr.openjdk.java.net/~henryjen/jdk9/8038644/1/webrev/,
Diff from previous is following,
> diff --git a/src/share/classes/sun/java2d/SunGraphics2D.java
b/src/share/classes/sun/java2d/SunGraphics2D.java
> --- a/src/share/classes/sun/java2d/SunGraphics2D.java
> +++ b/src/share/classes/sun/java2d/SunGraphics2D.java
> @@ -1432,7 +1432,9 @@
>
> RenderingHints makeHints(Map hints) {
> RenderingHints model = new RenderingHints(null);
> -model.putAll(hints);
> +if (hints != null) {
> +model.putAll(hints);
> +}
> model.put(SunHints.KEY_RENDERING,
>SunHints.Value.get(SunHints.INTKEY_RENDERING,
> renderHint));
Cheers,
Henry
On 03/28/2014 10:01 AM, Henry Jen wrote:
Hi,
Please review the webrev to clean up raw and unchecked warnings in
sun.java2d packag at,
http://cr.openjdk.java.net/~henryjen/jdk9/8038644/0/webrev/
Cheers,
Henry
[OpenJDK 2D-Dev] JDK9: RFR: 8039342: Fix raw and unchecked warnings in sun.awt.*
Hi,
Please review the webrev cleans up raw and unchecked warnings in sun.awt,
http://cr.openjdk.java.net/~henryjen/jdk9/8039342/0/webrev/
The following changes in AreaOp::pruneEdges() is particular worth
attention, when numedges < 2, two different type are mixed up in the
past with use of rawtypes; However, I think it could only work if the
Vector is empty?
Cheers,
Henry
@@ -193,16 +193,20 @@
}
return 1;
}
};
-private Vector pruneEdges(Vector edges) {
+private Vector pruneEdges(Vector edges) {
int numedges = edges.size();
if (numedges < 2) {
-return edges;
+Vector rt = new Vector<>();
+for (Edge edge: edges) {
+rt.add(edge.getCurve());
}
-Edge[] edgelist = (Edge[]) edges.toArray(new Edge[numedges]);
+return rt;
+}
+Edge[] edgelist = edges.toArray(new Edge[numedges]);
Arrays.sort(edgelist, YXTopComparator);
if (false) {
System.out.println("pruning: ");
for (int i = 0; i < numedges; i++) {
System.out.println("edgelist["+i+"] = "+edgelist[i]);
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8039342: Fix raw and unchecked warnings in sun.awt.*
Thanks Joe for pointing out the review request ned to go to awt-dev.
Cheers,
Henry
On 04/07/2014 04:13 PM, Joe Darcy wrote:
Hi Henry,
Thanks for looking into this. FWIW, while I was working on
JDK-8039109: Fix unchecked and raw lint warnings in java.awt
I started looking at some sun.awt.* type too and I noticed the same
problem in AreaOp.pruneEdges(). The comment I added to my in-progress
webrev is:
203 /*
204 * The implementation of this method consistently treats its
205 * return type either as a Vector or a Vector.
206 */
so there does seem to be some internal inconsistency in what the Vector
is meant to hold.
In any case, your changes look good to me. (I'll adapt my changes for
8039109 based on your changes in this bug.)
Cheers,
-Joe
On 04/07/2014 01:46 PM, Henry Jen wrote:
Hi,
Please review the webrev cleans up raw and unchecked warnings in sun.awt,
http://cr.openjdk.java.net/~henryjen/jdk9/8039342/0/webrev/
The following changes in AreaOp::pruneEdges() is particular worth
attention, when numedges < 2, two different type are mixed up in the
past with use of rawtypes; However, I think it could only work if the
Vector is empty?
Cheers,
Henry
@@ -193,16 +193,20 @@
}
return 1;
}
};
-private Vector pruneEdges(Vector edges) {
+private Vector pruneEdges(Vector edges) {
int numedges = edges.size();
if (numedges < 2) {
-return edges;
+Vector rt = new Vector<>();
+for (Edge edge: edges) {
+rt.add(edge.getCurve());
}
-Edge[] edgelist = (Edge[]) edges.toArray(new Edge[numedges]);
+return rt;
+}
+Edge[] edgelist = edges.toArray(new Edge[numedges]);
Arrays.sort(edgelist, YXTopComparator);
if (false) {
System.out.println("pruning: ");
for (int i = 0; i < numedges; i++) {
System.out.println("edgelist["+i+"] = "+edgelist[i]);
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8039342: Fix raw and unchecked warnings in sun.awt.*
Thanks Joe for pointing out the request should go to awt-dev.
Cheers,
Henry
On 04/07/2014 04:13 PM, Joe Darcy wrote:
Hi Henry,
Thanks for looking into this. FWIW, while I was working on
JDK-8039109: Fix unchecked and raw lint warnings in java.awt
I started looking at some sun.awt.* type too and I noticed the same
problem in AreaOp.pruneEdges(). The comment I added to my in-progress
webrev is:
203 /*
204 * The implementation of this method consistently treats its
205 * return type either as a Vector or a Vector.
206 */
so there does seem to be some internal inconsistency in what the Vector
is meant to hold.
In any case, your changes look good to me. (I'll adapt my changes for
8039109 based on your changes in this bug.)
Cheers,
-Joe
On 04/07/2014 01:46 PM, Henry Jen wrote:
Hi,
Please review the webrev cleans up raw and unchecked warnings in sun.awt,
http://cr.openjdk.java.net/~henryjen/jdk9/8039342/0/webrev/
The following changes in AreaOp::pruneEdges() is particular worth
attention, when numedges < 2, two different type are mixed up in the
past with use of rawtypes; However, I think it could only work if the
Vector is empty?
Cheers,
Henry
@@ -193,16 +193,20 @@
}
return 1;
}
};
-private Vector pruneEdges(Vector edges) {
+private Vector pruneEdges(Vector edges) {
int numedges = edges.size();
if (numedges < 2) {
-return edges;
+Vector rt = new Vector<>();
+for (Edge edge: edges) {
+rt.add(edge.getCurve());
}
-Edge[] edgelist = (Edge[]) edges.toArray(new Edge[numedges]);
+return rt;
+}
+Edge[] edgelist = edges.toArray(new Edge[numedges]);
Arrays.sort(edgelist, YXTopComparator);
if (false) {
System.out.println("pruning: ");
for (int i = 0; i < numedges; i++) {
System.out.println("edgelist["+i+"] = "+edgelist[i]);
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8039342: Fix raw and unchecked warnings in sun.awt.*
On 04/09/2014 02:35 PM, Phil Race wrote: Yes it looks to be about 70% AWT and about 30% 2d .. Remember there's a magic decoder ring at http://openjdk.java.net/groups/2d/2dawtfiles.html It may not have 100% coverage but it should help. I looked over the 2D ones as below .. src/share/classes/sun/awt/FontConfiguration.java src/share/classes/sun/awt/PlatformFont.java src/share/classes/sun/awt/geom/AreaOp.java src/share/classes/sun/awt/geom/Crossings.java src/share/classes/sun/awt/geom/Curve.java src/share/classes/sun/awt/geom/Order2.java src/share/classes/sun/awt/geom/Order3.java src/share/classes/sun/awt/image/BufImgSurfaceData.java src/share/classes/sun/awt/image/GifImageDecoder.java src/share/classes/sun/awt/image/ImageDecoder.java src/share/classes/sun/awt/image/ImageFetcher.java src/share/classes/sun/awt/image/ImageRepresentation.java src/share/classes/sun/awt/image/ImagingLib.java src/share/classes/sun/awt/image/JPEGImageDecoder.java src/share/classes/sun/awt/image/OffScreenImageSource.java src/share/classes/sun/awt/image/PNGImageDecoder.java src/share/classes/sun/awt/image/ToolkitImage.java src/solaris/classes/sun/awt/X11FontManager.java src/solaris/classes/sun/awt/X11GraphicsDevice.java src/solaris/classes/sun/awt/X11GraphicsEnvironment.java src/solaris/classes/sun/awt/motif/MFontConfiguration.java It seems fine except that 1. How are you testing all these changes ? I ran jtreg on test/sun/awt and test/java/awt(which contains quite a few manual test that I simply click pass without verifying based on instruction). Also jprt. 2. The changes in sun/awt/geom need closer examination and I would really like Jim to weigh in on those .. Would certainly appreciate that. Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8039342: Fix raw and unchecked warnings in sun.awt.*
Ping. The webrev is updated(rebase) to latest jdk9/client repo, http://cr.openjdk.java.net/~henryjen/jdk9/8039342/1/webrev/ I also run Java2Demo with the result build and it seems fine. Cheers, Henry On 04/09/2014 03:31 PM, Henry Jen wrote: On 04/09/2014 02:35 PM, Phil Race wrote: It seems fine except that 1. How are you testing all these changes ? I ran jtreg on test/sun/awt and test/java/awt(which contains quite a few manual test that I simply click pass without verifying based on instruction). Also jprt. 2. The changes in sun/awt/geom need closer examination and I would really like Jim to weigh in on those .. Would certainly appreciate that. Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK9: RFR: 8039342: Fix raw and unchecked warnings in sun.awt.*
Thanks for the reviw.
On 04/23/2014 02:37 PM, Jim Graham wrote:
Shouldn't Order2, lines 50,77 be ""?
Ditto for Order3, lines 56,108?
I don't think we ever use these methods with any other type of Vector,
but I guess I can see that the choice you made might be a more general
choice.
Right, I took a more general approach.
This concludes my review of the geom files per Phil's request. They
look fine, other than my suggestions for returning an empty curve list
from pruneEdges and the above comments...
I changed to return empty curve list, but not static one as there is no
immutable Vector I can use without implementing one.
While I see the result is private member curves in java.awt.geom.Area, I
am not 100% certain that this cannot be leaked. I am afraid it get
tainted somehow thus I make a new instance just to be cautious. However,
if you feel strongly it's fine to return a static instance, we can do that.
Following is the diff to address your comments, let me know if other
changes are needed.
Cheers,
Henry
diff --git a/src/share/classes/sun/awt/geom/AreaOp.java
b/src/share/classes/sun/awt/geom/AreaOp.java
--- a/src/share/classes/sun/awt/geom/AreaOp.java
+++ b/src/share/classes/sun/awt/geom/AreaOp.java
@@ -198,11 +198,8 @@
private Vector pruneEdges(Vector edges) {
int numedges = edges.size();
if (numedges < 2) {
-Vector rt = new Vector<>();
-for (Edge edge: edges) {
-rt.add(edge.getCurve());
-}
-return rt;
+// empty vector is expected with less than 2 edges
+return new Vector<>();
}
Edge[] edgelist = edges.toArray(new Edge[numedges]);
Arrays.sort(edgelist, YXTopComparator);
diff --git a/src/share/classes/sun/awt/geom/Order2.java
b/src/share/classes/sun/awt/geom/Order2.java
--- a/src/share/classes/sun/awt/geom/Order2.java
+++ b/src/share/classes/sun/awt/geom/Order2.java
@@ -47,7 +47,7 @@
private double ycoeff1;
private double ycoeff2;
-public static void insert(Vector curves, double tmp[],
+public static void insert(Vector curves, double tmp[],
double x0, double y0,
double cx0, double cy0,
double x1, double y1,
@@ -74,7 +74,7 @@
tmp[i1 + 4], tmp[i1 + 5], direction);
}
-public static void addInstance(Vector curves,
+public static void addInstance(Vector curves,
double x0, double y0,
double cx0, double cy0,
double x1, double y1,
diff --git a/src/share/classes/sun/awt/geom/Order3.java
b/src/share/classes/sun/awt/geom/Order3.java
--- a/src/share/classes/sun/awt/geom/Order3.java
+++ b/src/share/classes/sun/awt/geom/Order3.java
@@ -53,7 +53,7 @@
private double ycoeff2;
private double ycoeff3;
-public static void insert(Vector curves, double tmp[],
+public static void insert(Vector curves, double tmp[],
double x0, double y0,
double cx0, double cy0,
double cx1, double cy1,
@@ -105,7 +105,7 @@
}
}
-public static void addInstance(Vector curves,
+public static void addInstance(Vector curves,
double x0, double y0,
double cx0, double cy0,
double cx1, double cy1,
Cheers,
Henry
...jim
On 4/7/14 1:46 PM, Henry Jen wrote:
Hi,
Please review the webrev cleans up raw and unchecked warnings in sun.awt,
http://cr.openjdk.java.net/~henryjen/jdk9/8039342/0/webrev/
The following changes in AreaOp::pruneEdges() is particular worth
attention, when numedges < 2, two different type are mixed up in the
past with use of rawtypes; However, I think it could only work if the
Vector is empty?
Cheers,
Henry
@@ -193,16 +193,20 @@
}
return 1;
}
};
-private Vector pruneEdges(Vector edges) {
+private Vector pruneEdges(Vector edges) {
int numedges = edges.size();
if (numedges < 2) {
-return edges;
+Vector rt = new Vector<>();
+for (Edge edge: edges) {
+rt.add(edge.getCurve());
}
-Edge[] edgelist = (Edge[]) edges.toArray(new Edge[numedges]);
+return rt;
+}
+Edge[] edgelist = edges.toArray(new Edge[numedges]);
Arrays.sort(edgelist, YXTopComparator);
if (false) {
System.out.println("pruning: ");
for (int i = 0; i < numedges; i++) {
System.out.println("edgelist["+i+"] = "+edgelist[i]);
Re: [OpenJDK 2D-Dev] JDK 9 RFR of JDK-8042864 : Fix raw and unchecked warnings in javax.print
On 05/15/2014 12:07 PM, Joe Darcy wrote:
Hello,
Please review these change to fix
JDK-8042864 : Fix raw and unchecked warnings in javax.print
http://cr.openjdk.java.net/~darcy/8042864.0/
Patch below.
Looks good to me, just nit-picking.
--- old/src/share/classes/javax/print/PrintServiceLookup.java 2014-05-15
12:04:20.0 -0700
+++ new/src/share/classes/javax/print/PrintServiceLookup.java 2014-05-15
12:04:20.0 -0700
@@ -208,7 +207,7 @@
*/
public static boolean registerServiceProvider(PrintServiceLookup
sp) {
synchronized (PrintServiceLookup.class) {
-Iterator psIterator = getAllLookupServices().iterator();
+Iterator psIterator = getAllLookupServices().iterator();
while (psIterator.hasNext()) {
try {
Object lus = psIterator.next();
We know this is Iterator, but this works.
---
old/src/share/classes/javax/print/attribute/AttributeSetUtilities.java
2014-05-15 12:04:22.0 -0700
+++
new/src/share/classes/javax/print/attribute/AttributeSetUtilities.java
2014-05-15 12:04:22.0 -0700
@@ -523,7 +523,7 @@
public static Class
verifyAttributeCategory(Object object, Class interfaceName) {
-Class result = (Class) object;
+Class result = (Class) object;
if (interfaceName.isAssignableFrom (result)) {
return result;
}
Should the cast be (Class) instead of (Class)?
---
old/src/share/classes/javax/print/attribute/standard/DialogTypeSelection.java
2014-05-15 12:04:24.0 -0700
+++
new/src/share/classes/javax/print/attribute/standard/DialogTypeSelection.java
2014-05-15 12:04:23.0 -0700
@@ -110,7 +110,7 @@
* @return Printing attribute class (category), an instance of class
* {@link java.lang.Class java.lang.Class}.
*/
-public final Class getCategory() {
+public final Class getCategory() {
return DialogTypeSelection.class;
}
Would this be too specific for this public API? is
defined in interface Attribute.
---
old/src/share/classes/javax/print/attribute/standard/PrinterStateReasons.java
2014-05-15 12:04:25.0 -0700
+++
new/src/share/classes/javax/print/attribute/standard/PrinterStateReasons.java
2014-05-15 12:04:24.0 -0700
@@ -242,16 +242,18 @@
extends AbstractSet
{
private Severity mySeverity;
-private Set myEntrySet;
+//
+private Set> myEntrySet;
-public PrinterStateReasonSet(Severity severity, Set entrySet) {
+public PrinterStateReasonSet(Severity severity,
+ Set> entrySet) {
mySeverity = severity;
myEntrySet = entrySet;
}
public int size() {
int result = 0;
-Iterator iter = iterator();
+Iterator iter = iterator();
We know it is Iterator.
while (iter.hasNext()) {
iter.next();
++ result;
Cheers,
Henry
Re: [OpenJDK 2D-Dev] RFR: 8044740: Convert all JDK versions used in @since tag to 1.n[.n] in jdk repo
Thanks for all reviewing and feedbacks on core-libs-dev[1], I tried to respond to feedbacks with this email and send off to other mailing lists. I am wondering if jdk9-dev is the appropriate list for such a trivious but broad change, so that we can have one instead of many lists, and we still probably miss another. Lets follow up this thread on jdk9-dev. Regarding to whether we should keep JDK, the later convention is 1.#, and as David pointed out the document also list @since that way, I think we should settle on that. For other standards such as SAX or JCE, I propose to convert them to the version of JDK those APIs are included. To retain that information, we can introduce a custom tag, perhaps @standard or @conformingTo? @conformingTo [, ]* For example, @conformingTo SAX 2.0. Repo wise, I think it's best if I can commit to jdk9/dev as a single commit instead of scattering to dev and client. But I can cope if this is absolutely necessary. Some changes to implementation classes, as I mentioned, only when it is straightforward. Essentially, I did a s/(@since *)JDK(.*)/\1\2 against all files. Some changes not obvious are simply remove tailing space, a (positive) side effect of the tools I use so I kept them. Cheers, Henry [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027113.html On 06/03/2014 06:22 PM, Henry Jen wrote: Hi, In an effort to determine APIs availability in a given version, it became obvious that a consistent way to express @since tag would be beneficial. So started with the most obvious ones, where we have various expression for JDK version, this webrev make sure we use @since 1.n[.n] for JDK versions. The main focus is on public APIs, private ones are taken care if it is straightforward, otherwise, we try to keep the information. Some public APIs are using @since format, they are also preserved for now, but I think it worth discussion whether we want to change to the version as included in J2SE. There are APIs without @since information, separate webrevs will be coming to complete those information. Bug: https://bugs.openjdk.java.net/browse/JDK-8044740 The webrev can be found at http://cr.openjdk.java.net/~henryjen/jdk9/8044740/0/webrev but it's probably easier just look into the raw diff, http://cr.openjdk.java.net/~henryjen/jdk9/8044740/0/webrev/jdk.changeset Cheers, Henry
[OpenJDK 2D-Dev] RFR: 8044551, 8044396: Fix raw and unchecked lint warnings in platform-specific sun.awt and sun.java2d
Hi, Please review a follow-up to clean up rawtype and unchecked warnings in platform-specific code. Bug: https://bugs.openjdk.java.net/browse/JDK-8044396 https://bugs.openjdk.java.net/browse/JDK-8044551 Webrev: http://cr.openjdk.java.net/~henryjen/jdk9/8044551/0/webrev/ Cheers, Henry
Re: [OpenJDK 2D-Dev] RFR: 8044551, 8044396: Fix raw and unchecked lint warnings in platform-specific sun.awt and sun.java2d
Thanks Joe and Phil for reviewing. Cheers, Henry On 06/10/2014 08:45 PM, Phil Race wrote: Ditto -phil. On 6/5/14 7:34 PM, Joe Darcy wrote: Hi Henry, From a quick look, the changes seem fine. Thanks, -Joe On 06/05/2014 04:51 PM, Henry Jen wrote: Hi, Please review a follow-up to clean up rawtype and unchecked warnings in platform-specific code. Bug: https://bugs.openjdk.java.net/browse/JDK-8044396 https://bugs.openjdk.java.net/browse/JDK-8044551 Webrev: http://cr.openjdk.java.net/~henryjen/jdk9/8044551/0/webrev/ Cheers, Henry
Re: [OpenJDK 2D-Dev] JDK 9 RFR of JDK-8048980 : Fix raw and unchecked lint warnings in platform-specific sun.font files
Looks good to me.
Cheers,
Henry
On 07/03/2014 03:28 PM, Joe Darcy wrote:
A nice small fix, ready to review...
Thanks,
-Joe
On 07/01/2014 05:35 PM, Joe Darcy wrote:
Hello,
Please review this small change to address a few remaining unchecked
and raw types warnings in platform-specific sun.font code; full patch
below:
JDK-8048980 : Fix raw and unchecked lint warnings in
platform-specific sun.font files
http://cr.openjdk.java.net/~darcy/8048980.0/
Thanks,
-Joe
--- old/src/macosx/classes/sun/font/CFontConfiguration.java 2014-07-01
17:26:37.0 -0700
+++ new/src/macosx/classes/sun/font/CFontConfiguration.java 2014-07-01
17:26:37.0 -0700
@@ -106,6 +106,6 @@
@Override
protected void initReorderMap() {
-reorderMap = new HashMap();
+reorderMap = new HashMap<>();
}
}
--- old/src/solaris/classes/sun/font/FcFontConfiguration.java
2014-07-01 17:26:37.0 -0700
+++ new/src/solaris/classes/sun/font/FcFontConfiguration.java
2014-07-01 17:26:37.0 -0700
@@ -170,7 +170,7 @@
@Override
protected void initReorderMap() {
-reorderMap = new HashMap();
+reorderMap = new HashMap<>();
}
@Override
--- old/src/solaris/classes/sun/font/XMap.java2014-07-01
17:26:38.0 -0700
+++ new/src/solaris/classes/sun/font/XMap.java2014-07-01
17:26:37.0 -0700
@@ -37,7 +37,7 @@
class XMap {
-private static HashMap xMappers = new HashMap();
+private static HashMap xMappers = new HashMap<>();
/* ConvertedGlyphs has unicode code points as indexes and values
* are platform-encoded multi-bytes chars packed into java chars.
@@ -49,7 +49,7 @@
char[] convertedGlyphs;
static synchronized XMap getXMapper(String encoding) {
-XMap mapper = (XMap)xMappers.get(encoding);
+XMap mapper = xMappers.get(encoding);
if (mapper == null) {
mapper = getXMapperInternal(encoding);
xMappers.put(encoding, mapper);
--- old/src/solaris/classes/sun/font/XRGlyphCache.java 2014-07-01
17:26:38.0 -0700
+++ new/src/solaris/classes/sun/font/XRGlyphCache.java 2014-07-01
17:26:38.0 -0700
@@ -190,20 +190,23 @@
for (XRGlyphCacheEntry cacheEntry : glyphList) {
if (cacheEntry.isGrayscale(containsLCDGlyphs)) {
if (grayGlyphs == null) {
-grayGlyphs = new
ArrayList(glyphList.size());
+grayGlyphs = new ArrayList<>(glyphList.size());
}
cacheEntry.setGlyphSet(grayGlyphSet);
grayGlyphs.add(cacheEntry);
} else {
if (lcdGlyphs == null) {
-lcdGlyphs = new
ArrayList(glyphList.size());
+lcdGlyphs = new ArrayList<>(glyphList.size());
}
cacheEntry.setGlyphSet(lcdGlyphSet);
lcdGlyphs.add(cacheEntry);
}
}
-
-return new List[] { grayGlyphs, lcdGlyphs };
+// Arrays and generics don't play well together
+@SuppressWarnings({"unchecked", "rawtypes"})
+List[] tmp =
+(List[]) (new List[] { grayGlyphs,
lcdGlyphs });
+return tmp;
}
/**
