Avoid creating new TemplateElement[] for TemplateElement.accept calls.
Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/83eca1b2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/83eca1b2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/83eca1b2 Branch: refs/heads/master Commit: 83eca1b2a7c3b6141b9f71adacea435a54d1b2e1 Parents: 325bbd0 Author: ddekany <ddek...@apache.org> Authored: Sun Dec 6 17:25:14 2015 +0100 Committer: ddekany <ddek...@apache.org> Committed: Sun Dec 6 19:05:36 2015 +0100 ---------------------------------------------------------------------- .../freemarker/core/AssignmentInstruction.java | 6 -- src/main/java/freemarker/core/AutoEscBlock.java | 2 +- src/main/java/freemarker/core/Case.java | 9 +- .../java/freemarker/core/ConditionalBlock.java | 5 +- src/main/java/freemarker/core/ElseOfList.java | 2 +- src/main/java/freemarker/core/EscapeBlock.java | 2 +- src/main/java/freemarker/core/IfBlock.java | 3 +- .../java/freemarker/core/NoAutoEscBlock.java | 2 +- .../java/freemarker/core/NoEscapeBlock.java | 2 +- .../java/freemarker/core/OutputFormatBlock.java | 2 +- .../java/freemarker/core/RecoveryBlock.java | 2 +- src/main/java/freemarker/core/Sep.java | 2 +- .../java/freemarker/core/TemplateElement.java | 93 ++++++++++++-------- ...nterruptionSupportTemplatePostProcessor.java | 2 +- src/test/java/freemarker/core/ASTPrinter.java | 50 +++++++++-- 15 files changed, 117 insertions(+), 67 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/AssignmentInstruction.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/AssignmentInstruction.java b/src/main/java/freemarker/core/AssignmentInstruction.java index 1890b11..c3526fb 100644 --- a/src/main/java/freemarker/core/AssignmentInstruction.java +++ b/src/main/java/freemarker/core/AssignmentInstruction.java @@ -110,12 +110,6 @@ final class AssignmentInstruction extends TemplateElement { } @Override - public TemplateElement postParseCleanup(boolean stripWhitespace) throws ParseException { - super.postParseCleanup(stripWhitespace); - return this; - } - - @Override boolean isNestedBlockRepeater() { return false; } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/AutoEscBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/AutoEscBlock.java b/src/main/java/freemarker/core/AutoEscBlock.java index 9622d39..303aba0 100644 --- a/src/main/java/freemarker/core/AutoEscBlock.java +++ b/src/main/java/freemarker/core/AutoEscBlock.java @@ -34,7 +34,7 @@ final class AutoEscBlock extends TemplateElement { @Override TemplateElement[] accept(Environment env) throws TemplateException, IOException { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } @Override http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/Case.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/Case.java b/src/main/java/freemarker/core/Case.java index 586fc3e..baba698 100644 --- a/src/main/java/freemarker/core/Case.java +++ b/src/main/java/freemarker/core/Case.java @@ -19,10 +19,6 @@ package freemarker.core; -import java.io.IOException; - -import freemarker.template.TemplateException; - /** * Represents a case in a switch statement. */ @@ -39,9 +35,8 @@ final class Case extends TemplateElement { } @Override - TemplateElement[] accept(Environment env) - throws TemplateException, IOException { - return new TemplateElement[] { getNestedBlock() }; + TemplateElement[] accept(Environment env) { + return getRegulatedChildren(); } @Override http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/ConditionalBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/ConditionalBlock.java b/src/main/java/freemarker/core/ConditionalBlock.java index 5a733c1..97563cb 100644 --- a/src/main/java/freemarker/core/ConditionalBlock.java +++ b/src/main/java/freemarker/core/ConditionalBlock.java @@ -36,7 +36,6 @@ final class ConditionalBlock extends TemplateElement { final Expression condition; private final int type; - boolean isLonelyIf; ConditionalBlock(Expression condition, TemplateElement nestedBlock, int type) { this.condition = condition; @@ -47,7 +46,7 @@ final class ConditionalBlock extends TemplateElement { @Override TemplateElement[] accept(Environment env) throws TemplateException, IOException { if (condition == null || condition.evalToBoolean(env)) { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } return null; } @@ -66,7 +65,7 @@ final class ConditionalBlock extends TemplateElement { if (getNestedBlock() != null) { buf.append(getNestedBlock().getCanonicalForm()); } - if (isLonelyIf) { + if (!(getParent() instanceof IfBlock)) { buf.append("</#if>"); } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/ElseOfList.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/ElseOfList.java b/src/main/java/freemarker/core/ElseOfList.java index 240589a..8aa6462 100644 --- a/src/main/java/freemarker/core/ElseOfList.java +++ b/src/main/java/freemarker/core/ElseOfList.java @@ -34,7 +34,7 @@ final class ElseOfList extends TemplateElement { @Override TemplateElement[] accept(Environment env) throws TemplateException, IOException { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } @Override http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/EscapeBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/EscapeBlock.java b/src/main/java/freemarker/core/EscapeBlock.java index 1095372..a9be235 100644 --- a/src/main/java/freemarker/core/EscapeBlock.java +++ b/src/main/java/freemarker/core/EscapeBlock.java @@ -48,7 +48,7 @@ class EscapeBlock extends TemplateElement { @Override TemplateElement[] accept(Environment env) throws TemplateException, IOException { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } Expression doEscape(Expression expression) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/IfBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/IfBlock.java b/src/main/java/freemarker/core/IfBlock.java index f976c8e..61e046e 100644 --- a/src/main/java/freemarker/core/IfBlock.java +++ b/src/main/java/freemarker/core/IfBlock.java @@ -48,7 +48,7 @@ final class IfBlock extends TemplateElement { env.replaceElementStackTop(cblock); if (condition == null || condition.evalToBoolean(env)) { if (cblock.getNestedBlock() != null) { - return new TemplateElement[] { cblock.getNestedBlock() }; + return cblock.getRegulatedChildren(); } } } @@ -60,7 +60,6 @@ final class IfBlock extends TemplateElement { throws ParseException { if (getRegulatedChildCount() == 1) { ConditionalBlock cblock = (ConditionalBlock) getRegulatedChild(0); - cblock.isLonelyIf = true; cblock.setLocation(getTemplate(), cblock, this); return cblock.postParseCleanup(stripWhitespace); } else { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/NoAutoEscBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/NoAutoEscBlock.java b/src/main/java/freemarker/core/NoAutoEscBlock.java index 15f3399..acc336a 100644 --- a/src/main/java/freemarker/core/NoAutoEscBlock.java +++ b/src/main/java/freemarker/core/NoAutoEscBlock.java @@ -34,7 +34,7 @@ final class NoAutoEscBlock extends TemplateElement { @Override TemplateElement[] accept(Environment env) throws TemplateException, IOException { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } @Override http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/NoEscapeBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/NoEscapeBlock.java b/src/main/java/freemarker/core/NoEscapeBlock.java index 8513f1d..eb2081a 100644 --- a/src/main/java/freemarker/core/NoEscapeBlock.java +++ b/src/main/java/freemarker/core/NoEscapeBlock.java @@ -33,7 +33,7 @@ class NoEscapeBlock extends TemplateElement { @Override TemplateElement[] accept(Environment env) throws TemplateException, IOException { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } @Override http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/OutputFormatBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/OutputFormatBlock.java b/src/main/java/freemarker/core/OutputFormatBlock.java index e4da5a1..c0660aa 100644 --- a/src/main/java/freemarker/core/OutputFormatBlock.java +++ b/src/main/java/freemarker/core/OutputFormatBlock.java @@ -37,7 +37,7 @@ final class OutputFormatBlock extends TemplateElement { @Override TemplateElement[] accept(Environment env) throws TemplateException, IOException { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } @Override http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/RecoveryBlock.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/RecoveryBlock.java b/src/main/java/freemarker/core/RecoveryBlock.java index 7ec1c8a..8c483b6 100644 --- a/src/main/java/freemarker/core/RecoveryBlock.java +++ b/src/main/java/freemarker/core/RecoveryBlock.java @@ -31,7 +31,7 @@ final class RecoveryBlock extends TemplateElement { @Override TemplateElement[] accept(Environment env) throws TemplateException, IOException { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } @Override http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/Sep.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/Sep.java b/src/main/java/freemarker/core/Sep.java index 522ee2f..7691bfa 100644 --- a/src/main/java/freemarker/core/Sep.java +++ b/src/main/java/freemarker/core/Sep.java @@ -42,7 +42,7 @@ class Sep extends TemplateElement { } if (iterCtx.hasNext()) { - return new TemplateElement[] { getNestedBlock() }; + return getRegulatedChildren(); } return null; } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/TemplateElement.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/TemplateElement.java b/src/main/java/freemarker/core/TemplateElement.java index 2dfcfad..7501588 100644 --- a/src/main/java/freemarker/core/TemplateElement.java +++ b/src/main/java/freemarker/core/TemplateElement.java @@ -46,16 +46,20 @@ abstract public class TemplateElement extends TemplateObject { /** * Used by elements that has no fixed schema for its child elements. For example, a {@code #case} can enclose any - * kind of elements. Only one of {@link #nestedBlock} and {@link #regulatedChildBuffer} can be non-{@code null}. - * This element is typically a {@link MixedContent}, at least before {@link #postParseCleanup(boolean)} (which - * optimizes out {@link MixedContent} with child count less than 2). + * kind of elements. Only one of {@link #nestedBlock} and {@link #regulatedChildBuffer} can be non-{@code null} + * before {@link #postParseCleanup(boolean)}. After {@link #postParseCleanup(boolean)}, the + * {@link #regulatedChildBuffer} is always filled and used for execution. This element is typically a + * {@link MixedContent}, at least before {@link #postParseCleanup(boolean)} (which optimizes out + * {@link MixedContent} with child count less than 2). */ private TemplateElement nestedBlock; /** - * Used by elements that has a fixed schema for its child elements. For example, {@code #switch} can only have - * {@code #case} and {@code #default} child elements. Only one of {@link #nestedBlock} and - * {@link #regulatedChildBuffer} can be non-{@code null}. + * Before {@link #postParseCleanup(boolean)}, it's only used by elements that has a fixed schema for its child + * elements. For example, {@code #switch} can only have {@code #case} and {@code #default} child elements. Before + * {@link #postParseCleanup(boolean)}, only one of {@link #nestedBlock} and {@link #regulatedChildBuffer} can be + * non-{@code null}. After {@link #postParseCleanup(boolean)}, the {@link #regulatedChildBuffer} is always filled + * and used for execution. */ private TemplateElement[] regulatedChildBuffer; private int regulatedChildCount; @@ -148,17 +152,18 @@ abstract public class TemplateElement extends TemplateObject { } public TemplateSequenceModel getChildNodes() { - if (regulatedChildBuffer != null) { - final SimpleSequence seq = new SimpleSequence(regulatedChildCount); - for (int i = 0; i < regulatedChildCount; i++) { - seq.add(regulatedChildBuffer[i]); - } - return seq; - } SimpleSequence result = new SimpleSequence(1); if (nestedBlock != null) { result.add(nestedBlock); - } + } else { + if (regulatedChildBuffer != null) { + final SimpleSequence seq = new SimpleSequence(regulatedChildCount); + for (int i = 0; i < regulatedChildCount; i++) { + seq.add(regulatedChildBuffer[i]); + } + return seq; + } + } return result; } @@ -201,13 +206,13 @@ abstract public class TemplateElement extends TemplateObject { } public int getChildCount() { - if (nestedBlock instanceof MixedContent) { - return nestedBlock.getChildCount(); - } + // Note: regulatedChildren is possibly filled for optimization despite that nestedBlock is filled too, but then + // it should only be utilized for execution, as those children might skip a MixedContent parent. if (nestedBlock != null) { - return 1; + return nestedBlock instanceof MixedContent ? nestedBlock.getChildCount() : 1; + } else { + return regulatedChildCount; } - return regulatedChildCount; } /** @@ -215,11 +220,11 @@ abstract public class TemplateElement extends TemplateObject { * {@link MixedContent}. */ public Enumeration children() { - if (nestedBlock instanceof MixedContent) { - return nestedBlock.children(); - } + // Note: regulatedChildren is possibly filled for optimization despite that nestedBlock is filled too, but then + // it should only be utilized for execution, as those children might skip a MixedContent parent. if (nestedBlock != null) { - return Collections.enumeration(Collections.singletonList(nestedBlock)); + return nestedBlock instanceof MixedContent + ? nestedBlock.children() : Collections.enumeration(Collections.singletonList(nestedBlock)); } else if (regulatedChildBuffer != null) { return new _ArrayEnumeration(regulatedChildBuffer, regulatedChildCount); } @@ -227,10 +232,11 @@ abstract public class TemplateElement extends TemplateObject { } public TemplateElement getChildAt(int index) { + // Note: regulatedChildren is possibly filled for optimization despite that nestedBlock is filled too, but then + // it should only be utilized for execution, as those children might skip a MixedContent parent. if (nestedBlock instanceof MixedContent) { return nestedBlock.getChildAt(index); - } - if (nestedBlock != null) { + } else if (nestedBlock != null) { if (index == 0) { return nestedBlock; } @@ -249,11 +255,17 @@ abstract public class TemplateElement extends TemplateObject { public void setChildAt(int index, TemplateElement element) { if (nestedBlock instanceof MixedContent) { nestedBlock.setChildAt(index, element); + if (regulatedChildBuffer != null) { + regulatedChildBuffer[index] = element; + } } else if (nestedBlock != null) { if (index == 0) { nestedBlock = element; element.index = 0; element.parent = this; + if (regulatedChildBuffer != null) { + regulatedChildBuffer[0] = element; + } } else { throw new IndexOutOfBoundsException("invalid index"); } @@ -358,7 +370,7 @@ abstract public class TemplateElement extends TemplateObject { } /** - * Walk the AST subtree rooted by this element, and do simplifications where possible, also remove superfluous + * Walk the AST subtree rooted by this element, and do simplifications where possible, also removes superfluous * whitespace. * * @param stripWhitespace @@ -370,7 +382,25 @@ abstract public class TemplateElement extends TemplateObject { */ TemplateElement postParseCleanup(boolean stripWhitespace) throws ParseException { int regulatedChildCount = this.regulatedChildCount; - if (regulatedChildCount != 0) { + if (nestedBlock != null) { + nestedBlock = nestedBlock.postParseCleanup(stripWhitespace); + if (nestedBlock.isIgnorable()) { + nestedBlock = null; + } else { + nestedBlock.parent = this; + if (nestedBlock instanceof MixedContent) { + // As MixedContent.accept does nothing but return its regulatedChildren, while it will be present as + // a child, the parent element also will have regularedChildren that contains the children of the + // MixedContent directly. + this.regulatedChildBuffer = nestedBlock.getRegulatedChildren(); + this.regulatedChildCount = nestedBlock.getRegulatedChildCount(); + } else { + // Because execution will use regularChildren only. + this.regulatedChildBuffer = new TemplateElement[] { nestedBlock }; + this.regulatedChildCount = 1; + } + } + } else if (regulatedChildCount != 0) { for (int i = 0; i < regulatedChildCount; i++) { TemplateElement te = regulatedChildBuffer[i]; te = te.postParseCleanup(stripWhitespace); @@ -402,14 +432,7 @@ abstract public class TemplateElement extends TemplateObject { } regulatedChildBuffer = trimmedregulatedChildBuffer; } - } else if (nestedBlock != null) { - nestedBlock = nestedBlock.postParseCleanup(stripWhitespace); - if (nestedBlock.isIgnorable()) { - nestedBlock = null; - } else { - nestedBlock.parent = this; - } - } + } return this; } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/main/java/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java b/src/main/java/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java index 29ec292..7a0e545 100644 --- a/src/main/java/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java +++ b/src/main/java/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java @@ -72,7 +72,7 @@ class ThreadInterruptionSupportTemplatePostProcessor extends TemplatePostProcess // Because nestedElements (means fixed schema for the children) and nestedBlock (means no fixed schema) are // mutually exclusive, and we only care about the last kind: if (te.isNestedBlockRepeater()) { - if (regulatedChildrenCount != 0) { + if (nestedBlock == null && regulatedChildrenCount != 0) { // Only elements that use nestedBlock instead of regulatedChildren should be block repeaters. // Note that nestedBlock and nestedElements are (should be) mutually exclusive. throw new BugException(); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/83eca1b2/src/test/java/freemarker/core/ASTPrinter.java ---------------------------------------------------------------------- diff --git a/src/test/java/freemarker/core/ASTPrinter.java b/src/test/java/freemarker/core/ASTPrinter.java index 9d2c53d..ace9818 100644 --- a/src/test/java/freemarker/core/ASTPrinter.java +++ b/src/test/java/freemarker/core/ASTPrinter.java @@ -263,11 +263,18 @@ public class ASTPrinter { int ln = te.getRegulatedChildCount(); for (int i = 0; i < ln; i++) { TemplateElement child = te.getRegulatedChild(i); - if (child.getParentElement() != te) { + TemplateElement parentElement = child.getParentElement(); + // As MixedContent.accept does nothing but return its regulatedChildren, it's optimized out in the final + // AST tree. While it will be present as a child, the parent element also will have regularedChildren + // that contains the children of the MixedContent directly. + if (parentElement instanceof MixedContent && parentElement.getParent() != null) { + parentElement = parentElement.getParent(); + } + if (parentElement != te) { throw new InvalidASTException("Wrong parent node." + "\nNode: " + child.dump(false) + "\nExpected parent: " + te.dump(false) - + "\nActual parent: " + child.getParentElement().dump(false)); + + "\nActual parent: " + parentElement.dump(false)); } if (child.getIndex() != i) { throw new InvalidASTException("Wrong node index." @@ -280,9 +287,42 @@ public class ASTPrinter { throw new InvalidASTException("Mixed content with child count less than 2 should removed by optimizatoin, " + "but found one with " + te.getRegulatedChildCount() + " child(ren)."); } - if (te.getRegulatedChildCount() != 0 && te.getNestedBlock() != null) { - throw new InvalidASTException("Can't have both nestedBlock and regulatedChildren." - + "\nNode: " + te.dump(false)); + TemplateElement nestedBlock = te.getNestedBlock(); + TemplateElement[] regulatedChildren = te.getRegulatedChildren(); + if (nestedBlock != null) { + if (regulatedChildren == null) { + throw new InvalidASTException( + "RegularChildren must be filled after postParseCleanup if there's a nestedBlock." + + "\nNode: " + te.dump(false)); + } + if (nestedBlock instanceof MixedContent + && (te.getRegulatedChildCount() != nestedBlock.getRegulatedChildCount() + || regulatedChildren != nestedBlock.getRegulatedChildren())) { + throw new InvalidASTException( + "MixedContent.regularChildren must be the same as its parent's regularChildren." + + "\nNode: " + te.dump(false)); + } else if (!(nestedBlock instanceof MixedContent) && te.getRegulatedChildCount() != 1) { + throw new InvalidASTException( + "non-MixedContent-related regularChildren must be of lenght 1 where there's a nestedBlock, " + + "but was " + te.getRegulatedChildCount() + + "\nNode: " + te.dump(false)); + } + } + if (regulatedChildren != null) { + for (int i = 0; i < te.getRegulatedChildCount(); i++) { + if (regulatedChildren[i] == null) { + throw new InvalidASTException( + "regularChildren can't be null at index " + i + + "\nNode: " + te.dump(false)); + } + } + for (int i = te.getRegulatedChildCount(); i < regulatedChildren.length; i++) { + if (regulatedChildren[i] != null) { + throw new InvalidASTException( + "regularChildren can't be non-null at index " + i + + "\nNode: " + te.dump(false)); + } + } } }
