Re: java.lang.Character

[Eric Blake <ebb9 at email dot byu dot edu>]

Tom Tromey wrote:
> 
> All nice features, plus which merging has its own inherent benefits in
> terms of future changes.  As you can see this little piece of code has
> a lot of subtle parts; changing it is probably a bigger task than you
> thought you were getting into...  So thanks for doing this.

Yes, it is quite subtle.  I think I got everything, though, with this
latest version.

> 
> Our current Character actually has two implementations, depending on
> whether you configure for the compact or fast implementation.
> However, as you've discovered, it is possible to have a more compact
> and yet equally fast (or perhaps faster, depending on caching issues)
> implementation.  See also PR 1442.

I don't have write access to GNATS, but I think this implementation is
exactly what the reporter wanted.  The new include/java-chartables.h is
about 10k of static data, in 65k of source code, and it will be faster;
the old version was 3 megabytes of source! (It's highly redundant,
though - bzip2 got the 3.5 meg diff down to this 80k attachment).

> 
> I don't think it is important to keep the configury option.  Once your
> new implementation goes in, let's just delete the code from
> configure.in.

I think I did that right.  You'd better check my work.  And since I
don't have a working copy of the libgcj automake for cygwin, I'll let
you regenerate configure and automake.in.

> 
> Some other things to know:
> 
> * The chartables.pl script also generates a java-chardecomp.h file
>   which is used by java.text to implement decompositions.
>   As I recall this code is fatally buggy -- the native code can read
>   past the end of the generated arrays -- but no-one has noticed.

For now, I moved chartables.pl to scripts/unicode-decomp.pl, and
regenerated the decomp tables based on Unicode 3.0.  But I did not touch
the Collator class (including the native code), so if there was a bug,
it still exists.  Also, I noticed that Unicode 3.0 states that the
complete canonical decompositions are obtained by a recursive
application of the decompostions listed in UnicodeData.txt.  Neither the
old chartables.pl nor the code for java.text.Collator seem to do the
recursive decomposition; and I did not add it in, so that may be buggy
as well.  But as my patch is focusing on java.lang.Character, we can
leave the Collator issues for a future patch.

> 
>   It would be nice to merge this code with Classpath.  That's pretty
>   hairy though.  Unless the new Character has decomposition data in
>   it, we must unfortunately keep chartables.pl and modify it not to
>   generate chartables.h.

I haven't even looked at the Classpath version of Collator, to see how
easy (or hard) it would be to merge.

> 
> * The Mauve Character tests are probably buggy and could use a
>   critical look-over.

I'm not sure what bugs it may have, but it is definitely using an old
version of the UnicodeData.txt, so it will probably disagree with gcj in
several places.  Also, the return values of some of the methods of
Character have changed between JDK releases; for example,
isJavaLetter(ch) used to be specified as isLetter(ch) || ch == '_' || ch
== '$'; but in 1.4 is specified as a synonym for
isJavaIdentifierStart(ch).

2002-02-21  Eric Blake  <ebb9@email.byu.edu>

	* scripts/unicode-decomp.pl: Move from chartables.pl, and remove
	the code for generating include/java-chartables.h.
	* scripts/unicode-blocks.pl: Copy from Classpath.
	* scripts/unicode-muncher.pl: Copy from Classpath.
	* scritps/MakeCharTables.java: New file.
	* gnu/gcj/convert/Blocks-3.txt: New file.
	* gnu/gcj/convert/UnicodeData-3.0.0.txt: New file.
	* gnu/gcj/convert/UnicodeCharacterDatabase-3.0.0.html: New file.
	* gnu/java/lang/CharData.java: Copy from Classpath.
	* Makefile.am (ordinary_java_source_files): Add
	gnu/java/lang/CharData.java.
	* configure.in: Remove --enable-fast-character option.
	* java/lang/Character.java: Merge algorithms and Javadoc with
	Classpath.
	* java/lang/natCharacter.cc: Implement Unicode lookup table more
	efficiently.
	* include/java-chardecomp.h: Regenerate.
	* include/java-chartables.h: Regenerate.
-- 
This signature intentionally left boring.

Eric Blake             ebb9@email.byu.edu
  BYU student, free software programmer

Attachment: diff.patch.bz2
Description: Binary data