Hi Andrew and Pekka, Thank you for your efforts (regarding patch for Collections.emptySet/Map/List).
To continue pushing the patches, I've cherry-picked 2 patches (to https://github.com/ivmai/classpath/tree/ivmai4review-v3) containing StrictMath fixes and the corresponding tests: https://github.com/ivmai/classpath/commit/b0103d175bb012fcec411cc9e76257f3f257c39d (StrictMath changes) https://github.com/ivmai/classpath/commit/214c8bab72dc4689edf7ae1cd49e3a109c0746d7 (StrictMath test) Some notes: - In the 1st patch I've removed several trailing spaces at EOLn in ChangeLog - I think it is ok not to post this as a separate patch - In the 2nd patch I've also fixed java.lang.reflect/ArrayTest (a one-line fix - I've commented out loadLibrary call but it might be better to remove this line at all) 1st patch (ChangeLog diff is omitted): commit b0103d175bb012fcec411cc9e76257f3f257c39d Author: Ivan Maidanski <iv...@mail.ru> Date: Tue Jul 19 21:14:16 2011 +0400 Fix bugs in several StrictMath methods 2011-07-19 Ivan Maidanski <iv...@mail.ru> * java/lang/StrictMath.java: (acos(double)): Bug fix for x <= -1/2 case. (atan(double)): Fix documentation typo. (pow(double,double)): Fix a comment; put y == 1/2 case handling after x == -0 case (since pow(-0, 1/2) == 0 but sqrt(-0) == -0); return -0 (or -INF) for pow(-0, 2*k) where k is non-zero integer; simplify expression for "negative" variable. (IEEEremainder(double,double)): Bug fix for x == -0 and x == -|y| cases; bug fix for |y| >= 2**-1021 and |x| > |y|/2. (remPiOver2(double[],double[],int,int)): Reset "recompute" at the beginning of every do/while iteration. (tan(double,double,boolean)): Negate the result if x <= -0.6744. diff --git a/java/lang/StrictMath.java b/java/lang/StrictMath.java index 88f5e57..b4fe411 100644 --- a/java/lang/StrictMath.java +++ b/java/lang/StrictMath.java @@ -1,5 +1,6 @@ /* java.lang.StrictMath -- common mathematical functions, strict Java - Copyright (C) 1998, 2001, 2002, 2003, 2006 Free Software Foundation, Inc. + Copyright (C) 1998, 2001, 2002, 2003, 2006, 2010 + Free Software Foundation, Inc. This file is part of GNU Classpath. @@ -456,9 +457,10 @@ public final strictfp class StrictMath double r = x - (PI_L / 2 - x * (p / q)); return negative ? PI / 2 + r : PI / 2 - r; } + double z = (1 - x) * 0.5; if (negative) // x<=-0.5. { - double z = (1 + x) * 0.5; + // z = (1+orig_x)*0.5 double p = z * (PS0 + z * (PS1 + z * (PS2 + z * (PS3 + z * (PS4 + z * PS5))))); double q = 1 + z * (QS1 + z * (QS2 + z * (QS3 + z * QS4))); @@ -466,8 +468,7 @@ public final strictfp class StrictMath double w = p / q * s - PI_L / 2; return PI - 2 * (s + w); } - double z = (1 - x) * 0.5; // x>0.5. - double s = sqrt(z); + double s = sqrt(z); // x>=0.5 double df = (float) s; double c = (z - df * df) / (s + df); double p = z * (PS0 + z * (PS1 + z * (PS2 + z * (PS3 + z @@ -478,12 +479,12 @@ public final strictfp class StrictMath } /** - * The trigonometric function <em>arcsin</em>. The range of angles returned + * The trigonometric function <em>arctan</em>. The range of angles returned * is -pi/2 to pi/2 radians (-90 to 90 degrees). If the argument is NaN, the * result is NaN; and the arctangent of 0 retains its sign. * * @param x the tan to turn back into an angle - * @return arcsin(x) + * @return arctan(x) * @see #atan2(double, double) */ public static double atan(double x) @@ -1520,8 +1521,8 @@ public final strictfp class StrictMath if (x != x || y != y) return Double.NaN; - // When x < 0, yisint tells if y is not an integer (0), even(1), - // or odd (2). + // When x < 0, yisint tells if y is not an integer (0), odd (1), + // or even (2). int yisint = 0; if (x < 0 && floor(y) == y) yisint = (y % 2 == 0) ? 2 : 1; @@ -1539,8 +1540,6 @@ public final strictfp class StrictMath } if (y == 2) return x * x; - if (y == 0.5) - return sqrt(x); // More special cases, of x. if (x == 0 || ax == Double.POSITIVE_INFINITY || ax == 1) @@ -1554,10 +1553,18 @@ public final strictfp class StrictMath else if (yisint == 1) ax = -ax; } + else + { + // Check for x == -0 with odd y. + if (1 / x < 0 && ay % 2 == 1) // yisint cannot be used here + ax = -ax; + } return ax; } if (x < 0 && yisint == 0) return Double.NaN; + if (y == 0.5) // x!=0 + return sqrt(x); // Now we can start! double t; @@ -1639,7 +1646,7 @@ public final strictfp class StrictMath } // Split up y into y1+y2 and compute (y1+y2)*(t1+t2). - boolean negative = x < 0 && yisint == 1; + boolean negative = yisint == 1; // Implies x<0. double y1 = (float) y; double p_l = (y - y1) * t1 + y * t2; double p_h = y1 * t1; @@ -1690,12 +1697,14 @@ public final strictfp class StrictMath if (x == Double.NEGATIVE_INFINITY || ! (x < Double.POSITIVE_INFINITY) || y == 0 || y != y) return Double.NaN; + if (x == 0) // Filter out -0 case. + return x; + if (x == y || -x == y) + return 0 * x; // Get correct sign. boolean negative = x < 0; x = abs(x); y = abs(y); - if (x == y || x == 0) - return 0 * x; // Get correct sign. // Achieve x < 2y, then take first shot at remainder. if (y < TWO_1023) @@ -1713,11 +1722,11 @@ public final strictfp class StrictMath } else { - y *= 0.5; - if (x > y) + double half = y * 0.5; + if (x > half) { x -= y; - if (x >= y) + if (x >= half) x -= y; } } @@ -2186,7 +2195,7 @@ public final strictfp class StrictMath int[] iq = new int[20]; double[] f = new double[20]; double[] q = new double[20]; - boolean recompute = false; + boolean recompute; // Initialize jk, jz, jv, q0; note that 3>q0. int jk = 4; @@ -2210,6 +2219,7 @@ public final strictfp class StrictMath do { + recompute = false; // Distill q[] into iq[] reversingly. for (i = 0, j = jz, z = q[jz]; j > 0; i++, j--) { @@ -2482,15 +2492,16 @@ public final strictfp class StrictMath v = invert ? -1 : 1; return (negative ? -1 : 1) * (v - 2 * (x - (w * w / (w + v) - r))); } - if (! invert) - return w; - - // Compute -1.0/(x+r) accurately. - z = (float) w; - v = r - (z - x); - double a = -1 / w; - double t = (float) a; - return t + a * (1 + t * z + t * v); + if (invert) + { + // Compute -1.0/(x+r) accurately. + z = (float) w; + v = r - (z - x); + double a = -1 / w; + double t = (float) a; + w = t + a * (1 + t * z + t * v); + } + return negative ? -w : w; } /** 2nd patch (ChangeLog omitted): commit 214c8bab72dc4689edf7ae1cd49e3a109c0746d7 Author: Ivan Maidanski <iv...@mail.ru> Date: Tue Jul 19 21:35:14 2011 +0400 Add StrictMath tests 2011-07-19 Ivan Maidanski <iv...@mail.ru> * test/Makefile.am: Add java.lang.reflect and java.lang to SUBDIRS. * test/java.lang/Makefile.am: New file. * test/java.lang/StrictMathTest.java: Likewise. * test/java.lang.reflect/ArrayTest.java: (main(String[])): Comment out loadLibrary() call (since this should be done by VMArray implementation). diff --git a/ChangeLog b/ChangeLog index 2b26411..cf5abf5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2011-07-19 Ivan Maidanski <iv...@mail.ru> + * test/Makefile.am: Add java.lang.reflect and java.lang to SUBDIRS. + * test/java.lang/Makefile.am: New file. + * test/java.lang/StrictMathTest.java: Likewise. + * test/java.lang.reflect/ArrayTest.java: (main(String[])): Comment + out loadLibrary() call (since this should be done by VMArray + implementation). + +2011-07-19 Ivan Maidanski <iv...@mail.ru> + * java/lang/StrictMath.java: (acos(double)): Bug fix for x <= -1/2 case. (atan(double)): Fix documentation typo. diff --git a/test/Makefile.am b/test/Makefile.am index bd3972a..0a0767e 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -1,4 +1,4 @@ ## Input file for automake to generate the Makefile.in used by configure -SUBDIRS = java.net java.io java.util gnu.java.lang.reflect - +SUBDIRS = gnu.java.lang.reflect java.lang java.lang.reflect java.net \ + java.io java.util diff --git a/test/java.lang.reflect/ArrayTest.java b/test/java.lang.reflect/ArrayTest.java index 4433e9b..8357360 100644 --- a/test/java.lang.reflect/ArrayTest.java +++ b/test/java.lang.reflect/ArrayTest.java @@ -2,7 +2,7 @@ import java.lang.reflect.Array; public class ArrayTest { public static void main(String[] args) { - System.loadLibrary("javalangreflect"); + //System.loadLibrary("javalangreflect"); Object[] objArray = new Object[9]; boolean[] boolArray = new boolean[9]; @@ -27,7 +27,7 @@ public class ArrayTest { E.printStackTrace(); } System.out.println(": newInstance(<primitive Class>,int)"); - + try { objArray = (Object[])Array.newInstance(java.lang.Object.class, 9); System.out.print(objArray != null ? "PASSED" : "FAILED"); diff --git a/test/java.lang/Makefile.am b/test/java.lang/Makefile.am new file mode 100644 index 0000000..bbd9b0a --- /dev/null +++ b/test/java.lang/Makefile.am @@ -0,0 +1,3 @@ +## Input file for automake to generate the Makefile.in used by configure + +check_JAVA = StrictMathTest.java diff --git a/test/java.lang/StrictMathTest.java b/test/java.lang/StrictMathTest.java new file mode 100644 index 0000000..345a321 --- /dev/null +++ b/test/java.lang/StrictMathTest.java @@ -0,0 +1,131 @@ +/* StrictMathTest.java -- tests StrictMath class + Copyright (C) 2010 Free Software Foundation, Inc. + +This file is part of GNU Classpath. + +GNU Classpath is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2, or (at your option) +any later version. + +GNU Classpath is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GNU Classpath; see the file COPYING. If not, write to the +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +02110-1301 USA. + +Linking this library statically or dynamically with other modules is +making a combined work based on this library. Thus, the terms and +conditions of the GNU General Public License cover the whole +combination. + +As a special exception, the copyright holders of this library give you +permission to link this library with independent modules to produce an +executable, regardless of the license terms of these independent +modules, and to copy and distribute the resulting executable under +terms of your choice, provided that you also meet, for each linked +independent module, the terms and conditions of the license of that +module. An independent module is a module which is not derived from +or based on this library. If you modify this library, you may extend +this exception to your version of the library, but you are not +obligated to do so. If you do not wish to do so, delete this +exception statement from your version. */ + + +/** + * @author Ivan Maidanski + */ +public class StrictMathTest { + + public static void main(String[] args) { + acosTest(); + powTest(); // 2 tests + IEEEremainderTest(); // 2 tests + tanTest(); // 2 tests + sinTest(); // 2 tests + printStatus("StrictMath"); + } + + private static int passed; + private static int failed; + + static void printStatus(String className) { + if (failed != 0) { + System.out.println(className + " test failures statistic (" + failed + + " fails and " + passed + " passes)."); + } else { + System.out.println("PASSED: [" + className + "] All " + passed + + " tests."); + } + } + + static void assertTrue(boolean cond, String testName) { + if (cond) { + passed++; + } else { + failed++; + System.out.println("FAILED " + testName); + } + } + + static void assertEquals(long a, long b, String testName) { + assertTrue(a == b, testName); + } + + static void assertEquals(double a, double b, double prec, String testName) { + assertTrue(Math.abs(a - b) <= prec, testName); + } + + static void assertTaskNoTimeout(Runnable task, long timeoutMillis, + String testName) { + Thread thread = new Thread(task, "Task " + testName); + thread.setDaemon(true); + thread.start(); + try { + thread.join(timeoutMillis); + } catch (InterruptedException e) { + // Should not happen. + assertTrue(false, testName); + } + assertTrue(!thread.isAlive(), testName); + } + + public static void acosTest() { + assertEquals(2.2142974, StrictMath.acos(-0.6), 1e-5, "acos"); + } + + public static void powTest() { + assertEquals(0, Double.doubleToRawLongBits(StrictMath.pow(-0.0, 0.5)), + "pow"); + assertEquals(~(-1L >>> 1), + Double.doubleToRawLongBits(StrictMath.pow(-0.0, 3)), + "pow #2"); + } + + public static void IEEEremainderTest() { + assertEquals(~(-1L >>> 1), + Double.doubleToRawLongBits( + StrictMath.IEEEremainder(-0.0, 1)), + "IEEEremainder"); + assertEquals(-1.7999999, StrictMath.IEEEremainder(2, 3.8), 1E-5, + "IEEEremainder #2"); + } + + public static void sinTest() { + assertTaskNoTimeout(new Runnable() { + + public void run() { + assertEquals(0.6402884, StrictMath.sin(1.9e209), 1e-5, "sin #2"); + } + }, 1000, "sin"); + } + + public static void tanTest() { + assertEquals(-0.0290081, StrictMath.tan(-0.029), 1e-5, "tan"); + assertEquals(34.2325327, StrictMath.tan(-1.6), 1e-5, "tan #2"); + } +} Regards, Ivan
