Review request for java.text.** warning cleanup (original) (raw)

Deepak Bhole dbhole at redhat.com
Mon Dec 19 10:01:04 PST 2011

As Andrew already mentioned, I was away and got back on Friday. I've just sent a message to John to get the final matters sorted out. Hopefully this will get pushed soon!

Cheers, Deepak

s'marks

On 12/6/11 5:26 PM, John Rose wrote: >On Dec 5, 2011, at 12:30 PM, Deepak Bhole wrote: > >>> Since this is an important point, I checked into it. Result: Wildcards >>> cannot (in general) be compatibly switched with named type parameters >>> in public APIs. >>> See the demonstration here: >>> [1]https://gist.github.com/1424776 >>> (This is subject to correction by someone more expert than me in API >>> evolution, like Alex Buckley, Joe Darcy, or Maurizio Cimadamore. I'm a >>> JVM hacker!) >>> >> >>Thanks for the pointer! Yes, it is rather unfortunate that it cannot >>changed then :( I have reverted it and added a more localized @SW >>instead. > >Good. I puzzled some more over the localized @SW and found a formulation >that does not require any @SW at all. It's a matter of choosing wildcards very >carefully. See patch below. > >Basically, it's one of those cases where a type T isn't good enough, but a type >"? extends T" will do the job. (I can't tell you all such cases, but this is one of them!) > >>> I'm OK with that. Replacing clone by an equivalent copy constructor >>> looks trivial enough to me, even though it changes bytecode output. >>> >> >>As mentioned in my other email to Rémi, the clone constructor was my >>initial plan but didn't work. Instead I am just initializing a temp var >>(so that I can add a localized @SW) and then assigning the outer scope >>var to it. > >Good. > >>> Yep. SW("serial") seems to be the wrong thing, almost everywhere. >> >>Out of curiosity, why is this? Is it because not having a serial can >>cause invalid class errors or are there other reasons? > >If you don't have a SVUID, the system makes one for you. (It is the same >one your IDE can generate explicitly.) If you way @SW("serial"), you are >telling the system that it can adjust the SVUID at will. You get into trouble >with this if somebody adds another (non-transient) field. This changes the >implicit SVUID. This can lead to surprising results down the road. > >>Thanks for taking a look. New webrev: >>http://cr.openjdk.java.net/~dbhole/java.text-warning-cleanup/webrev.02/ >> >>As with before, diff between rev 01 and 02 is there too: >>http://cr.openjdk.java.net/~dbhole/java.text-warning-cleanup/webrev.02/rev-01-02.patch > > > >On Dec 2, 2011, at 2:12 PM, John Rose wrote: > >>>>In the same way, consider pushing the annotation into the body of class AttributeEntry, even though that class is very simple. >>>> >>> >>>That one is for the declaration itself -- the warning is about the Map >>>supertype not being parameterized. >> >> >>I didn't get that clearly from the comment. Suggest: >>+// Suppress warning about raw Map.Entry. Parameterizing Map.Entry correctly >>+// requires changing return of getKey >>++// Must suppress the whole class to suppress warning about raw supertype Map.Entry. >>++// Parameterizing Map.Entry correctly would requires changing return of getKey. > >I went back to class AttributeEntry and realized that it also does not need @SW. >Since it is a non-public class, it is OK to change the supertype and the return value >of getKey. A simple correction to the type parameters of a non-public name is in >scope for this project, as I read Stuart here: > http://mail.openjdk.java.net/pipermail/jdk8-dev/2011-December/000380.html > >-+// Must suppress the whole class to suppress warning about raw supertype Map.Entry. >-+// Parameterizing Map.Entry correctly would requires changing return of getKey. >-+ at SuppressWarnings("rawtypes") >-class AttributeEntry implements Map.Entry { >++class AttributeEntry implements Map.Entry<Attribute,Object> { >... >- public Object getKey() { >++ public Attribute getKey() { > >(I could be mistaken about this; please double check. I made a public API >mistake in my own java.lang.invoke warnings cleanup!) > >Also, I noticed some extraneous blank lines surrounding @SW occurrences. >They aren't needed. Also, a few occurrences of @SW({"foo"}) do not need >the braces: @SW("foo"). > >Here are all the adjustments I suggest, in one patch: > https://gist.github.com/1440966 > >See if they work for you. If so, we're done. > >-- John

More information about the jdk8-dev mailing list