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
