Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Received-SPF: neutral (athena.apache.org: 194.196.100.164 is neither permitted
 nor denied by domain of mark.hindess@googlemail.com)
Message-Id: <201002172019.o1HKJf33002559@d06av01.portsmouth.uk.ibm.com>
In-reply-to: <20100217192622.3B6B847841B@athena.apache.org>
References: <a43fbc6a1002170835l7d149233ib59d0915009561ab@mail.gmail.com>
 <201002171703.o1HH3GNp017551@d12av04.megacenter.de.ibm.com>
 <a43fbc6a1002171030p15b49041pee64ed782ad56c67@mail.gmail.com>
 <20100217192622.3B6B847841B@athena.apache.org>
Comments: In-reply-to Mark Hindess <mark.hindess@googlemail.com>
   message dated "Wed, 17 Feb 2010 19:25:32 +0000."
From: Mark Hindess <mark.hindess@googlemail.com>
To: dev@harmony.apache.org
Subject: Re: svn commit: r910980 -
 /harmony/enhanced/classlib/branches/java6/modules/luni/src/main/java/java/util/TreeMap.java
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Date: Wed, 17 Feb 2010 20:19:41 +0000


In message <20100217192622.3B6B847841B@athena.apache.org>, Mark Hindess writes:
>
> In message <a43fbc6a1002171030p15b49041pee64ed782ad56c67@mail.gmail.com>,
> Jesse Wilson writes:
> >
> > On Wed, Feb 17, 2010 at 9:03 AM, Mark Hindess
> > <mark.hindess@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.

> > > 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.
> >
> > Sounds good.
>
> I've reopened (but removed the must-fix-for-6.0M1 status) the JIRA at:
> 
>   https://issues.apache.org/jira/browse/HARMONY-6448
> 
> to track this issue.

Closed again as you opened HARMONY-6451 to track it.

I'll update this thread and the JIRA when I've fixed it.

Regards,
-Mark.