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);
}
/**
