This is the mail archive of the
kawa@sources.redhat.com
mailing list for the Kawa project.
Re: bug in PrettyWriter
- To: Felix S Klock II <pnkfelix at MIT dot EDU>
- Subject: Re: bug in PrettyWriter
- From: Per Bothner <per at bothner dot com>
- Date: 18 Aug 2001 12:14:07 -0700
- Cc: kawa at sources dot redhat dot com
- References: <200108180000.UAA14382@crash-test-felix.lcs.mit.edu>
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/