David Daney wrote:
Casey Marshall wrote:
On Dec 13, 2006, at 12:21 PM, David Daney wrote:
Casey Marshall wrote:
On Dec 13, 2006, at 12:56 AM, Marco Trudel wrote:
Hey guys
java.util.Arrays.binarySearch(Object[] a, Object key, Comparator
c) exchanges a[i] and key, this can lead to ClassCastExceptions
as shown in ComparatorTest.java. Arrays.patch fixes it.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30061
I think we all agree that classpath follows the specs and that the
comparators are wrong implemented (Andrew pointed that out already). So
this is not the discussion...
This exact problem bit me once. I developed code on the RI that
inadvertently had a comparator that depended on the order of the
arguments.
I freely admit that my code was incorrect, but it did take me a
while to figure out what was happening.
The problem is that Classpath is doing it inconsistent with a Sun JRE.
This actively breaks user code and possible third party libraries that a
user might not be interested or even able to fix. See below.
I feel that if there is no performance penalty, then we should
follow the behavior of the RI.
Absolutely. My arguments:
1. As I said already, this is actually a classpath regression.
First I debugged with an old classpath checkout I had lying around and
couldn't find the bug. So I updated my revision and saw that a
refactoring of Arrays.binarySearch() introduced the problem.
2. Keeping the argument order from Arrays.binarySort() to
Comparator.compare() is logical. Although not required by the spec, this
is what a user expects.
3. This breaks code/libraries that worked fine with a Sun JRE.
4. There's no loss in swapping the order as proposed by the patch. It
only solves problems and still fulfills the specification 100% as
before. The only difference is that it will conform to RI and to old
implementations in Classpath.
In general I really urge against going out of our way to support
behavior like this.
Can you explain that further? You're against changes that
- keep the specification as correct as they were before
- will adapt classpath to the RI behavior
- will prevent users from running into problems
- restore old classpath behavior (regression fix)
- have no negative side effects whatsoever
Sorry if that seems to be a mean summary. This are just the points I'm
seeing and I'm actually really surprised about the number and kind of
reactions to that patch or that the mentioned bugreport has been closed
as invalid.
I would have assumed that someone says: "Hey great, this only fixes
problems and doesn't do anything bad. Let's commit it".
So as I said, I don't mean to offend you, I just can't follow why we
wouldn't follow the RI when we still fulfill the specifications.
I changed my mind! As a matter of principle, since I had to fix my
code, everyone should have to suffer in the same manner, as their
penalty for disobeying the mandates of the specification! :)
Whaaat? David! I really hope - for the sake of humanity - that you're
joking. Ah right, there's a ":)" at the end. I was so shocked from that
statement that I missed it first...
Marco
