This is an automated email from the ASF dual-hosted git repository. ddekany pushed a commit to branch 2.3-gae in repository https://gitbox.apache.org/repos/asf/freemarker.git
commit fa54bbfbcf1d2a5bc98e69a0f4a9950d9aff1b3a Author: ddekany <[email protected]> AuthorDate: Wed Aug 7 23:06:32 2019 +0200 Fixed bug when lazily generated result was passed to ?join/?seq_contains/?seq_index_of, and the resulting method wasn't immediately called. Now left hand operand will be eagerly evaluated in such case. --- .../core/BuiltInWithDirectCallOptimization.java | 31 ++++++++++++++++++++++ .../java/freemarker/core/BuiltInsForSequences.java | 20 +++++++------- src/main/javacc/FTL.jj | 18 +++++++++++++ .../core/LazilyGeneratedCollectionTest.java | 22 +++++++++++++++ src/test/java/freemarker/core/MapBiTest.java | 5 ++++ 5 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/main/java/freemarker/core/BuiltInWithDirectCallOptimization.java b/src/main/java/freemarker/core/BuiltInWithDirectCallOptimization.java new file mode 100644 index 0000000..a92ae43 --- /dev/null +++ b/src/main/java/freemarker/core/BuiltInWithDirectCallOptimization.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package freemarker.core; + +abstract class BuiltInWithDirectCallOptimization extends SpecialBuiltIn { + + /** + * Called if the built-in is directly followed by a "(" (ignoring comments and white-space). This can be utilized + * for optimizations that only work correctly if the method returned by the built-in is a called immediately (as + * opposed to being stored in a variable and then called at an arbitrary later point in time). + */ + protected abstract void setDirectlyCalled(); + +} diff --git a/src/main/java/freemarker/core/BuiltInsForSequences.java b/src/main/java/freemarker/core/BuiltInsForSequences.java index 2ba6b11..55d634e 100644 --- a/src/main/java/freemarker/core/BuiltInsForSequences.java +++ b/src/main/java/freemarker/core/BuiltInsForSequences.java @@ -181,11 +181,11 @@ class BuiltInsForSequences { } - static class joinBI extends BuiltIn { + static class joinBI extends BuiltInWithDirectCallOptimization { @Override - protected boolean isLazilyGeneratedTargetResultSupported() { - return true; + protected void setDirectlyCalled() { + target.enableLazilyGeneratedResult(); } private class BIMethodForCollection implements TemplateMethodModelEx { @@ -295,11 +295,11 @@ class BuiltInsForSequences { } } - static class seq_containsBI extends BuiltIn { + static class seq_containsBI extends BuiltInWithDirectCallOptimization { @Override - protected boolean isLazilyGeneratedTargetResultSupported() { - return true; + protected void setDirectlyCalled() { + target.enableLazilyGeneratedResult(); } private class BIMethodForCollection implements TemplateMethodModelEx { @@ -367,15 +367,15 @@ class BuiltInsForSequences { } - static class seq_index_ofBI extends BuiltIn { + static class seq_index_ofBI extends BuiltInWithDirectCallOptimization { @Override - protected boolean isLazilyGeneratedTargetResultSupported() { - return true; + protected void setDirectlyCalled() { + target.enableLazilyGeneratedResult(); } private class BIMethod implements TemplateMethodModelEx { - + protected final TemplateSequenceModel m_seq; protected final TemplateCollectionModel m_col; protected final Environment m_env; diff --git a/src/main/javacc/FTL.jj b/src/main/javacc/FTL.jj index a157871..363e8bc 100644 --- a/src/main/javacc/FTL.jj +++ b/src/main/javacc/FTL.jj @@ -2182,6 +2182,7 @@ Expression BuiltIn(Expression lhoExp) : ArrayList<Expression> args = null; Token openParen; Token closeParen; + MethodCall methodCall; } { <BUILT_IN> @@ -2236,6 +2237,7 @@ Expression BuiltIn(Expression lhoExp) : return result; } } + [ LOOKAHEAD({ result instanceof BuiltInWithParseTimeParameters @@ -2265,10 +2267,26 @@ Expression BuiltIn(Expression lhoExp) : return result; } ] + + [ + LOOKAHEAD(<OPEN_PAREN>, { result instanceof BuiltInWithDirectCallOptimization }) + methodCall = MethodArgs(result) + { + ((BuiltInWithDirectCallOptimization) result).setDirectlyCalled(); + return methodCall; + } + ] + { + if (result instanceof BuiltInWithDirectCallOptimization) { + // We had no (...) + return result; + } + // Should have already return-ed throw new AssertionError("Unhandled " + SpecialBuiltIn.class.getName() + " subclass: " + result.getClass()); } + } // Only supported as the argument of certain built-ins, so it's not called inside Expression. diff --git a/src/test/java/freemarker/core/LazilyGeneratedCollectionTest.java b/src/test/java/freemarker/core/LazilyGeneratedCollectionTest.java index fbbf226..0dee5f4 100644 --- a/src/test/java/freemarker/core/LazilyGeneratedCollectionTest.java +++ b/src/test/java/freemarker/core/LazilyGeneratedCollectionTest.java @@ -265,6 +265,28 @@ public class LazilyGeneratedCollectionTest extends TemplateTest { assertOutput("${seqLong?filter(x->true)[2..*3]?size}", "[size][get 0][get 1][get 2][get 3][get 4]3"); } + @Test + public void testNonDirectCalledBuiltInsAreNotLazy() throws Exception { + assertOutput("" + + "<#assign changing = 1>" + + "<#assign method = [1, 2]?filter(it -> it != changing)?join>" + + "<#assign changing = 2>" + + "${method(', ')}", + "2"); + assertOutput("" + + "<#assign changing = 1>" + + "<#assign method = [1, 2]?filter(it -> it != changing)?seq_contains>" + + "<#assign changing = 2>" + + "${method(2)?c}", + "true"); + assertOutput("" + + "<#assign changing = 1>" + + "<#assign method = [1, 2]?filter(it -> it != changing)?seq_index_of>" + + "<#assign changing = 2>" + + "${method(2)}", + "0"); + } + public static abstract class ListContainingTemplateModel { protected final List<Number> elements; diff --git a/src/test/java/freemarker/core/MapBiTest.java b/src/test/java/freemarker/core/MapBiTest.java index 3e70c0f..927ea52 100644 --- a/src/test/java/freemarker/core/MapBiTest.java +++ b/src/test/java/freemarker/core/MapBiTest.java @@ -165,6 +165,11 @@ public class MapBiTest extends TemplateTest { "<#function tenTimes(x)><#assign s += '${x};'><#return x * 10></#function>" + "${(1..3)?map(tenTimes)?seqIndexOf(20)} ${s}", "1 1;2;"); + assertOutput("" + + "<#assign s = ''>" + + "<#function tenTimes(x)><#assign s += '${x};'><#return x * 10></#function>" + + "${[1, 2, 3, 2, 5]?map(tenTimes)?seqLastIndexOf(20)} ${s}", "3 1;2;3;2;5;"); + // For these this test can't check that there was no sequence built, but at least we know they are working: assertOutput("${(1..3)?map(it -> it * 10)?min}", "10"); assertOutput("${(1..3)?map(it -> it * 10)?max}", "30");
