RFR: 8022479: clean up warnings from sun.tools.asm

Stuart Marks stuart.marks at oracle.com
Wed Aug 7 06:28:55 UTC 2013


Hi,

Please review the fix for this warnings cleanup bug.

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8022479
     (not yet available publicly, but should be shortly)

Webrev: http://cr.openjdk.java.net/~smarks/reviews/8022479/webrev.0/

There are a few items of note.

This is *very* old code. You can see some of the authors' names in the code. 
It's replete with uses of Vector, Hashtable, and Enumeration. It also has a 
kind of musty style, which might have been reasonable from the standpoint of C 
programmers (which we all were at the time!). For example, there are a bunch of 
cases of direct field access to another class. There are also cases where a 
class contains a Vector that's returned by a getter method. I guess we just 
have to trust the caller not to modify it at the wrong time.

I've confined most of my cleanups to the addition of generics (which gets rid 
of the rawtypes and unchecked warnings). There were a smattering of other 
warnings I've cleaned up as well. I've also replaced few cases of calls to 
"new" on boxed types like Integer and Long with calls to the respective 
valueOf() methods. I didn't check all the code for this, though, just instances 
I happened to run across.

There is much more cleanup that could be done that I've elected not to do, such 
as replacing Vector, Hashtable, and Enumeration with ArrayList, HashMap, and 
Iterator, and using the enhanced-for loop. These changes would *probably* be 
safe, but they would require more analysis and testing than I'm willing to do 
at the moment.

Two locations deserve a bit of scrutiny.

1) There are four occurrences in Assembler.java of calls to 
MemberDefinition.getArguments(). Unfortunately MemberDefinition resides in the 
sun.tools.java package and getArguments() returns a raw Vector. This is also 
overridden in a couple places spread across a couple sun.tools packages, and 
there are hints that it being a LocalMember or MemberDefinition. It seems out 
of scope to try to fix up the return value of getArguments() and its various 
overrides.

Locally in the Assembler.java file, though, the contents of the returned vector 
are always cast to MemberDefinition, so it seems sensible to make an unchecked 
cast of the getArguments() return value to Vector<MemberDefinition> and to 
suppress warnings at that point. Usage beyond those points within 
Assembler.java is filled with sweet and juicy generic goodness.

2) SwitchData.java contains a field whereCaseTab which is a 
Hashtable<Integer,Long> ... most of the time. In the addTableDefault() method, 
a value is put into the Hashtable under the key "default" -- a String. Ick.

To deal with this I declared it as Hashtable<Integer,Long> since that's the 
typical case, and for the put(String, Long) case I cast through raw and 
suppressed the warning. The get() method already takes an Object so we're OK 
there. Note that "default" as a key for this Hashtable is used in a couple 
other places in Assembler.java. It's not intractable to change these to some 
other sentinel value, but, you know, where does one stop?

(I said "Ick" when encountering a mix-type collection, and in a generified 
world we very rarely see this style anymore. Upon further reflection, though, 
from the standpoint of a pre-generics world, this is pretty sensible. Using a 
string as a sentinel key is guaranteed not to collide with any Integer keys. 
Sometimes it's possible to use -1 or Integer.MAX_VALUE as a sentinel, but if 
any integer value is permitted as a key, what does one do then?)

With this changeset, sun.tools.asm is reduced from about 76 to zero warnings.

Thanks,

s'marks



More information about the core-libs-dev mailing list