Re: Removing remaining static initializers from basic classes

[Mark Wielaard]

Hi Grzegorz,

On Sat, 2004-03-27 at 05:26, Grzegorz B. Prokopski wrote:
> After recent discusions some of the static initializers of basic classes
> have been removed or worked around by using inner classes. The attached
> patch removes two other such cases and works around another one.

Thanks for this. The kaffe hackers were also complaining about some of
these static initializers (especially the one in Throwable.java).
Please include a ChangeLog entry when submitting a patch.
In this case I would have written something like:

2004-03-24  Grzegorz B. Prokopski  <address@hidden>

        * java/lang/Object.java (static): Remove static initializer.
        * java/lang/Throwable.java (nl): Remove static field.
        (StaticData): New private static inner class.
        (stackTraceStringBuffer): Use StaticData.nl.

Some comments below, but I think this should go in as is.
There are two minor indentation nitpicks (see below).
I see we still haven't completely documented them since the Hacking
Guide says: "There are a number of exceptions to the GNU Coding
Standards that we make for GNU Classpath and these will be documented
soon as well as hopefully providing developers with a code formatting
tool that closely matches those rules."
Sorry about that. But in general follow the GNU Coding Standards.

Tom, any news on the Jalopy savannah import?

________________________________________________________________________
> Index: java/lang/Object.java
> ===================================================================
> RCS file: /cvsroot/classpath/classpath/java/lang/Object.java,v
> retrieving revision 1.14
> diff -u -r1.14 Object.java
> --- java/lang/Object.java     26 Mar 2002 06:19:38 -0000      1.14
> +++ java/lang/Object.java     27 Mar 2004 04:16:09 -0000
> @@ -63,18 +63,8 @@
>  {
>    // WARNING: Object is a CORE class in the bootstrap cycle. See the comments
>    // in vm/reference/java/lang/Runtime for implications of this fact.
> -
> -  /**
> -   * Load in all native methods in the java.lang package. Note that this
> -   * call is actually a no-op, since it triggers the class initialization
> -   * of System, which loads the same library; but it is necessary to start
> -   * the System class initialization for the bootstrap sequence to work.
> -   */
> -  static
> -  {
> -    if (Configuration.INIT_LOAD_LIBRARY)
> -      System.loadLibrary("javalang");
> -  }
> +  // Many JVMs do not allow for static initializers in this class,
> +  // hence we do not use them in the default implementation.

Yes. If a VM wants to load a library for Object (which I doubt) the it
should be done from VMObject (or they can just override Object itself of
course). BTW we should probably also move getClass() to VMObject. (Or
should we add a class field reference to Object itself?)
java.lang.Object is a special class in many cases that I suppose VMs
will override completely in most cases. But we should have a usable one
in Classpath that VMs could (in theory at least) use out of the box
(with VMObject).

> Index: java/lang/Throwable.java
> ===================================================================
> RCS file: /cvsroot/classpath/classpath/java/lang/Throwable.java,v
> retrieving revision 1.18
> diff -u -r1.18 Throwable.java
> --- java/lang/Throwable.java  4 Oct 2002 15:26:25 -0000       1.18
> +++ java/lang/Throwable.java  27 Mar 2004 04:16:09 -0000
> @@ -400,7 +400,18 @@
>      pw.print(stackTraceString());
>    }
>  
> -  private static final String nl = System.getProperty("line.separator");
> +  /*
> +   * We use inner class to avoid a static initializer in this basic class

Missing dot at end of sentence.

> +   */
> +  private static class StaticData {

Bracket should go at next line:

     private static class StaticData
     {
> +
> +    private final static String nl;
> +
> +    static {

Here also:

       static
       {
> +      nl = System.getProperty("line.separator");
> +    }
> +  }
> +
>    // Create whole stack trace in a stringbuffer so we don't have to print
>    // it line by line. This prevents printing multiple stack traces from
>    // different threads to get mixed up when written to the same PrintWriter.
> @@ -453,6 +464,7 @@
>    private static void stackTraceStringBuffer(StringBuffer sb, String name,
>                                       StackTraceElement[] stack, int equal)
>    {
> +    String nl = StaticData.nl;
>      // (finish) first line
>      sb.append(name);
>      sb.append(nl);

Nice trick :)

> Index: java/lang/reflect/Array.java
> ===================================================================
> RCS file: /cvsroot/classpath/classpath/java/lang/reflect/Array.java,v
> retrieving revision 1.10
> diff -u -r1.10 Array.java
> --- java/lang/reflect/Array.java      7 Jan 2004 09:18:46 -0000       1.10
> +++ java/lang/reflect/Array.java      27 Mar 2004 04:16:10 -0000
> @@ -78,13 +78,6 @@
>   */
>  public final class Array
>  {
> -  static
> -  {
> -    if (Configuration.INIT_LOAD_LIBRARY)
> -      {
> -        System.loadLibrary("javalangreflect");
> -      }
> -  }
>  
>    /**
>     * This class is uninstantiable.
> 
> ______________________________________________________________________
Agreed this doesn't belong in Array.java itself.
Ideally we split this class into a regular Array and a VMArray class in
the future that contains the "native" plug points.

Thanks,

Mark

signature.asc
Description: This is a digitally signed message part