In message <201002172019.o1hkjf33002...@d06av01.portsmouth.uk.ibm.com>, Mark Hindess writes: > > > In message <20100217192622.3b6b8478...@athena.apache.org>, Mark > Hindess writes: > > > In message > > <a43fbc6a1002171030p15b49041pee64ed782ad56...@mail.gmail.com>, Jesse > > Wilson writes: > > > > > On Wed, Feb 17, 2010 at 9:03 AM, Mark Hindess > > > <mark.hind...@googlemail.com> wrote: > > > > > > > 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. > > > > > > That's curious, but I guess I haven't looked into the navigable > > > stuff that's new in 1.6. > > > > > > > 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. > > > > > > I think toComparable() is the right place. TreeMaps operates in > > > two different modes: > > > > > > - *With a comparator.* In this mode objects don't need to be > > > comparable. Null is permitted, because comparators can decide > > > where null fits in the total ordering. > > > > > - *Without a comparator.* In this mode everything must be > > > comparable. Null is not comparable and must be forbidden. > > > > That was my understanding too but I'm assuming it is not that > > simple. I suppose the other option is that toComparable is being > > called when it should not be. > > Turns out it is simply a problem with the ordering of the checks for: > > a) null keys > b) empty maps > > For several method the RI obviously checks if a map is empty and returns > null with no exception regardless of the key where as with your fix the > null key causes an exception first. A number of methods are protected > with an isEmpty check - root == null - that short circuits the full > traversal but some are not. I'll fix them shortly and re-instate your > fix.
Fixed in r911169. -Mark.
