[gwt-contrib] Change in gwt[master]: adding compare for several number types
Daniel Kurka has submitted this change and it was merged.
Change subject: adding compare for several number types
..
adding compare for several number types
-Byte.compare
-Short.compare
-Integer.compare
-Long.compare
-Float.compare
(Double already exists)
fixes issue 7998
Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Review-Link: https://gwt-review.googlesource.com/#/c/3180/
---
M user/super/com/google/gwt/emul/java/lang/Byte.java
M user/super/com/google/gwt/emul/java/lang/Double.java
M user/super/com/google/gwt/emul/java/lang/Float.java
M user/super/com/google/gwt/emul/java/lang/Integer.java
M user/super/com/google/gwt/emul/java/lang/Long.java
M user/super/com/google/gwt/emul/java/lang/Short.java
M user/test/com/google/gwt/emultest/java/lang/ByteTest.java
M user/test/com/google/gwt/emultest/java/lang/FloatTest.java
M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java
M user/test/com/google/gwt/emultest/java/lang/LongTest.java
M user/test/com/google/gwt/emultest/java/lang/ShortTest.java
11 files changed, 85 insertions(+), 51 deletions(-)
Approvals:
Leeroy Jenkins: Verified
Goktug Gokdogan: Looks good to me, approved
diff --git a/user/super/com/google/gwt/emul/java/lang/Byte.java
b/user/super/com/google/gwt/emul/java/lang/Byte.java
index 664589e..ec3e4d9 100644
--- a/user/super/com/google/gwt/emul/java/lang/Byte.java
+++ b/user/super/com/google/gwt/emul/java/lang/Byte.java
@@ -33,6 +33,10 @@
private static Byte[] boxedValues = new Byte[256];
}
+ public static int compare(byte x, byte y) {
+return x - y;
+ }
+
public static Byte decode(String s) throws NumberFormatException {
return Byte.valueOf((byte) __decodeAndValidateInt(s, MIN_VALUE,
MAX_VALUE));
}
@@ -93,13 +97,7 @@
}
public int compareTo(Byte b) {
-if (value < b.value) {
- return -1;
-} else if (value > b.value) {
- return 1;
-} else {
- return 0;
-}
+return compare(value, b.value);
}
@Override
diff --git a/user/super/com/google/gwt/emul/java/lang/Double.java
b/user/super/com/google/gwt/emul/java/lang/Double.java
index e32f9d2..74bdcd9 100644
--- a/user/super/com/google/gwt/emul/java/lang/Double.java
+++ b/user/super/com/google/gwt/emul/java/lang/Double.java
@@ -86,22 +86,24 @@
};
public static int compare(double x, double y) {
+if (x < y) {
+ return -1;
+}
+if (x > y) {
+ return 1;
+}
+if (x == y) {
+ return 0;
+}
+
if (isNaN(x)) {
if (isNaN(y)) {
return 0;
} else {
return 1;
}
-} else if (isNaN(y)) {
- return -1;
-}
-
-if (x < y) {
- return -1;
-} else if (x > y) {
- return 1;
} else {
- return 0;
+ return -1;
}
}
diff --git a/user/super/com/google/gwt/emul/java/lang/Float.java
b/user/super/com/google/gwt/emul/java/lang/Float.java
index fe4bc2b..84e2ef1 100644
--- a/user/super/com/google/gwt/emul/java/lang/Float.java
+++ b/user/super/com/google/gwt/emul/java/lang/Float.java
@@ -34,13 +34,7 @@
private static final long POWER_32_INT = 4294967296L;
public static int compare(float x, float y) {
-if (x < y) {
- return -1;
-} else if (x > y) {
- return 1;
-} else {
- return 0;
-}
+return Double.compare(x, y);
}
public static int floatToIntBits(float value) {
@@ -187,13 +181,7 @@
}
public int compareTo(Float b) {
-if (value < b.value) {
- return -1;
-} else if (value > b.value) {
- return 1;
-} else {
- return 0;
-}
+return compare(value, b.value);
}
@Override
diff --git a/user/super/com/google/gwt/emul/java/lang/Integer.java
b/user/super/com/google/gwt/emul/java/lang/Integer.java
index 04e2b00..a0c2f6f 100644
--- a/user/super/com/google/gwt/emul/java/lang/Integer.java
+++ b/user/super/com/google/gwt/emul/java/lang/Integer.java
@@ -57,6 +57,10 @@
return x & 0x003f;
}
+ public static int compare(int x, int y) {
+return signum(x - y);
+ }
+
public static Integer decode(String s) throws NumberFormatException {
return Integer.valueOf((int) __decodeAndValidateInt(s, MIN_VALUE,
MAX_VALUE));
}
@@ -287,13 +291,7 @@
}
public int compareTo(Integer b) {
-if (value < b.value) {
- return -1;
-} else if (value > b.value) {
- return 1;
-} else {
- return 0;
-}
+return compare(value, b.value);
}
@Override
diff --git a/user/super/com/google/gwt/emul/java/lang/Long.java
b/user/super/com/google/gwt/emul/java/lang/Long.java
index c0b08d7..eaee9ec 100644
--- a/user/super/com/google/gwt/emul/java/lang/Long.java
+++ b/user/super/com/google/gwt/emul/java/lang/Long.java
@@ -58,6 +58,10 @@
return Integer.bitCount(high) + Integer.bitCount(low);
}
+ public static int compare(long x, long y) {
+return signum(x - y);
+ }
+
public static Long decode(String s)
[gwt-contrib] Change in gwt[master]: adding compare for several number types
Goktug Gokdogan has posted comments on this change. Change subject: adding compare for several number types .. Patch Set 14: Code-Review+2 -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 14 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types
Hello John A. Tamplin, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#14). Change subject: adding compare for several number types .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Double.java M user/super/com/google/gwt/emul/java/lang/Float.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 11 files changed, 85 insertions(+), 51 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 14 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types
Hello John A. Tamplin, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#13). Change subject: adding compare for several number types .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Double.java M user/super/com/google/gwt/emul/java/lang/Float.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 11 files changed, 86 insertions(+), 52 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 13 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types
Goktug Gokdogan has posted comments on this change.
Change subject: adding compare for several number types
..
Patch Set 12:
(1 comment)
The comparison functions are usually used in non linear algorithm like
sorting so average # of checks done for each comparison can significantly
impact performance.
File user/super/com/google/gwt/emul/java/lang/Double.java
Line 95:
x == y is also more common than NaN so it is better to check it earlier:
{
if (x < y) {
return -1;
}
if (x > y) {
return 1;
}
if (x == y) {
return 0;
}
// At this point, at least one of them is NaN
if (isNaN(x)) {
if (isNaN(y)) {
return 0;
}
return 1;
} else {
return -1;
}
}
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 12
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types
John A. Tamplin has posted comments on this change. Change subject: adding compare for several number types .. Patch Set 12: Code-Review+2 The intent wasn't to make it more readable, but to give a slight performance optimization on the assumption that most comparisons don't involve NaNs. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 12 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types
Daniel Kurka has posted comments on this change. Change subject: adding compare for several number types .. Patch Set 12: Updated Patch to deal with normal numbers first, but I don't think this ends up being more readable. But I am fine either way. Float.compare also uses Double.compare to avoid redundancy. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 12 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types
Hello John A. Tamplin, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#12). Change subject: adding compare for several number types .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Double.java M user/super/com/google/gwt/emul/java/lang/Float.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 11 files changed, 84 insertions(+), 50 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 12 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Goktug Gokdogan has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 11: Like John said, it was compilation error because you kept the 'else' statement as is. Just follow my code snippet or delegate to Double like John suggested and I can send a patch for Double later. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 11 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
John A. Tamplin has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 11: That was because you left "else return 0" before the NaN checks. You would have to move that branch to the end, below the NaN checks to re-order them. BTW, since GWT treats floats and doubles the same, should we just have Float.compareTo delegate to Double.compareTo? Less code to be different, and it should get completely inlined. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 11 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 11: Changing the order in Float/Double compare will lead to an unreachable code compiler warning. This must be the reason why Double was implemented the way it was. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 11 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Goktug Gokdogan has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 11: You said you changed the comparison order in Float but you reverted it back? -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 11 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
John A. Tamplin has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 11: Code-Review+2 -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 11 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello John A. Tamplin, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#11). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Float.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 10 files changed, 83 insertions(+), 35 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 11 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 9:
(6 comments)
File user/super/com/google/gwt/emul/java/lang/Byte.java
Line 37: return Integer.compare(x, y);
Done
File user/super/com/google/gwt/emul/java/lang/Float.java
Line 37: if (isNaN(x)) {
changed order
File user/super/com/google/gwt/emul/java/lang/Integer.java
Line 61: return signum(x - y);
see:
https://code.google.com/p/google-web-toolkit/issues/detail?id=8185&thanks=8185
File user/super/com/google/gwt/emul/java/lang/Short.java
Line 37: return Integer.compare(x, y);
Done
File user/test/com/google/gwt/emultest/java/lang/FloatTest.java
Line 81: }
see:
https://code.google.com/p/google-web-toolkit/issues/detail?id=8185&thanks=8185
File user/test/com/google/gwt/emultest/java/lang/IntegerTest.java
Line 120: assertEquals(0, Integer.compare(1, 1));
Let's deal with the corner cases in another CL:
https://code.google.com/p/google-web-toolkit/issues/detail?id=8185
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 9
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello John A. Tamplin, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#10). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Double.java M user/super/com/google/gwt/emul/java/lang/Float.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 11 files changed, 91 insertions(+), 43 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 10 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Thomas Broyer has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: (1 comment) File user/test/com/google/gwt/emultest/java/lang/FloatTest.java Line 81: } "-0" is valid JS, and 1/0 is Infinity while 1/-0 is -Infinity (per spec and tested in Chrome and Firefox). That said, we don't deal with that elsewhere. E.g. our Math.signum() emulations use >0 and <0 which won't distinguish +0 and -0. Math.signum(Double.NaN) is also broken BTW. This is fixable (with explicit isNaN and ==0 checks) but nobody complained so far. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: Yes -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Goktug Gokdogan has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: (1 comment) File user/super/com/google/gwt/emul/java/lang/Integer.java Line 61: return signum(x - y); ".. deserves whatever pain they get" =) I would not try hard for something like this but currently I don't see any good reason to make this behave different than the de facto JDK. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: Yes -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
John A. Tamplin has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 9: -Code-Review
(4 comments)
File user/super/com/google/gwt/emul/java/lang/Float.java
Line 37: if (isNaN(x)) {
You have to make sure those comparisons behave as desired when one or both
operands are NaNs.
File user/super/com/google/gwt/emul/java/lang/Integer.java
Line 61: return signum(x - y);
The spec of the Javadoc doesn't care, so anyone that writes code that
depends on implementation details, whether our own JRE emulation or
different JVMs, deserves whatever pain they get.
File user/test/com/google/gwt/emultest/java/lang/FloatTest.java
Line 81: }
Floating point varies significantly across all browsers. At least when I
was working on it, Safari JSCore steals bits from floats for tag values, so
there are some values that aren't representable there and you get different
roundoff than other engines.
I don't believe you can type "-0" in JS, but there are other ways to
generate them. I seem to recall code in Dan's float-to-int bytes tests
that generated them.
File user/test/com/google/gwt/emultest/java/lang/IntegerTest.java
Line 120: assertEquals(0, Integer.compare(1, 1));
We already say in the JRE compatibility doc that we don't handle integer
overflows. However, people aren't going to think of a comparison as an
integer overflow case, so if we can handle it cheaply we should.
After MAX_VALUE - MIN_VALUE overflows, what happens after the result is
coerced to an int? If we can't deal with the overflow after the fact, we
would need to check for overflow before doing the comparison.
Or, maybe that is why we didn't use subtraction in the first place.
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 9
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Goktug Gokdogan has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 9:
(4 comments)
File user/super/com/google/gwt/emul/java/lang/Float.java
Line 37: if (isNaN(x)) {
Why do you need to do something like that?
Following should work fine:
// All these checks should be false if x or y isNaN
if(x < y) return -1;
if(x > y) return 1;
if(x == y) return 0;
// below same isNaN check as before
if( isNaN(x) )
File user/super/com/google/gwt/emul/java/lang/Short.java
Line 37: return Integer.compare(x, y);
One should never rely on it for sure, but if somehow they rely on it, it
will work or fail consistently in both production and dev mode (i.e. less
surprises).
In general, it is best to not diverge prod vs dev if there is no good
reason to do so (like a big performance hit).
File user/test/com/google/gwt/emultest/java/lang/FloatTest.java
Line 81: }
I guess there is no -0 in js and in that case there is not much to do,
On the other hand, I'm not very familiar how we handle numerical difference
in js vs java. Thomas? John?
File user/test/com/google/gwt/emultest/java/lang/IntegerTest.java
Line 120: assertEquals(0, Integer.compare(1, 1));
I think we need to but we can create a bug and defer it to another time.
John & Thomas, any input here?
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 9
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 9:
(4 comments)
File user/super/com/google/gwt/emul/java/lang/Float.java
Line 37: if (isNaN(x)) {
I don't think its a good idea, since with this we would have to do
something like:
if(!isNaN(x) && !isNaN(y)) {
// do normal stuff
}else{
//test again
}
If we want to change this, we should also change this in Double.compare
(its implemented the same way)
File user/super/com/google/gwt/emul/java/lang/Short.java
Line 37: return Integer.compare(x, y);
I don"t really understand what the difference would be between prod vs. dev
if we do not use x-y. Correct me if I am wrong but one should never rely on
the exact value returned from compare.
File user/test/com/google/gwt/emultest/java/lang/FloatTest.java
Line 81: }
In JavaScript something like this passes:
assertEquals(0, Float.compare(0.0f, -0.0f));
While it does not pass in Java. Do we want to make sure we cover this
corner case (as well as others) and take the performance hit?
Not sure here. Could we just cover this in the docs of the JRE Emulation?
File user/test/com/google/gwt/emultest/java/lang/IntegerTest.java
Line 120: assertEquals(0, Integer.compare(1, 1));
We actually look terrible at the Borders:
This will just fail:
Integer.compare(Integer.MAX_VALUE, Integer.MIN_VALUE) > 0
since we do not got overflow for ints in javascript.
I am not sure how to handle such corner cases, do we want to handle them,
any input?
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 9
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Goktug Gokdogan has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 9:
(6 comments)
File user/super/com/google/gwt/emul/java/lang/Byte.java
Line 37: return Integer.compare(x, y);
return x - y otherwise this will be inconsistent dev vs prod
File user/super/com/google/gwt/emul/java/lang/Float.java
Line 37: if (isNaN(x)) {
Please start checks with the more common case (i.e. x & y is not NaN).
File user/super/com/google/gwt/emul/java/lang/Integer.java
Line 61: return signum(x - y);
If we don't return 1 and -1 then it will be inconsistent across dev vs prod
File user/super/com/google/gwt/emul/java/lang/Short.java
Line 37: return Integer.compare(x, y);
Short should just return x-y otherwise this will be inconsistent dev vs
prod.
File user/test/com/google/gwt/emultest/java/lang/FloatTest.java
Line 81: }
Not sure if there is such a difference in js like in java but can you add a
test for -0 vs 0?
File user/test/com/google/gwt/emultest/java/lang/IntegerTest.java
Line 120: assertEquals(0, Integer.compare(1, 1));
Can you add test for borders like John suggested?
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 9
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Thomas Broyer has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: BTW, re. "@Thomas you were right about the FloatTests emul failed in prod mode, jenkins did not catch that." Jenkins only compiles code and run apicheck and checkstyle, it does not run tests (more precisely, it calls "ant dist-dev apicheck checkstyle"). -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
John A. Tamplin has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: Code-Review+2 Didn't expect it to remove my previous score. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
John A. Tamplin has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: -Code-Review (1 comment) File user/super/com/google/gwt/emul/java/lang/Integer.java Line 61: return signum(x - y); That is a change from before, but from the Javadoc it should be fine. However, I worry about overflow conditions, such as compare(2^31, -2^31) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: Yes -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Thomas Broyer has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: (1 comment) File user/super/com/google/gwt/emul/java/lang/Integer.java Line 61: return signum(x - y); Do we actually need the call to signum() here? The Javadoc only talks about positive or negative values, not specifically 1 and -1, and Character#compareTo currently just returns 'x - y'. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: Yes -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Thomas Broyer has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: Code-Review+2 -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: I'll leave this open for another 1-2 days to give other the chance to comment -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
John A. Tamplin has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 9: Code-Review+2 -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 8: (2 comments) File user/super/com/google/gwt/emul/java/lang/Byte.java Line 100: return Byte.compare(value, b.value); Done File user/test/com/google/gwt/emultest/java/lang/FloatTest.java Line 66: Done -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 8 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: Yes -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello John A. Tamplin, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#9). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Float.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 10 files changed, 83 insertions(+), 35 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 9 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
John A. Tamplin has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 8: Code-Review+2 (2 comments) LGTM with a couple of nits. File user/super/com/google/gwt/emul/java/lang/Byte.java Line 100: return Byte.compare(value, b.value); "Byte." is unnecessary, right? (Eclipse with the GWT-recommended settings should be warning you about this). File user/test/com/google/gwt/emultest/java/lang/FloatTest.java Line 66: Trailing spaces. -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 8 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: Yes -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Andrey Korzhevskiy has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 8: @Daniel: Sure -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 8 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Andrey Korzhevskiy Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 8: @Andrey could you open another bug for this, so this patch does not grow even more? -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 8 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello John A. Tamplin, Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#8). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Float.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 10 files changed, 82 insertions(+), 28 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 8 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 7:
(5 comments)
File user/super/com/google/gwt/emul/java/lang/Byte.java
Line 100: return Byte.compare(value, b.value);
Done
File user/super/com/google/gwt/emul/java/lang/Float.java
Line 37: if (isNaN(x)) {
There was no bug for this, I just ran the tests in prod mode and they
failed with NaN.
Line 46:
Done
File user/super/com/google/gwt/emul/java/lang/Short.java
Line 37: return (x < y) ? -1 : ((x == y) ? 0 : 1);
Not sure if there are any performance implications here with coercion
File user/test/com/google/gwt/emultest/java/lang/FloatTest.java
Line 59:
Done
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 7
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
John A. Tamplin has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 7: Code-Review+1
(7 comments)
LGTM with minor changes
File user/super/com/google/gwt/emul/java/lang/Byte.java
Line 36: public static int compare(byte x, byte y) {
Same question as short - could we use Integer.signum here?
Line 100: return Byte.compare(value, b.value);
I noticed you didn't make this change in other places -- you probably
should, particularly in Float where compareTo doesn't handle NaNs the same
way as compare.
File user/super/com/google/gwt/emul/java/lang/Float.java
Line 37: if (isNaN(x)) {
Is there a bug for this, or did you just notice the behavior wan't correct
before?
Line 46:
Trailing spaces
File user/super/com/google/gwt/emul/java/lang/Short.java
Line 37: return (x < y) ? -1 : ((x == y) ? 0 : 1);
Couldn't you use Integer.signum here? Or do coercion rules make that less
efficient or inaccurate?
File user/test/com/google/gwt/emultest/java/lang/FloatTest.java
Line 55: public void testCompare() {
It doesn't look like we have Float.compareTo under test. Should we
duplicate this method and test that as well?
Line 59:
Trailing spaces
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 7
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Andrey Korzhevskiy has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 7: Sorry for intrusion, can we also get Java 7 compare methods for Boolean and Character classes? -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 7 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Andrey Korzhevskiy Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#7). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Float.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 10 files changed, 64 insertions(+), 14 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 7 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change. Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. Patch Set 6: Sorry about the comments getting messed up. Gerrit still confuses me somtimes. Am I supposed to update comments first and then supply another patch or the other way around? @Thomas you were right about the FloatTests emul failed in prod mode, jenkins did not catch that. I will update Float.compare/ FloatTest -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 6 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Daniel Kurka has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 5:
(3 comments)
File user/super/com/google/gwt/emul/java/lang/Byte.java
Line 37: return (x < y) ? -1 : ((x == y) ? 0 : 1);
Updated compareTo to use compare as well, but did not go for if/else
combination
File user/super/com/google/gwt/emul/java/lang/Integer.java
Line 61: return signum(x - y);
rewrote compareTo here as well
File user/test/com/google/gwt/emultest/java/lang/ByteTest.java
Line 38: assertTrue("Byte.compare failed for 1 == 1",
Byte.compare((byte) 1, (byte) 1) == 0);
Done
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 5
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#6). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 9 files changed, 48 insertions(+), 14 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 6 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Thomas Broyer has posted comments on this change.
Change subject: adding compare for several number types -Byte.compare
-Short.compare -Integer.compare -Long.compare -Float.compare (Double
already exists) fixes issue 7998
..
Patch Set 5: Code-Review+1
(5 comments)
File user/super/com/google/gwt/emul/java/lang/Byte.java
Line 37: return (x < y) ? -1 : ((x == y) ? 0 : 1);
How about using the same if/else if/else from compareTo, and rewriting
compareTo as compare(value, b.value) ?
File user/super/com/google/gwt/emul/java/lang/Integer.java
Line 61: return signum(x - y);
Should we rewrite compareTo in terms of signum or compare?
File user/super/com/google/gwt/emul/java/lang/Long.java
Line 62: return signum(x - y);
Anyone knows the performance difference between comparing longs and
substracting them?
File user/test/com/google/gwt/emultest/java/lang/ByteTest.java
Line 38: assertTrue("Byte.compare failed for 1 == 1",
Byte.compare((byte) 1, (byte) 1) == 0);
nit: assertEquals?
File user/test/com/google/gwt/emultest/java/lang/FloatTest.java
Line 56: assertTrue("Float.compare failed for 1 < 2", Float.compare(1f,
2f) < 0);
DoubleTest has many tests with infinity and NaN, should we add them here?
(note we didn't have tests about compareTo)
--
To view, visit https://gwt-review.googlesource.com/3180
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246
Gerrit-PatchSet: 5
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurka
Gerrit-Reviewer: Daniel Kurka
Gerrit-Reviewer: John A. Tamplin
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-Reviewer: Thomas Broyer
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#5). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 9 files changed, 46 insertions(+), 0 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 5 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#4). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 9 files changed, 46 insertions(+), 0 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 4 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: adding compare for several number types -Byte.compare -...
Hello Thomas Broyer, Leeroy Jenkins, I'd like you to reexamine a change. Please visit https://gwt-review.googlesource.com/3180 to look at the new patch set (#3). Change subject: adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 .. adding compare for several number types -Byte.compare -Short.compare -Integer.compare -Long.compare -Float.compare (Double already exists) fixes issue 7998 Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Review-Link: https://gwt-review.googlesource.com/#/c/3180/ --- M user/super/com/google/gwt/emul/java/lang/Byte.java M user/super/com/google/gwt/emul/java/lang/Integer.java M user/super/com/google/gwt/emul/java/lang/Long.java M user/super/com/google/gwt/emul/java/lang/Short.java M user/test/com/google/gwt/emultest/java/lang/ByteTest.java M user/test/com/google/gwt/emultest/java/lang/FloatTest.java M user/test/com/google/gwt/emultest/java/lang/IntegerTest.java M user/test/com/google/gwt/emultest/java/lang/LongTest.java M user/test/com/google/gwt/emultest/java/lang/ShortTest.java 9 files changed, 46 insertions(+), 0 deletions(-) -- To view, visit https://gwt-review.googlesource.com/3180 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib33c93ff0fb3f7e4b93994a29d6e2a65898be246 Gerrit-PatchSet: 3 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Daniel Kurka Gerrit-Reviewer: Daniel Kurka Gerrit-Reviewer: John A. Tamplin Gerrit-Reviewer: Leeroy Jenkins Gerrit-Reviewer: Thomas Broyer -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
