What about the regression risk? With this fix, the equality would differ
from the prior releases. Would that be ignorable?
Naoto
On 6/5/13 12:23 AM, Yuka Kamiya wrote:
Hi Alan,
Thank you for taking care of this.
I investigated how the method had been used. Your fix looks okay. It is
not only safe but improves the generator's behavior.
I think that we were just lucky that the method had merely wasted CPU
and didn't cause a worse problem....
Thanks,
--
Yuka
(2013/06/04 21:49), Alan Bateman wrote:
The new build emits a lot of warnings, the warnings when building the
build tools are the most scary. One warning that is starting to stand
out (as other issues are addressed) is the GenerateBreakIteratorData
tool. In the case the issue is that it overrides equals but not hashCode.
This is easily fixed by adding a hashCode method but if I read the
code correctly, the equals method doesn't match its javadoc. The
javadoc claims that it returns true if "that" has the exact same
characters but that isn't so. I would like to fix this tool with the
attached patch but it requires careful review because the broken
equals method is used. I've run the jdk tests with this fix and don't
see any issues but I don't know how well that BreakIterator is exercised.
-Alan
diff -r 780fbbd50ce4
make/tools/src/build/tools/generatebreakiteratordata/CharSet.java
---
a/make/tools/src/build/tools/generatebreakiteratordata/CharSet.java
Tue Jun 04 11:52:29 2013 +0100
+++
b/make/tools/src/build/tools/generatebreakiteratordata/CharSet.java
Tue Jun 04 13:08:24 2013 +0100
@@ -39,6 +39,7 @@
package build.tools.generatebreakiteratordata;
+import java.util.Arrays;
import java.util.Hashtable;
/**
@@ -701,7 +702,14 @@
* the exact same characters as this one
*/
public boolean equals(Object that) {
- return (that instanceof CharSet) &&
chars.equals(((CharSet)that).chars);
+ return (that instanceof CharSet) && Arrays.equals(chars,
((CharSet)that).chars);
+ }
+
+ /**
+ * Returns the hash code for this set of characters
+ */
+ public int hashCode() {
+ return Arrays.hashCode(chars);
}
/**
