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
