Marco pinged me about various patches of his, and in response I
re-read this whole thread.
I am going to check in this patch. I have three reasons for this,
based on a few thought experiments.
First, I tried to imagine under what conditions Sun could change their
implementation. That is pretty hard to picture. This API is
well-defined, the code is very unlikely to have a reportable bug, and
if the implementation did change, it would break actually existing
code. So, if somehow Sun had to decide to change anything here, I
think the only thing they could reasonably do would be to document the
order.
Second, I tried to imagine an alternate universe where, years and
years ago, Eric Blake happened to pick the other order for comparisons
here. In this universe we would never have looked at or considered
this code for change at all. So in some sense this change has a lower
energy than the code-as-is.
I'm sympathetic to the idea that users ought to fix their code -- they
ought to. But I think it is more important that we make existing
programs work. The final thought experiment for me was someone trying
to run an applet in mozilla and running across this issue. Their
take-away from this will be "Classpath is broken" -- since it works
fine on proprietary VMs.
Marco, I changed your comment a bit.
Tom
Index: ChangeLog
from Marco Trudel <[EMAIL PROTECTED]>
* java/util/Arrays.java (binarySearch): Change comparison order.
Index: java/util/Arrays.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/Arrays.java,v
retrieving revision 1.34
diff -u -r1.34 Arrays.java
--- java/util/Arrays.java 8 Jan 2007 17:55:34 -0000 1.34
+++ java/util/Arrays.java 20 Jan 2007 02:01:37 -0000
@@ -1,5 +1,5 @@
/* Arrays.java -- Utility class with methods to operate on arrays
- Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006,
+ Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007,
Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -639,10 +639,13 @@
while (low <= hi)
{
mid = (low + hi) >>> 1;
- final int d = Collections.compare(key, a[mid], c);
+ // NOTE: Please keep the order of a[mid] and key. Although
+ // not required by the specs, the RI has it in this order as
+ // well, and real programs (erroneously) depend on it.
+ final int d = Collections.compare(a[mid], key, c);
if (d == 0)
return mid;
- else if (d < 0)
+ else if (d > 0)
hi = mid - 1;
else
// This gets the insertion point right on the last loop
