Skip to content

Commit

Permalink
Make Array.sort(float[]/double[]) JDK compliant
Browse files Browse the repository at this point in the history
This adds a test for double and float sorting that includes NaNs
and infinite values, and corrects the implementation of Arrays.sort()
to use Double.compare() and Float.compare(), which properly handle
NaN values.

Bug: #9504
Bug-Link: #9504
Change-Id: I2ad9abffbd38e70ba7d5a20011392d879383b7b8
  • Loading branch information
akbertram authored and gkdn committed Apr 7, 2017
1 parent eec1f90 commit 3d46828
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 24 deletions.
49 changes: 25 additions & 24 deletions user/super/com/google/gwt/emul/java/util/Arrays.java
Expand Up @@ -41,6 +41,7 @@
import java.util.stream.StreamSupport; import java.util.stream.StreamSupport;


import javaemul.internal.ArrayHelper; import javaemul.internal.ArrayHelper;
import javaemul.internal.DoubleCompareHolder;
import javaemul.internal.LongCompareHolder; import javaemul.internal.LongCompareHolder;


/** /**
Expand Down Expand Up @@ -1261,57 +1262,57 @@ public static void parallelSetAll(long[] array, IntToLongFunction generator) {
} }


public static void sort(byte[] array) { public static void sort(byte[] array) {
nativeNumberSort(array); nativeIntegerSort(array);
} }


public static void sort(byte[] array, int fromIndex, int toIndex) { public static void sort(byte[] array, int fromIndex, int toIndex) {
checkCriticalArrayBounds(fromIndex, toIndex, array.length); checkCriticalArrayBounds(fromIndex, toIndex, array.length);
nativeNumberSort(array, fromIndex, toIndex); nativeIntegerSort(array, fromIndex, toIndex);
} }


public static void sort(char[] array) { public static void sort(char[] array) {
nativeNumberSort(array); nativeIntegerSort(array);
} }


public static void sort(char[] array, int fromIndex, int toIndex) { public static void sort(char[] array, int fromIndex, int toIndex) {
checkCriticalArrayBounds(fromIndex, toIndex, array.length); checkCriticalArrayBounds(fromIndex, toIndex, array.length);
nativeNumberSort(array, fromIndex, toIndex); nativeIntegerSort(array, fromIndex, toIndex);
} }


public static void sort(double[] array) { public static void sort(double[] array) {
nativeNumberSort(array); nativeSort(array, DoubleCompareHolder.getDoubleComparator());
} }


public static void sort(double[] array, int fromIndex, int toIndex) { public static void sort(double[] array, int fromIndex, int toIndex) {
checkCriticalArrayBounds(fromIndex, toIndex, array.length); checkCriticalArrayBounds(fromIndex, toIndex, array.length);
nativeNumberSort(array, fromIndex, toIndex); nativeSort(array, fromIndex, toIndex, DoubleCompareHolder.getDoubleComparator());
} }


public static void sort(float[] array) { public static void sort(float[] array) {
nativeNumberSort(array); nativeSort(array, DoubleCompareHolder.getDoubleComparator());
} }


public static void sort(float[] array, int fromIndex, int toIndex) { public static void sort(float[] array, int fromIndex, int toIndex) {
checkCriticalArrayBounds(fromIndex, toIndex, array.length); checkCriticalArrayBounds(fromIndex, toIndex, array.length);
nativeNumberSort(array, fromIndex, toIndex); nativeSort(array, fromIndex, toIndex, DoubleCompareHolder.getDoubleComparator());
} }


public static void sort(int[] array) { public static void sort(int[] array) {
nativeNumberSort(array); nativeIntegerSort(array);
} }


public static void sort(int[] array, int fromIndex, int toIndex) { public static void sort(int[] array, int fromIndex, int toIndex) {
checkCriticalArrayBounds(fromIndex, toIndex, array.length); checkCriticalArrayBounds(fromIndex, toIndex, array.length);
nativeNumberSort(array, fromIndex, toIndex); nativeIntegerSort(array, fromIndex, toIndex);
} }


public static void sort(long[] array) { public static void sort(long[] array) {
nativeLongSort(array, LongCompareHolder.getLongComparator()); nativeSort(array, LongCompareHolder.getLongComparator());
} }


public static void sort(long[] array, int fromIndex, int toIndex) { public static void sort(long[] array, int fromIndex, int toIndex) {
checkCriticalArrayBounds(fromIndex, toIndex, array.length); checkCriticalArrayBounds(fromIndex, toIndex, array.length);
nativeLongSort(array, fromIndex, toIndex); nativeSort(array, fromIndex, toIndex, LongCompareHolder.getLongComparator());
} }


public static void sort(Object[] array) { public static void sort(Object[] array) {
Expand All @@ -1323,12 +1324,12 @@ public static void sort(Object[] array, int fromIndex, int toIndex) {
} }


public static void sort(short[] array) { public static void sort(short[] array) {
nativeNumberSort(array); nativeIntegerSort(array);
} }


public static void sort(short[] array, int fromIndex, int toIndex) { public static void sort(short[] array, int fromIndex, int toIndex) {
checkCriticalArrayBounds(fromIndex, toIndex, array.length); checkCriticalArrayBounds(fromIndex, toIndex, array.length);
nativeNumberSort(array, fromIndex, toIndex); nativeIntegerSort(array, fromIndex, toIndex);
} }


public static <T> void sort(T[] x, Comparator<? super T> c) { public static <T> void sort(T[] x, Comparator<? super T> c) {
Expand Down Expand Up @@ -1734,36 +1735,36 @@ private static void mergeSort(Object[] temp, Object[] array, int low,
} }


/** /**
* Sort an entire array of number primitives. * Sort an entire array using the given comparator
*/ */
private static native void nativeLongSort(Object array, Object compareFunction) /*-{ private static native void nativeSort(Object array, Object compareFunction) /*-{
array.sort(compareFunction); array.sort(compareFunction);
}-*/; }-*/;


/** /**
* Sort a subset of an array of number primitives. * Sort a subset of an array using the given comparator
*/ */
private static void nativeLongSort(Object array, int fromIndex, int toIndex) { private static void nativeSort(Object array, int fromIndex, int toIndex, Object compareFunction) {
Object temp = ArrayHelper.unsafeClone(array, fromIndex, toIndex); Object temp = ArrayHelper.unsafeClone(array, fromIndex, toIndex);
nativeLongSort(temp, LongCompareHolder.getLongComparator()); nativeSort(temp, compareFunction);
ArrayHelper.copy(temp, 0, array, fromIndex, toIndex - fromIndex); ArrayHelper.copy(temp, 0, array, fromIndex, toIndex - fromIndex);
} }


/** /**
* Sort an entire array of number primitives. * Sort an entire array of number primitives of integral type.
*/ */
private static native void nativeNumberSort(Object array) /*-{ private static native void nativeIntegerSort(Object array) /*-{
array.sort(function(a, b) { array.sort(function(a, b) {
return a - b; return a - b;
}); });
}-*/; }-*/;


/** /**
* Sort a subset of an array of number primitives. * Sort a subset of an array of primitives of integral type.
*/ */
private static void nativeNumberSort(Object array, int fromIndex, int toIndex) { private static void nativeIntegerSort(Object array, int fromIndex, int toIndex) {
Object temp = ArrayHelper.unsafeClone(array, fromIndex, toIndex); Object temp = ArrayHelper.unsafeClone(array, fromIndex, toIndex);
nativeNumberSort(temp); nativeIntegerSort(temp);
ArrayHelper.copy(temp, 0, array, fromIndex, toIndex - fromIndex); ArrayHelper.copy(temp, 0, array, fromIndex, toIndex - fromIndex);
} }


Expand Down
@@ -0,0 +1,27 @@
/*
* Copyright 2017 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package javaemul.internal;

/**
* A helper class for double comparison.
*/
public class DoubleCompareHolder {
// TODO(dankurka): replace this with @JsFunction once its available in GWT.
public static native Object getDoubleComparator() /*-{
return @java.lang.Double::compare(*);
}-*/;
}

50 changes: 50 additions & 0 deletions user/test/com/google/gwt/emultest/java/util/ArraysTest.java
Expand Up @@ -1320,6 +1320,56 @@ public void testPrimitiveSort() {
assertTrue(Arrays.equals(new int[]{Integer.MIN_VALUE, 1, 2, 3, 3, Integer.MAX_VALUE}, array)); assertTrue(Arrays.equals(new int[]{Integer.MIN_VALUE, 1, 2, 3, 3, Integer.MAX_VALUE}, array));
} }


/**
* Tests sorting of doubles
*/
public void testDoubleSort() {
double[] array = new double[] {
41.5, Double.NaN, Double.NaN, 3, 932535, 1, Double.NaN,
-3, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY, Double.NaN};
Arrays.sort(array);

// Test the array element-by-element
// as GWT's Arrays.equal() does not consider NaNs "equal"
assertEquals(Double.NEGATIVE_INFINITY, array[0]);
assertEquals(-3.0, array[1]);
assertEquals(1.0, array[2]);
assertEquals(3.0, array[3]);
assertEquals(41.5, array[4]);
assertEquals(932535.0, array[5]);
assertEquals(Double.POSITIVE_INFINITY, array[6]);
assertTrue(Double.isNaN(array[7]));
assertTrue(Double.isNaN(array[8]));
assertTrue(Double.isNaN(array[9]));
assertTrue(Double.isNaN(array[10]));
}

/**
* Tests sorting of floats
*/
public void testFloatSort() {
float[] array = new float[] {
41.5f, Float.NaN, Float.NaN, 3, 932535, 1, Float.NaN,
-3, Float.POSITIVE_INFINITY, Float.NEGATIVE_INFINITY, Float.NaN};
Arrays.sort(array);

// Test the array element-by-element
// as GWT's Arrays.equal() does not consider NaNs "equal"
assertEquals(Float.NEGATIVE_INFINITY, array[0]);
assertEquals(-3.0f, array[1]);
assertEquals(1.0f, array[2]);
assertEquals(3.0f, array[3]);
assertEquals(41.5f, array[4]);
assertEquals(932535.0f, array[5]);
assertEquals(Float.POSITIVE_INFINITY, array[6]);
assertTrue(Float.isNaN(array[7]));
assertTrue(Float.isNaN(array[8]));
assertTrue(Float.isNaN(array[9]));
assertTrue(Float.isNaN(array[10]));
}



/** /**
* Tests sorting a subrange of a primitive array. * Tests sorting a subrange of a primitive array.
*/ */
Expand Down

0 comments on commit 3d46828

Please sign in to comment.