On 1/10/12 4:35 PM, Kurchi Hazra wrote:
Updated webrev with all the above changes incorporated:
http://cr.openjdk.java.net/~khazra/7117570/webrev.04/

SnmpNamedListTableCache.java
L216,221,225: should it be List<?> rather than List<Object>?
Will that help get rid of the unchecked suppressed warning?
- It does remove the suppressed warning added to this function, but will
give rise to added suppress warnings in other places. For example:

src/share/classes/sun/management/snmp/util/SnmpListTableCache.java:109
(since now the argument passed to be has to be List<?>, else the compiler
complains)

Do you still want me to change it to List<?> and not List<Object>?

I was looking at this too. Arguably the best conversion of a raw type List to generics is List<?> but as you noted this causes a ripple effect, where other things have to be converted to wildcards as well. It looks like you have to change updateCachedDatas() and SnmpListTableCache.updateCachedDatas(), but then that's it. That doesn't look too bad. It could be that I've missed something and that you'll have to change more and more, in which case I'd reconsider making the change to use wildcards.

You mentioned in your previous email that sun.management and its
subpackages are warning free with your changeset but I suspect
there are still warnings e.g.
JvmMemoryImpl.java L160 - this casts the key to MemoryUsage.
This and I also see some other casts that are somehow not being
reported even if I turn on -Werror in make/sun/management/snmp. I will
let this be for the time being and first get this changeset pushed. Probably
in a new CR I will try adding -Werror to make/java/management and
make/sun/management.

This cast doesn't generate an unchecked warning, because it *is* checked -- the VM checks the downcast from Object to MemoryUsage and throws ClassCastException if there's a mismatch. The compiler only emits unchecked warnings when the cast can't be checked at compile time *and* it can't be checked at runtime.

Offhand I don't know whether this code is correct in making this cast. (The cast is *checked* but it might not be *safe*.) The code does catch RuntimeException and it does something reasonable, so maybe it's OK. This effort is about cleaning up warnings, though, and since there's no warning here I don't think we need to worry about it.

s'marks




Thanks,
Kurchi



Did you get a chance to check the incremental build and see if
there are warnings or not? e.g. cd sun/management; make clean all
I suspect the snmp code still has compiler warnings but that's fine
since it's very old code that requires more cleanup work for the
future.

Mandy

On 1/9/2012 12:02 PM, Kurchi Hazra wrote:
Hi,

As an effort to cleanup build warnings, this webrev involves small changes in
sun.management.* and its subpackages:

Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7117570
Webrev: http://cr.openjdk.java.net/~khazra/7117570/webrev.03/


Thanks,
Kurchi

Reply via email to