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

[Kurchi Hazra]

Yes, I have that removed. Thanks for the review!

- Kurchi

On 6/18/2013 1:57 AM, Chris Hegarty wrote:
> For the purposes of this issue, I think your changes to hashCode are 
> in line with the equals implementation. Trivially, you could also 
> remove the '(obj != null) && ' from equals ;-) Otherwise, looks fine 
> to me.
>
> -Chris.
>
> On 06/18/2013 12:01 AM, Kurchi Hazra wrote:
>> I spent some more time reviewing this, I believe the equals()
>> implementation is correct. Given one string, we get a unique
>> Identifier. The Identifier has the associated Type cached - so given an
>> Identifier, we get a unique Type too. One usage of the
>> ClassDeclaration#equals() method is in ClassDefinition#subClassOf(),
>> which walks up the inheritance path of the concerned
>> class to check if any of those are equal to the argument
>> ClassDeclaration (and hence have equal Types)..
>>
>> - Kurchi
>>
>> On 6/17/2013 10:45 AM, Kurchi Hazra wrote:
>>>
>>> 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
>>

-- 
-Kurchi