Use StringBuilder instead of PrintWriter for type display

Use StringBuilder instead of PrintWriter for type display

[Stan Cox]

This changes type display so it builds in a StringBuilder
instead of displaying a piece at a time.  Most of the changes are
changing writer.print to stringBuilder.append.  A printf style %s token
is used to support types like
"int (*ptr_arr) [4]" where (in this instance) the name and pointer
info have to be inserted a printf %s string is used.  For instance
ArrayType would construct "int %s [4]" and then PointerType would
massage that to "int (*%s) [4] and if it is member of a struct
CompositeType would massage it to "int (*ptr_arr) [4]" (otherwise "int
(*) [4]).  This allows removal of some instanceof checks and some
specialized code.

EvalCommands - Don't pass writer to toPrint

--- a/frysk-core/frysk/hpd/EvalCommands.java
+++ b/frysk-core/frysk/hpd/EvalCommands.java
@@ -71,8 +71,7 @@ abstract class EvalCommands extends
ParameterizedCommand {
 	static final Printer TYPE = new Printer() {
 		void print(Expression e, PrintWriter writer, Format format,
 			   ByteBuffer memory) {
-		    e.getType().toPrint(writer, 0);
-		    writer.println();
+		    writer.println(e.getType().toPrint());
 		}
 	    };
 	static final Printer TREE = new Printer() {

ArrayType - Insert %s where name/ptr would go

--- a/frysk-core/frysk/value/ArrayType.java
+++ b/frysk-core/frysk/value/ArrayType.java
@@ -275,20 +275,17 @@ public class ArrayType
 	return noNullByte;
     }
 
-    public void toPrint(PrintWriter writer, String s, int indent) {
-	type.toPrint(writer, indent);
-	writer.print(" " + s);
+    public void toPrint(StringBuilder stringBuilder, int indent) {
+	type.toPrint(stringBuilder, indent);
+	stringBuilder.append(" ");
+	stringBuilder.append("%s");
 	for(int i = 0; i < this.dimension.length; i++) {
-	    writer.print("[");
-	    writer.print(dimension[i]);
-	    writer.print("]");
+	    stringBuilder.append("[");
+	    stringBuilder.append(dimension[i]);
+	    stringBuilder.append("]");
 	}
     }
     
-    public void toPrint(PrintWriter writer, int indent) {
-	this.toPrint(writer, "", indent);
-    }
-    
     /* getALUs are double dispatch functions to determine 
      * the ArithmeticUnit for an operation between two types.
      */    

CompositeType - Insert name for %s, remove some specialized code

--- a/frysk-core/frysk/value/CompositeType.java
+++ b/frysk-core/frysk/value/CompositeType.java
@@ -275,7 +275,7 @@ public abstract class CompositeType
 	writer.print("}");
     }
 
-    public void toPrint(PrintWriter writer, int indentation) {
+    public void toPrint(StringBuilder stringBuilder, int indentation) {
 	if (indentation == 0)
 	    indentation = 2;
 	String indentPrefix = "";
@@ -283,10 +283,10 @@ public abstract class CompositeType
 	    indentPrefix = indentPrefix + " ";
 
 	// {class,union,struct} NAME
-	writer.print(getPrefix());
+	stringBuilder.append(getPrefix());
 	if (getName() != null && getName().length() > 0) {
-	    writer.print(" ");
-	    writer.print(getName());
+	    stringBuilder.append(" ");
+	    stringBuilder.append(getName());
 	}
 	// : public PARENT ...
 	boolean first = true;
@@ -298,57 +298,43 @@ public abstract class CompositeType
 	    if (!member.inheritance)
 		break;
 	    if (first) {
-		writer.print(" : ");
+		stringBuilder.append(" : ");
 		first = false;
 	    } else {
-		writer.print(", ");
+		stringBuilder.append(", ");
 	    }
 	    if (member.access != null) {
-		writer.print(member.access.toPrint());
-		writer.print(" ");
+		stringBuilder.append(member.access.toPrint());
+		stringBuilder.append(" ");
 	    }
-	    writer.print(member.type.getName());
+	    stringBuilder.append(member.type.getName());
 	    member = null;
 	}
 	// { content ... }
 	Access previousAccess = null;
-	writer.print(" {\n");
+	stringBuilder.append(" {\n");
 
 	while (member != null) {
-	    boolean printName = true;
 	    if (member.access != previousAccess) {
 		previousAccess = member.access;
 		if (member.access != null) {
-		    writer.print(" ");
-		    writer.print(member.access.toPrint());
-		    writer.print(":\n");
+		    stringBuilder.append(" ");
+		    stringBuilder.append(member.access.toPrint());
+		    stringBuilder.append(":\n");
 		}
 	    }
-	    writer.print(indentPrefix);
-	    if (member.type instanceof ArrayType) {
-		// For handling int x[2]
-		printName = false;
-		((ArrayType) member.type).toPrint(writer, member.name, indentation +
2);
-	    }
-	    else if (member.type instanceof PointerType) {
-		// For handling int (*x)[2]
-		printName = false;
-		((PointerType) member.type).toPrint(writer, " " + member.name,
-						    indentation + 2);
-	    }
-	    else
-		member.type.toPrint (writer, indentation + 2);
-	    if (member.type instanceof frysk.value.FunctionType)
-		printName = false;
-	    if (printName) {
-		writer.print(" ");
-		writer.print(member.name);
+	    stringBuilder.append(indentPrefix);
+	    member.type.toPrint (stringBuilder, indentation + 2);
+	    if (! toPrintFormat(stringBuilder, member.name)) {
+		if (stringBuilder.charAt(stringBuilder.length() - 1) != '*')
+		    stringBuilder.append(" ");
+		stringBuilder.append(member.name);
 	    }
 	    if (member.bitSize > 0) {
-		writer.print(":");
-		writer.print(member.bitSize);
+		stringBuilder.append(":");
+		stringBuilder.append(member.bitSize);
 	    }
-	    writer.print(";\n");
+	    stringBuilder.append(";\n");
 	    // Advance
 	    if (i.hasNext())
 		member = (DynamicMember)i.next();
@@ -356,8 +342,8 @@ public abstract class CompositeType
 		member = null;
 	}
 	for (int indent = 1; indent <= indentation - 2; indent++)
-	    writer.print(" ");
-	writer.print("}");
+	    stringBuilder.append(" ");
+	stringBuilder.append("}");
     }
     
     public Value member(Value var1, String member)

PointerType Simplify handling of "type (*x) [n]"


diff --git a/frysk-core/frysk/value/PointerType.java
b/frysk-core/frysk/value/PointerType.java
index b6e2f27..1c702fd 100644
--- a/frysk-core/frysk/value/PointerType.java
+++ b/frysk-core/frysk/value/PointerType.java
@@ -79,9 +79,14 @@ public class PointerType
     void toPrint(PrintWriter writer, Location location, ByteBuffer
memory,
 		 Format format, int indent) {
 	// Print type of pointer
-	writer.print("(");
-	this.toPrint(writer, 0);
-	writer.print(") ");	
+	StringBuilder stringBuilder = new StringBuilder();
+	stringBuilder.append("(");
+	this.toPrint(stringBuilder, indent);
+	int percentIdx = stringBuilder.indexOf("%");
+	if (percentIdx >= 0)
+	    stringBuilder.replace(percentIdx, percentIdx + 2, "");
+	stringBuilder.append(") ");
+	writer.print(stringBuilder);
 	try {
 	   format.print(writer, location, this);
 	} catch (RuntimeException e) {
@@ -110,21 +115,20 @@ public class PointerType
 	}
     }
 
-  public void toPrint(PrintWriter writer, String s, int indent) {
+  public void toPrint(StringBuilder stringBuilder, int indent) {
 	// For handling int (*x)[2]
 	if (type instanceof ArrayType) {
-	    ((ArrayType)type).toPrint(writer, "(*" + s + ")", indent);
+	    ((ArrayType)type).toPrint(stringBuilder, indent);
+	    toPrintFormat(stringBuilder, "(*%s)");
 	}
 	else {
-	    type.toPrint(writer, indent);
-	    writer.print(" *" + s);
+	    type.toPrint(stringBuilder, indent);
+	    if (stringBuilder.charAt(stringBuilder.length() - 1) != '*')
+		stringBuilder.append(" ");
+	    stringBuilder.append("*");
 	}
-    }
+  }
 
-    public void toPrint(PrintWriter writer, int indent) {
-      this.toPrint(writer, "", indent);
-    }
-    
     protected Type clone(IntegerType accessor) {
 	return new PointerType(getName(), order(), getSize(), type, accessor);
     }

Type - Add toPrintFormat, sort of a poor man's printf

--- a/frysk-core/frysk/value/Type.java
+++ b/frysk-core/frysk/value/Type.java
@@ -112,16 +112,28 @@ public abstract class Type {
 
     /**
      * Print this Type after indenting INDENT spaces.
+     * @param stringBuilder TODO
      */
-    public abstract void toPrint(PrintWriter writer, int indent);
+    public abstract void toPrint(StringBuilder stringBuilder, int
indent);
+
     /**
      * Print this Type to a StringBuffer and return the String.
      */
     public final String toPrint() {
-	StringWriter stringWriter = new StringWriter();
-	PrintWriter writer = new PrintWriter(stringWriter);
-	toPrint(writer, 0);
-	return stringWriter.toString();
+	StringBuilder stringBuilder = new StringBuilder();
+	toPrint(stringBuilder, 0);
+	toPrintFormat(stringBuilder, "");
+	return stringBuilder.toString();
+    }
+
+    public boolean toPrintFormat(StringBuilder stringBuilder, String
string) {
+	    int percentIdx = stringBuilder.indexOf("%");
+	    if (percentIdx >= 0) {
+		stringBuilder.replace(percentIdx, percentIdx + 2, string);
+		return true;
+	    }
+	    else
+		return false;
     }
 
     /* getALUs are double dispatch functions to determine the 

These just have writer -> stringBuilder changes

EnumType
FunctionType
TypeDecorator
TypeDef
UnknownType
Variable
VoidType
ArithmeticType

Re: Use StringBuilder instead of PrintWriter for type display

[Andrew Cagney]

Stan Cox wrote:
> This changes type display so it builds in a StringBuilder
> instead of displaying a piece at a time.  Most of the changes are
> changing writer.print to stringBuilder.append.  A printf style %s token
> is used to support types like
> "int (*ptr_arr) [4]" where (in this instance) the name and pointer
> info have to be inserted a printf %s string is used.  For instance
> ArrayType would construct "int %s [4]" and then PointerType would
> massage that to "int (*%s) [4] and if it is member of a struct
> CompositeType would massage it to "int (*ptr_arr) [4]" (otherwise "int
> (*) [4]).  This allows removal of some instanceof checks and some
> specialized code.
>   
Yes, the result is much better.

Would a specialized class help: instead of a SingleString buffer, it 
would contain the prefix and postfix strings, and methods to edit them. 
For instance, given:
  struct s {
     (*foo)[10];
  }
ArrayType would .append("[10]");
called by PointerType would .insert("(*", ")");
called by CompositeType would .insert("foo");


Andrew

Re: Use StringBuilder instead of PrintWriter for type display

[Andrew Cagney]

Andrew Cagney wrote:
> Stan Cox wrote:
>> This changes type display so it builds in a StringBuilder
>> instead of displaying a piece at a time.  Most of the changes are
>> changing writer.print to stringBuilder.append.  A printf style %s token
>> is used to support types like
>> "int (*ptr_arr) [4]" where (in this instance) the name and pointer
>> info have to be inserted a printf %s string is used.  For instance
>> ArrayType would construct "int %s [4]" and then PointerType would
>> massage that to "int (*%s) [4] and if it is member of a struct
>> CompositeType would massage it to "int (*ptr_arr) [4]" (otherwise "int
>> (*) [4]).  This allows removal of some instanceof checks and some
>> specialized code.
>>   
> Yes, the result is much better.
>
Hmm, if the below works, then wouldn't passing in the middle part vis:
    toPrint(String middle)
be sufficient:
  CompositeType: member.type.toPrint("foo");
  PointerType: pointerTarget.type.toPrint("*" +middle); // *foo
  ArrayType returns  "(" + middle + ")" + "[...]" // (*foo)[...]

Andrew

 
> Would a specialized class help: instead of a SingleString buffer, it 
> would contain the prefix and postfix strings, and methods to edit 
> them. For instance, given:
>  struct s {
>     (*foo)[10];
>  }
> ArrayType would .append("[10]");
> called by PointerType would .insert("(*", ")");
> called by CompositeType would .insert("foo");
>
>
> Andrew
>

Re: Use StringBuilder instead of PrintWriter for type display

[Stan Cox]

> wouldn't passing in the middle part

Here is a variation that works top down, middle out instead of bottom
up left to right.  With the middle out approach, in
some situations it is necessary to go from the middle to the beginning
and end of the token, instead of the beginning and end of the entire
declaration.  For example: int (*ptr_to_func) (const int), where the
const applies to a parameter and not to the return value.  So in those
cases a temporary string buffer is used.   (This does not currently use
a specialized class.)  The diffs are comparing to the bottom up
approach.  (Both patch sets pass TestTypeEntry)


##### Similar to bottom up approach

--- /home/scox/accu/save/ArrayType.java 2007-12-07 18:15:43.000000000
-0500
+++ frysk/value/ArrayType.java  2007-12-10 11:18:30.000000000 -0500
@@ -276,9 +276,11 @@ public class ArrayType
     }
 
     public void toPrint(StringBuilder stringBuilder, int indent) {
-       type.toPrint(stringBuilder, indent);
-       stringBuilder.append(" ");
-       stringBuilder.append("%s");
+       StringBuilder tmpStringBuilder = new StringBuilder();
+       type.toPrint(tmpStringBuilder, indent);
+       if (indent == 0)
+           tmpStringBuilder.append(" ");
+       stringBuilder.insert(0, tmpStringBuilder);
        for(int i = 0; i < this.dimension.length; i++) {
            stringBuilder.append("[");
            stringBuilder.append(dimension[i]);

##### Similar to bottom up approach but uses temporary
memberStringBuilder for struct members

--- /home/scox/accu/save/CompositeType.java     2007-12-09
20:37:17.000000000 -0500
+++ frysk/value/CompositeType.java      2007-12-10 16:06:37.000000000
-0500
@@ -275,7 +275,7 @@ public abstract class CompositeType
        writer.print("}");
     }
 
-    public void toPrint(StringBuilder stringBuilder, int indentation) {
+    public void toPrint(StringBuilder stringBuilderParm, int
indentation) {
        if (indentation == 0)
            indentation = 2;
        String indentPrefix = "";
@@ -283,6 +283,7 @@ public abstract class CompositeType
            indentPrefix = indentPrefix + " ";
 
        // {class,union,struct} NAME
+       StringBuilder stringBuilder = new StringBuilder();
        stringBuilder.append(getPrefix());
        if (getName() != null && getName().length() > 0) {
            stringBuilder.append(" ");
@@ -314,6 +315,7 @@ public abstract class CompositeType
        Access previousAccess = null;
        stringBuilder.append(" {\n");
 
+       StringBuilder memberStringBuilder = new StringBuilder();
        while (member != null) {
            if (member.access != previousAccess) {
                previousAccess = member.access;
@@ -323,13 +325,11 @@ public abstract class CompositeType
                    stringBuilder.append(":\n");
                }
            }
-           stringBuilder.append(indentPrefix);
-           member.type.toPrint (stringBuilder, indentation + 2);
-           if (! toPrintFormat(stringBuilder, "%s", member.name)) {
-               if (stringBuilder.charAt(stringBuilder.length() - 1) !=
'*')
-                   stringBuilder.append(" ");
-               stringBuilder.append(member.name);
-           }
+           memberStringBuilder.delete(0, memberStringBuilder.length());
+           memberStringBuilder.append(" " + member.name);
+           member.type.toPrint(memberStringBuilder, indentation + 2);
+           memberStringBuilder.insert(0, indentPrefix);
+           stringBuilder.append(memberStringBuilder);
            if (member.bitSize > 0) {
                stringBuilder.append(":");
                stringBuilder.append(member.bitSize);
@@ -344,6 +344,7 @@ public abstract class CompositeType
        for (int indent = 1; indent <= indentation - 2; indent++)
            stringBuilder.append(" ");
        stringBuilder.append("}");
+       stringBuilderParm.insert(0, stringBuilder);
     }
     
     public Value member(Value var1, String member)


##### Similar to bottom up approach but uses tmpStringBuilder for
parameters

+       StringBuilder tmpStringBuilder = new StringBuilder();
+       if (stringBuilder.charAt(0) == ' ')
+           stringBuilder.deleteCharAt(0);
        if (returnType == null) 
-           stringBuilder.append("void");
+           tmpStringBuilder.insert(0, "void");
        else
-           returnType.toPrint(stringBuilder, 0);
-       stringBuilder.append(" ");
-       stringBuilder.append("%s");
+           returnType.toPrint(tmpStringBuilder, 0);
+       tmpStringBuilder.append(" ");
+       stringBuilder.insert(0, tmpStringBuilder);
        stringBuilder.append(" (");
        for (int i = 0; i < this.parmTypes.size(); i++) {
-           ((Type)this.parmTypes.get(i)).toPrint(stringBuilder, 0);
-           stringBuilder.append((String)this.parmNames.get(i));
+           tmpStringBuilder.delete(0, tmpStringBuilder.length());
+           ((Type)this.parmTypes.get(i)).toPrint(tmpStringBuilder, 0);
+           tmpStringBuilder.append((String)this.parmNames.get(i));
+           stringBuilder.append(tmpStringBuilder);
            if (i < this.parmTypes.size() - 1)
                // Not last arg
                stringBuilder.append(",");
        }
        stringBuilder.append(")");
     }


##### Similar to bottom up but needs to specialize the (*x) cases a
      bit more

--- /home/scox/accu/save/PointerType.java       2007-12-09
20:36:51.000000000 -0500
+++ frysk/value/PointerType.java        2007-12-10 15:03:34.000000000
-0500
@@ -118,21 +118,25 @@ public class PointerType extends Integer
 
     public void toPrint(StringBuilder stringBuilder, int indent) {
        // For handling int (*x)[2]
-       if (type instanceof ArrayType) {
-           ((ArrayType)type).toPrint(stringBuilder, indent);
-           toPrintFormat(stringBuilder, "%s", "(*%s)");
-       }
-       else if (type instanceof FunctionType) {
-           ((FunctionType)type).toPrint(stringBuilder, indent);
-           toPrintFormat(stringBuilder, "%s", "(*%s)");
+       if (type instanceof ArrayType || type instanceof FunctionType) {
+           if (indent > 0 && stringBuilder.length() > 0
+                   && stringBuilder.charAt(0) == ' ') {
+               stringBuilder.deleteCharAt(0);
+               stringBuilder.insert(0, " (*");
+           }
+           else
+               stringBuilder.insert(0, "(*");
+           stringBuilder.append(")");
+           type.toPrint(stringBuilder, indent);
        }
        else {
+           if (indent > 0 && stringBuilder.length() > 0
+                   && stringBuilder.charAt(0) == ' ')
+               stringBuilder.deleteCharAt(0);
+           stringBuilder.insert(0, "*");
+           if (! (type instanceof PointerType))
+               stringBuilder.insert(0, " ");
            type.toPrint(stringBuilder, indent);
-           if (! toPrintFormat(stringBuilder, "*%s", "**%s")) {
-               if (stringBuilder.charAt(stringBuilder.length() - 1) !=
'*')
-                   stringBuilder.append(" ");
-               stringBuilder.append("*");
-           }
        }


##### The other files are similar to bottom up approach but use insert
      instead of append, e.g.

--- /home/scox/accu/save/ArithmeticType.java    2007-12-06
14:20:13.000000000 -0500
+++ frysk/value/ArithmeticType.java     2007-12-10 09:42:16.000000000
-0500
@@ -70,7 +70,7 @@ public abstract class ArithmeticType
     }
 
     public void toPrint(StringBuilder stringBuilder, int indent) {
-       stringBuilder.append(getName());
+       stringBuilder.insert(0, getName());
     }

Re: Use StringBuilder instead of PrintWriter for type display

[Stan Cox]

I forgot to mention the primary difference is that the bottom up
approach in some cases (array dcls, ptr to functions) requires a token
to be inserted marker (%s) that later needs to be substituted.  The top
down approach in some cases (parameter lists, struct members) requires a
temporary buffer.