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
