harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hindess <mark.hind...@googlemail.com>
Subject Re: svn commit: r910980 - /harmony/enhanced/classlib/branches/java6/modules/luni/src/main/java/java/util/TreeMap.java
Date Wed, 17 Feb 2010 17:03:14 GMT

In message <a43fbc6a1002170835l7d149233ib59d0915009561ab@mail.gmail.com>,
Jesse Wilson writes:
> I noticed something weird in commit 910980:
> Author: hindessm
> Date: Wed Feb 17 14:07:42 2010
> New Revision: 910980
> URL: http://svn.apache.org/viewvc?rev=910980&view=rev
> Log:
> Thought I'd reverted this already ... reverting r903454.  See:
>   http://markmail.org/thread/cdxlmi26mjgor3om
> Modified:
>     harmony/enhanced/classlib/branches/java6/modules/luni/src/main/java/java/
> util/TreeMap.java
> I think you're moving in the wrong direction. The toComparable method
> needs a null check. You can see this by reading the TreeMap code from
> the Java 5
> branch<http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/l
> uni/src/main/java/java/util/TreeMap.java>.
> Harmony doesn't have explicit test coverage here, but you can verify
> the bug by running other publicly-available JDK test suites against
> Harmony.
> Mark, can you revert this commit?

I have solid evidence that this change is wrong... it breaks Harmony
tests that pass on the RI.  I've no solid evidence that this change is
required.  So my inclination is not to put the check back.

A null check may be required but I'm not convinced this is the right
place for it.  The toComparable method is just a trivial helper function
that is clearly being used (probably quite reasonably) in a context that
doesn't require a null check.

Sorry, I've not got time to investigate this further right now but
rest assured we will get to the bottom of it and fix it correctly.
Raise a JIRA if you want to make sure it doesn't get forgotten.


View raw message