Code Review Request: 8016698: Cleanup overrides warning in sun/tools/ClassDeclaration.java

[Kurchi Hazra]

On 6/14/2013 11:56 PM, Alan Bateman wrote:
> On 14/06/2013 23:59, Kurchi Hazra wrote:
>>
>> Hi,
>> This is to elimnate the overrides warning from ClassDeclaration.java. 
>> This class is used by rmi, but sun/tools/java and
>> sun/tools/javac also heavily uses it, let me know if anyone else 
>> should also be reviewing it.
>>
>> Bug: http://bugs.sun.com/view_bug.do?bug_id=8016698 [To appear]
>> Webrev: http://cr.openjdk.java.net/~khazra/8016698/webrev.00/
>>
>> Thanks,
>> Kurchi
> This looks okay for the purposes of squashing the warning but the 
> existing equals method does look suspect. It might not matter for the 
> rmic usage but as the equals doesn't take account of the the 
> "definition" then it looks to me that it consider very different 
> ClassDeclarations as equals. It would be good to understand how it is 
> used to see whether we have an issue here or not.
Thanks for the review.
If you analyse "definition", it is not constant and changes as the state 
of the class changes (whether binary class is loaded, whether it
has been compiled and so on) - and so does the status field. The type 
field looks like the only good candidate to use in the equals method -
so the equals() looks correct to me.
Now the question is whether there is any need of overriding 
Object.equals() at all, that is, whether each ClassDeclaration object should
be treated as a unique one - I am looking around in the source code to 
check that.

Thanks,
- Kurchi