In message <a43fbc6a1002170835l7d149233ib59d091500956...@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. Regards, Mark.
