Re: bug in PrettyWriter

[Per Bothner <per at bothner dot com>]

Felix S Klock II <pnkfelix@MIT.EDU> writes:

> There is a bug in the pretty printing routine; it does not properly
> reset the prefix field when it discovers that the prefix needs to be 
> lengthened, due to the LHS of the assignment being bound to a local
> variable and not the field that was desired. 

Thanks for tracking this down.  Please try the appended patch.
> 
> However, I also fail to understand why the local variable prefix has
> been introduced in the first place.  It doesn't seem to serve any
> purpose; perhaps it is legacy code.

It is purely a performance hack.  (However, since I wasn't using it in
the inner loop, there wasn't much point!)  Using a local variable is
much more efficient than accessing a field - you're avoiding a memory
indirection and a possible null pointer test.  (Acessing a field of
'this' is no more efficient than accessing a field of any other local
variable.)

Some would say excessive and premature micro-optimization.

Index: PrettyWriter.java
===================================================================
RCS file: /cvs/kawa/kawa/gnu/text/PrettyWriter.java,v
retrieving revision 1.2
diff -u -r1.2 PrettyWriter.java
--- PrettyWriter.java	2001/07/17 19:28:54	1.2
+++ PrettyWriter.java	2001/08/18 19:06:00
@@ -383,14 +383,14 @@
       column = minimum;
     if (column > prefixLen)
       {
-	char[] newPrefix = new char[enoughSpace(prefixLen, column - prefixLen)];
-	System.arraycopy(prefix, 0, newPrefix, 0, current);
-	prefix = newPrefix;
+	prefix = new char[enoughSpace(prefixLen, column - prefixLen)];
+	System.arraycopy(this.prefix, 0, prefix, 0, current);
+	this.prefix = prefix;
       }
     if (column > current)
       {
 	for (int i = current;  i < column;  i++)
-	  this.prefix[i] = ' ';
+	  prefix[i] = ' ';
       }
     blocks[blockDepth + BLOCK_PREFIX_LENGTH] = column;
   }

-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/