You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2019/03/31 22:34:01 UTC

[freemarker] branch 2.3-gae updated (02fa5ed -> 700e87e)

This is an automated email from the ASF dual-hosted git repository.

ddekany pushed a change to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git.


    from 02fa5ed  Version number typo fix
     new abaed15  Some cleanup related to local lambdas, ?filter and ?map. Added optimizations to ?size operating on the result of ?filter/?map.
     new e7d91e6  Made ?size smarter when its result is compared to an integer literal. Specifically, for ?filter-ed sequences (or for the result of similar future Stream-API-like built-ins) it won't count more elements then it's necessary to tell the comparison result. Also, for a TemplateCollectionModelEx, it calls isEmpty() instead of size(), if for the comparison result we only have to know if we have more than 0 elements.
     new 700e87e  (Added some version history entry for ?filter/?map and local lambdas... documentation is still missing)

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/main/java/freemarker/core/BuiltIn.java         |  11 +-
 .../freemarker/core/BuiltInsForMultipleTypes.java  |  56 ++++-
 .../java/freemarker/core/BuiltInsForSequences.java | 101 +++++----
 .../java/freemarker/core/ComparisonExpression.java |  16 ++
 src/main/java/freemarker/core/Environment.java     |   2 -
 src/main/java/freemarker/core/EvalUtil.java        |  12 ++
 src/main/java/freemarker/core/IteratorBlock.java   |   8 +-
 ...ator.java => LazilyGeneratedSequenceModel.java} |  27 +--
 ...va => LazyCollectionTemplateModelIterator.java} |  33 +--
 ...enceIterator.java => LazySequenceIterator.java} |  19 +-
 src/main/java/freemarker/core/MiscUtil.java        |   7 +
 ...a => SameSizeLazilyGeneratedSequenceModel.java} |  36 ++--
 ...> SameSizeSeqLazilyGeneratedSequenceModel.java} |  35 +--
 src/manual/en_US/book.xml                          |  34 +++
 ...ilyGeneratedSeqTargetSupportInBuiltinsTest.java | 237 +++++++++++++++++++++
 .../core/ListWithStreamLikeBuiltinsTest.java       |   2 +-
 16 files changed, 511 insertions(+), 125 deletions(-)
 copy src/main/java/freemarker/core/{SequenceIterator.java => LazilyGeneratedSequenceModel.java} (61%)
 copy src/main/java/freemarker/core/{SingleIterationCollectionModel.java => LazyCollectionTemplateModelIterator.java} (58%)
 copy src/main/java/freemarker/core/{SequenceIterator.java => LazySequenceIterator.java} (70%)
 copy src/main/java/freemarker/core/{SingleIterationCollectionModel.java => SameSizeLazilyGeneratedSequenceModel.java} (52%)
 copy src/main/java/freemarker/core/{SingleIterationCollectionModel.java => SameSizeSeqLazilyGeneratedSequenceModel.java} (52%)
 create mode 100644 src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java


[freemarker] 01/03: Some cleanup related to local lambdas, ?filter and ?map. Added optimizations to ?size operating on the result of ?filter/?map.

Posted by dd...@apache.org.
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 abaed15caf0d3e43da2d1e6abfecbda7b03c20f9
Author: ddekany <dd...@apache.org>
AuthorDate: Sun Mar 31 16:43:31 2019 +0200

    Some cleanup related to local lambdas, ?filter and ?map. Added optimizations to ?size operating on the result of ?filter/?map.
---
 src/main/java/freemarker/core/BuiltIn.java         |   6 +-
 .../freemarker/core/BuiltInsForMultipleTypes.java  |  21 +++
 .../java/freemarker/core/BuiltInsForSequences.java | 101 +++++++-----
 src/main/java/freemarker/core/Environment.java     |   2 -
 src/main/java/freemarker/core/IteratorBlock.java   |   4 +-
 .../core/LazilyGeneratedSequenceModel.java         |  35 +++++
 .../core/LazyCollectionTemplateModelIterator.java  |  54 +++++++
 .../java/freemarker/core/LazySequenceIterator.java |  55 +++++++
 .../core/SameSizeLazilyGeneratedSequenceModel.java |  49 ++++++
 .../SameSizeSeqLazilyGeneratedSequenceModel.java   |  52 ++++++
 ...ilyGeneratedSeqTargetSupportInBuiltinsTest.java | 174 +++++++++++++++++++++
 .../core/ListWithStreamLikeBuiltinsTest.java       |   2 +-
 12 files changed, 507 insertions(+), 48 deletions(-)

diff --git a/src/main/java/freemarker/core/BuiltIn.java b/src/main/java/freemarker/core/BuiltIn.java
index 450dd6d..0679e21 100644
--- a/src/main/java/freemarker/core/BuiltIn.java
+++ b/src/main/java/freemarker/core/BuiltIn.java
@@ -387,16 +387,16 @@ abstract class BuiltIn extends Expression implements Cloneable {
         }
         bi.key = key;
         bi.target = target;
-        if (bi.isSingleIterationCollectionTargetSupported()) {
+        if (bi.isLazilyGeneratedSequenceModelTargetSupported()) {
             if (target instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
                 ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) target)
-                        .setLazyProcessingAllowed(true);
+                        .setLazyResultGenerationAllowed(true);
             }
         }
         return bi;
     }
 
-    protected boolean isSingleIterationCollectionTargetSupported() {
+    protected boolean isLazilyGeneratedSequenceModelTargetSupported() {
         return false;
     }
 
diff --git a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
index e0640c2..61f8f3f 100644
--- a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
+++ b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
@@ -40,6 +40,7 @@ import freemarker.template.TemplateHashModelEx;
 import freemarker.template.TemplateMethodModel;
 import freemarker.template.TemplateModel;
 import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateModelIterator;
 import freemarker.template.TemplateModelWithAPISupport;
 import freemarker.template.TemplateNodeModel;
 import freemarker.template.TemplateNumberModel;
@@ -481,6 +482,12 @@ class BuiltInsForMultipleTypes {
     }
 
     static class sizeBI extends BuiltIn {
+
+        @Override
+        protected boolean isLazilyGeneratedSequenceModelTargetSupported() {
+            return true;
+        }
+
         @Override
         TemplateModel _eval(Environment env) throws TemplateException {
             TemplateModel model = target.eval(env);
@@ -492,6 +499,20 @@ class BuiltInsForMultipleTypes {
                 size = ((TemplateCollectionModelEx) model).size();
             } else if (model instanceof TemplateHashModelEx) {
                 size = ((TemplateHashModelEx) model).size();
+            } else if (model instanceof LazilyGeneratedSequenceModel) {
+                // While this is a TemplateCollectionModel, and thus ?size will be O(N), and N might be infinite,
+                // it's for the result of ?filter(predicate) or such. Those "officially" return a sequence. Returning a
+                // TemplateCollectionModel (a LazilyGeneratedSequenceModel more specifically) is a (mostly) transparent
+                // optimization to avoid creating the result sequence in memory, which would be unnecessary work for
+                // ?size. Creating that result sequence would be O(N) too, so the O(N) time complexity should be
+                // expected by the template author, and we just made that calculation less wasteful here.
+                TemplateModelIterator iterator = ((LazilyGeneratedSequenceModel) model).iterator();
+                int counter = 0;
+                while (iterator.hasNext()) {
+                    counter++;
+                    iterator.next();
+                }
+                size = counter;
             } else {
                 throw new UnexpectedTypeException(
                         target, model,
diff --git a/src/main/java/freemarker/core/BuiltInsForSequences.java b/src/main/java/freemarker/core/BuiltInsForSequences.java
index 0bb2bd9..b4acd16 100644
--- a/src/main/java/freemarker/core/BuiltInsForSequences.java
+++ b/src/main/java/freemarker/core/BuiltInsForSequences.java
@@ -144,7 +144,7 @@ class BuiltInsForSequences {
     static class firstBI extends BuiltIn {
 
         @Override
-        protected boolean isSingleIterationCollectionTargetSupported() {
+        protected boolean isLazilyGeneratedSequenceModelTargetSupported() {
             return true;
         }
 
@@ -185,7 +185,7 @@ class BuiltInsForSequences {
     static class joinBI extends BuiltIn {
 
         @Override
-        protected boolean isSingleIterationCollectionTargetSupported() {
+        protected boolean isLazilyGeneratedSequenceModelTargetSupported() {
             return true;
         }
 
@@ -302,7 +302,7 @@ class BuiltInsForSequences {
     static class seq_containsBI extends BuiltIn {
 
         @Override
-        protected boolean isSingleIterationCollectionTargetSupported() {
+        protected boolean isLazilyGeneratedSequenceModelTargetSupported() {
             return true;
         }
 
@@ -374,7 +374,7 @@ class BuiltInsForSequences {
     static class seq_index_ofBI extends BuiltIn {
 
         @Override
-        protected boolean isSingleIterationCollectionTargetSupported() {
+        protected boolean isLazilyGeneratedSequenceModelTargetSupported() {
             return true;
         }
 
@@ -915,7 +915,7 @@ class BuiltInsForSequences {
         }
 
         @Override
-        protected boolean isSingleIterationCollectionTargetSupported() {
+        protected boolean isLazilyGeneratedSequenceModelTargetSupported() {
             return true;
         }
 
@@ -980,19 +980,20 @@ class BuiltInsForSequences {
     }
 
     /**
-     * Built-in that's similar to an Java 8 Stream intermediate operation. To be on the safe side, by default these
-     * are eager, and just produce a {@link TemplateSequenceModel}. But when circumstances allow, they become
-     * lazy, similarly to Java 8 Stream-s. Another characteristic of the built-ins that they usually accept
-     * lambda expressions as parameters.
+     * Built-in that's similar to a Java 8 Stream intermediate operation. To be on the safe side, by default these
+     * are eager, and just produce a {@link TemplateSequenceModel}. But when circumstances allow, they become lazy,
+     * similarly to Java 8 Stream intermediate operations. Another characteristic of these built-ins is that they
+     * usually accept lambda expressions as parameters.
      */
     static abstract class IntermediateStreamOperationLikeBuiltIn extends BuiltInWithParseTimeParameters {
 
         private Expression elementTransformerExp;
         private ElementTransformer precreatedElementTransformer;
-        private boolean lazyProcessingAllowed;
+        private boolean lazyResultGenerationAllowed;
 
         @Override
         void bindToParameters(List<Expression> parameters, Token openParen, Token closeParen) throws ParseException {
+            // At the moment all built-ins of this kind requires 1 parameter.
             if (parameters.size() != 1) {
                 throw newArgumentCountException("requires exactly 1", openParen, closeParen);
             }
@@ -1003,10 +1004,6 @@ class BuiltInsForSequences {
                 // We can't do this with other kind of expressions, as they need to be evaluated on runtime:
                 precreatedElementTransformer = new LocalLambdaElementTransformer(localLambdaExp);
             }
-
-            if (target instanceof IntermediateStreamOperationLikeBuiltIn) {
-                ((IntermediateStreamOperationLikeBuiltIn) target).setLazyProcessingAllowed(true);
-            }
         }
 
         @Override
@@ -1014,12 +1011,17 @@ class BuiltInsForSequences {
             return true;
         }
 
-        boolean isLazyProcessingAllowed() {
-            return lazyProcessingAllowed;
+        @Override
+        protected boolean isLazilyGeneratedSequenceModelTargetSupported() {
+            return true;
+        }
+
+        final boolean isLazyResultGenerationAllowed() {
+            return lazyResultGenerationAllowed;
         }
 
         /**
-         * Used to allow processing of the collection or sequence elements on an as-needed basis, similarly as
+         * Used to allow generating the result collection or sequence elements on an as-needed basis, similarly as
          * Java 8 Stream intermediate operations do it. This is initially {@code false}. The containing expression or
          * directive sets it to {@code true} if it can ensure that:
          * <ul>
@@ -1030,8 +1032,8 @@ class BuiltInsForSequences {
          *       built-in was called. This is required as lambda expression are {@link LocalLambdaExpression}-s.
          * </ul>
          */
-        void setLazyProcessingAllowed(boolean lazyProcessingAllowed) {
-            this.lazyProcessingAllowed = lazyProcessingAllowed;
+        void setLazyResultGenerationAllowed(boolean lazyResultGenerationAllowed) {
+            this.lazyResultGenerationAllowed = lazyResultGenerationAllowed;
         }
 
         protected List<Expression> getArgumentsAsList() {
@@ -1084,10 +1086,12 @@ class BuiltInsForSequences {
         private TemplateModelIterator getTemplateModelIterator(Environment env, TemplateModel model) throws TemplateModelException,
                 NonSequenceOrCollectionException, InvalidReferenceException {
             if (model instanceof TemplateCollectionModel) {
-                return ((TemplateCollectionModel) model).iterator();
+                return isLazyResultGenerationAllowed()
+                        ? new LazyCollectionTemplateModelIterator((TemplateCollectionModel) model)
+                        : ((TemplateCollectionModel) model).iterator();
             } else if (model instanceof TemplateSequenceModel) {
-                return new SequenceIterator((TemplateSequenceModel) model);
-            } else if (model instanceof TemplateModelIterator) { // For a stream mode LHO
+                return new LazySequenceIterator((TemplateSequenceModel) model);
+            } else if (model instanceof TemplateModelIterator) { // For a lazily generated LHO
                 return (TemplateModelIterator) model;
             } else {
                 throw new NonSequenceOrCollectionException(target, model, env);
@@ -1095,8 +1099,10 @@ class BuiltInsForSequences {
         }
 
         /**
-         * @param lhoIterator Use this to iterate through the items
-         * @param lho Maybe needed for operations specific to the built-in, like getting the size
+         * @param lhoIterator Use this to read the elements of the left hand operand
+         * @param lho Maybe needed for operations specific to the built-in, like getting the size, otherwise use the
+         *           {@code lhoIterator} only.
+         * @param elementTransformer The argument to the built-in (typically a lambda expression)
          *
          * @return {@link TemplateSequenceModel} or {@link TemplateCollectionModel} or {@link TemplateModelIterator}.
          */
@@ -1104,10 +1110,15 @@ class BuiltInsForSequences {
                 TemplateModelIterator lhoIterator, TemplateModel lho, ElementTransformer elementTransformer,
                 Environment env) throws TemplateException;
 
+        /**
+         * Wraps the built-in argument that specifies how to transform the elements of the sequence, to hide the
+         * complexity of doing that.
+         */
         interface ElementTransformer {
             TemplateModel transformElement(TemplateModel element, Environment env) throws TemplateException;
         }
 
+        /** {@link ElementTransformer} that wraps a local lambda expression. */
         private static class LocalLambdaElementTransformer implements ElementTransformer {
             private final LocalLambdaExpression elementTransformerExp;
 
@@ -1120,6 +1131,7 @@ class BuiltInsForSequences {
             }
         }
 
+        /** {@link ElementTransformer} that wraps a (Java) method call. */
         private static class MethodElementTransformer implements ElementTransformer {
             private final TemplateMethodModel elementTransformer;
 
@@ -1134,6 +1146,7 @@ class BuiltInsForSequences {
             }
         }
 
+        /** {@link ElementTransformer} that wraps a call to an FTL function (things defined with {@code #function}). */
         private static class FunctionElementTransformer implements ElementTransformer {
             private final Macro templateTransformer;
             private final Expression elementTransformerExp;
@@ -1145,6 +1158,8 @@ class BuiltInsForSequences {
 
             public TemplateModel transformElement(TemplateModel element, Environment env) throws
                     TemplateException {
+                // #function-s were originally designed to be called from templates directly, so they expect an
+                // Expression as argument. So we have to create a fake one.
                 ExpressionWithFixedResult functionArgExp = new ExpressionWithFixedResult(
                         element, elementTransformerExp);
                 return env.invokeFunction(env, templateTransformer,
@@ -1161,7 +1176,7 @@ class BuiltInsForSequences {
                 final TemplateModelIterator lhoIterator, final TemplateModel lho,
                 final ElementTransformer elementTransformer,
                 final Environment env) throws TemplateException {
-            if (!isLazyProcessingAllowed()) {
+            if (!isLazyResultGenerationAllowed()) {
                 List<TemplateModel> resultList = new ArrayList<TemplateModel>();
                 while (lhoIterator.hasNext()) {
                     TemplateModel element = lhoIterator.next();
@@ -1171,7 +1186,7 @@ class BuiltInsForSequences {
                 }
                 return new TemplateModelListSequence(resultList);
             } else {
-                return new SingleIterationCollectionModel(
+                return new LazilyGeneratedSequenceModel(
                         new TemplateModelIterator() {
                             boolean prefetchDone;
                             TemplateModel prefetchedElement;
@@ -1223,17 +1238,17 @@ class BuiltInsForSequences {
             }
         }
 
-        private boolean elementMatches(TemplateModel element, ElementTransformer elementTransformer, Environment env) throws
-                TemplateException {
+        private boolean elementMatches(TemplateModel element, ElementTransformer elementTransformer, Environment env)
+                throws TemplateException {
             TemplateModel transformedElement = elementTransformer.transformElement(element, env);
             if (!(transformedElement instanceof TemplateBooleanModel)) {
                 if (transformedElement == null) {
                     throw new _TemplateModelException(getElementTransformerExp(), env,
-                            "The element transformer function has returned no return value (has returned null) " +
-                            "instead of a boolean.");
+                            "The filter expression has returned no value (has returned null), " +
+                            "rather than a boolean.");
                 }
                 throw new _TemplateModelException(getElementTransformerExp(), env,
-                        "The element transformer function had to return a boolean value, but it has returned ",
+                        "The filter expression had to return a boolean value, but it returned ",
                         new _DelayedAOrAn(new _DelayedFTLTypeDescription(transformedElement)),
                         " instead.");
             }
@@ -1247,19 +1262,17 @@ class BuiltInsForSequences {
         protected TemplateModel calculateResult(
                 final TemplateModelIterator lhoIterator, TemplateModel lho, final ElementTransformer elementTransformer,
                 final Environment env) throws TemplateException {
-            if (!isLazyProcessingAllowed()) {
+            if (!isLazyResultGenerationAllowed()) {
                 List<TemplateModel> resultList = new ArrayList<TemplateModel>();
                 while (lhoIterator.hasNext()) {
-                    resultList.add(fetchAndTransformNextElement(lhoIterator, elementTransformer, env));
+                    resultList.add(fetchAndMapNextElement(lhoIterator, elementTransformer, env));
                 }
                 return new TemplateModelListSequence(resultList);
             } else {
-                return new SingleIterationCollectionModel(
-                        new TemplateModelIterator() {
-
+                TemplateModelIterator mappedLhoIterator = new TemplateModelIterator() {
                     public TemplateModel next() throws TemplateModelException {
                         try {
-                            return fetchAndTransformNextElement(lhoIterator, elementTransformer, env);
+                            return fetchAndMapNextElement(lhoIterator, elementTransformer, env);
                         } catch (TemplateException e) {
                             throw new _TemplateModelException(e, env, "Failed to transform element");
                         }
@@ -1268,17 +1281,25 @@ class BuiltInsForSequences {
                     public boolean hasNext() throws TemplateModelException {
                         return lhoIterator.hasNext();
                     }
-                });
+                };
+                if (lho instanceof TemplateCollectionModelEx) { // Preferred branch, as TempCollModEx has isEmpty() too
+                    return new SameSizeCollLazilyGeneratedSequenceModel(mappedLhoIterator,
+                            (TemplateCollectionModelEx) lho);
+                } else if (lho instanceof TemplateSequenceModel) {
+                    return new SameSizeSeqLazilyGeneratedSequenceModel(mappedLhoIterator, (TemplateSequenceModel) lho);
+                } else {
+                    return new LazilyGeneratedSequenceModel(mappedLhoIterator);
+                }
             }
         }
 
-        private TemplateModel fetchAndTransformNextElement(
+        private TemplateModel fetchAndMapNextElement(
                 TemplateModelIterator lhoIterator, ElementTransformer elementTransformer, Environment env)
                 throws TemplateException {
             TemplateModel transformedElement = elementTransformer.transformElement(lhoIterator.next(), env);
             if (transformedElement == null) {
                 throw new _TemplateModelException(getElementTransformerExp(), env,
-                        "The element transformer function has returned no return value (has returned null).");
+                        "The element mapper function has returned no return value (has returned null).");
             }
             return transformedElement;
         }
diff --git a/src/main/java/freemarker/core/Environment.java b/src/main/java/freemarker/core/Environment.java
index eff4c91..2bacc33 100644
--- a/src/main/java/freemarker/core/Environment.java
+++ b/src/main/java/freemarker/core/Environment.java
@@ -684,8 +684,6 @@ public final class Environment extends Configurable {
 
     /**
      * Evaluate expression with shadowing a single variable with a new local variable.
-     *
-     * @since 2.3.29
      */
     TemplateModel evaluateWithNewLocal(Expression exp, String lambdaArgName, TemplateModel lamdaArgValue)
             throws TemplateException {
diff --git a/src/main/java/freemarker/core/IteratorBlock.java b/src/main/java/freemarker/core/IteratorBlock.java
index 59e3c35..a51401a 100644
--- a/src/main/java/freemarker/core/IteratorBlock.java
+++ b/src/main/java/freemarker/core/IteratorBlock.java
@@ -84,7 +84,7 @@ final class IteratorBlock extends TemplateElement {
         this.forEach = forEach;
 
         if (listedExp instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
-            ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) listedExp).setLazyProcessingAllowed(true);
+            ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) listedExp).setLazyResultGenerationAllowed(true);
             fetchElementsOutsideLoopVarContext = true;
         } else {
             fetchElementsOutsideLoopVarContext = false;
@@ -307,7 +307,7 @@ final class IteratorBlock extends TemplateElement {
                         openedIterator = null;
                     } else {
                         // We must reuse this later, because TemplateCollectionModel-s that wrap an Iterator only
-                        // allow one iterator() call. (Also those returned by ?filter, etc. with lazy processing on.)
+                        // allow one iterator() call. (Also those returned by ?filter, etc. with lazy result generation.)
                         openedIterator = iterModel;
                         // Note: Loop variables will only become visible inside #items
                         env.visit(childBuffer);
diff --git a/src/main/java/freemarker/core/LazilyGeneratedSequenceModel.java b/src/main/java/freemarker/core/LazilyGeneratedSequenceModel.java
new file mode 100644
index 0000000..a0737f1
--- /dev/null
+++ b/src/main/java/freemarker/core/LazilyGeneratedSequenceModel.java
@@ -0,0 +1,35 @@
+/*
+ * 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;
+
+import freemarker.ext.beans.CollectionModel;
+import freemarker.template.TemplateModelIterator;
+import freemarker.template.TemplateSequenceModel;
+
+/**
+ * Same as {@link SingleIterationCollectionModel}, but marks the value as something that's in principle a
+ * {@link TemplateSequenceModel}, but to allow lazy result generation a {@link CollectionModel} is used internally.
+ * This is an optimization that we do where we consider it to be transparent enough for the user.
+ */
+class LazilyGeneratedSequenceModel extends SingleIterationCollectionModel {
+    public LazilyGeneratedSequenceModel(TemplateModelIterator iterator) {
+        super(iterator);
+    }
+}
diff --git a/src/main/java/freemarker/core/LazyCollectionTemplateModelIterator.java b/src/main/java/freemarker/core/LazyCollectionTemplateModelIterator.java
new file mode 100644
index 0000000..aa55c26
--- /dev/null
+++ b/src/main/java/freemarker/core/LazyCollectionTemplateModelIterator.java
@@ -0,0 +1,54 @@
+/*
+ * 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;
+
+import freemarker.template.TemplateCollectionModel;
+import freemarker.template.TemplateModel;
+import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateModelIterator;
+
+/**
+ * Delays creating the {@link TemplateModelIterator} of the wrapped model until it's actually needed.
+ */
+class LazyCollectionTemplateModelIterator implements TemplateModelIterator {
+
+    private final TemplateCollectionModel templateCollectionModel;
+    private TemplateModelIterator iterator;
+
+    public LazyCollectionTemplateModelIterator(TemplateCollectionModel templateCollectionModel) {
+        this.templateCollectionModel = templateCollectionModel;
+    }
+
+    public TemplateModel next() throws TemplateModelException {
+        ensureIteratorInitialized();
+        return iterator.next();
+    }
+
+    public boolean hasNext() throws TemplateModelException {
+        ensureIteratorInitialized();
+        return iterator.hasNext();
+    }
+
+    private void ensureIteratorInitialized() throws TemplateModelException {
+        if (iterator == null) {
+            iterator = templateCollectionModel.iterator();
+        }
+    }
+}
diff --git a/src/main/java/freemarker/core/LazySequenceIterator.java b/src/main/java/freemarker/core/LazySequenceIterator.java
new file mode 100644
index 0000000..8bcf78d
--- /dev/null
+++ b/src/main/java/freemarker/core/LazySequenceIterator.java
@@ -0,0 +1,55 @@
+/*
+ * 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;
+
+import freemarker.template.TemplateModel;
+import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateModelIterator;
+import freemarker.template.TemplateSequenceModel;
+
+/**
+ * {@link TemplateModelIterator} that wraps a sequence, delaying calling any of its methods until it's actually needed.
+ *
+ * @since 2.3.29
+ */
+class LazySequenceIterator implements TemplateModelIterator {
+    private final TemplateSequenceModel sequence;
+    private Integer size;
+    private int index = 0;
+
+    LazySequenceIterator(TemplateSequenceModel sequence) throws TemplateModelException {
+        this.sequence = sequence;
+
+    }
+    public TemplateModel next() throws TemplateModelException {
+        return sequence.get(index++);
+    }
+
+    public boolean hasNext() {
+        if (size == null) {
+            try {
+                size = sequence.size();
+            } catch (TemplateModelException e) {
+                throw new RuntimeException("Error when getting sequence size", e);
+            }
+        }
+        return index < size;
+    }
+}
diff --git a/src/main/java/freemarker/core/SameSizeLazilyGeneratedSequenceModel.java b/src/main/java/freemarker/core/SameSizeLazilyGeneratedSequenceModel.java
new file mode 100644
index 0000000..a0f4225
--- /dev/null
+++ b/src/main/java/freemarker/core/SameSizeLazilyGeneratedSequenceModel.java
@@ -0,0 +1,49 @@
+/*
+ * 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;
+
+import freemarker.template.TemplateCollectionModelEx;
+import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateModelIterator;
+import freemarker.template.utility.NullArgumentException;
+
+/**
+ * Used instead of {@link LazilyGeneratedSequenceModel} for operations that don't change the element count of the
+ * source, if the source can also give back an element count.
+ */
+class SameSizeCollLazilyGeneratedSequenceModel extends LazilyGeneratedSequenceModel
+        implements TemplateCollectionModelEx {
+    private final TemplateCollectionModelEx sizeSourceCollEx;
+
+    public SameSizeCollLazilyGeneratedSequenceModel(
+            TemplateModelIterator iterator, TemplateCollectionModelEx sizeSourceCollEx) {
+        super(iterator);
+        NullArgumentException.check(sizeSourceCollEx);
+        this.sizeSourceCollEx = sizeSourceCollEx;
+    }
+
+    public int size() throws TemplateModelException {
+        return sizeSourceCollEx.size();
+    }
+
+    public boolean isEmpty() throws TemplateModelException {
+        return sizeSourceCollEx.isEmpty();
+    }
+}
diff --git a/src/main/java/freemarker/core/SameSizeSeqLazilyGeneratedSequenceModel.java b/src/main/java/freemarker/core/SameSizeSeqLazilyGeneratedSequenceModel.java
new file mode 100644
index 0000000..d6d7b5f
--- /dev/null
+++ b/src/main/java/freemarker/core/SameSizeSeqLazilyGeneratedSequenceModel.java
@@ -0,0 +1,52 @@
+/*
+ * 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;
+
+import freemarker.template.TemplateCollectionModelEx;
+import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateModelIterator;
+import freemarker.template.TemplateSequenceModel;
+import freemarker.template.utility.NullArgumentException;
+
+/**
+ * Used instead of {@link LazilyGeneratedSequenceModel} for operations that don't change the element count of the
+ * source, if the source can also give back an element count.
+ *
+ * @since 2.3.29
+ */
+class SameSizeSeqLazilyGeneratedSequenceModel extends LazilyGeneratedSequenceModel
+        implements TemplateCollectionModelEx {
+    private final TemplateSequenceModel sizeSourceSeq;
+
+    public SameSizeSeqLazilyGeneratedSequenceModel(
+            TemplateModelIterator iterator, TemplateSequenceModel sizeSourceSeq) {
+        super(iterator);
+        NullArgumentException.check(sizeSourceSeq);
+        this.sizeSourceSeq = sizeSourceSeq;
+    }
+
+    public int size() throws TemplateModelException {
+        return sizeSourceSeq.size();
+    }
+
+    public boolean isEmpty() throws TemplateModelException {
+        return sizeSourceSeq.size() == 0;
+    }
+}
diff --git a/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java b/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java
new file mode 100644
index 0000000..37cefa8
--- /dev/null
+++ b/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java
@@ -0,0 +1,174 @@
+/*
+ * 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;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+
+import org.junit.Test;
+
+import freemarker.template.Configuration;
+import freemarker.template.SimpleNumber;
+import freemarker.template.TemplateCollectionModel;
+import freemarker.template.TemplateCollectionModelEx;
+import freemarker.template.TemplateModel;
+import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateModelIterator;
+import freemarker.template.TemplateSequenceModel;
+import freemarker.test.TemplateTest;
+
+/**
+ * Tests built-ins that are support getting {@link LazilyGeneratedSequenceModel} as their LHO input.
+ */
+public class LazilyGeneratedSeqTargetSupportInBuiltinsTest extends TemplateTest {
+
+    @Test
+    public void sizeTest() throws Exception {
+        assertOutput("${seq?size}",
+                "[size]3");
+        assertOutput("${collEx?size}",
+                "[size]3");
+        assertErrorContains("${coll?size}",
+                "sequence", "extended collection");
+
+        assertOutput("${seq?map(x -> x * 10)?size}",
+                "[size]3");
+        assertOutput("${collEx?map(x -> x * 10)?size}",
+                "[size]3");
+
+        assertOutput("${seq?filter(x -> x != 1)?size}",
+                "[size][get 0][get 1][get 2]2");
+        assertOutput("${collEx?filter(x -> x != 1)?size}",
+                "[iterator][hasNext][next][hasNext][next][hasNext][next][hasNext]2");
+    }
+
+    @Test
+    public void firstTest() throws Exception {
+        assertOutput("${coll?first}",
+                "[iterator][hasNext][next]1");
+        assertOutput("${coll?filter(x -> x % 2 == 0)?first}",
+                "[iterator][hasNext][next][hasNext][next]2");
+    }
+
+    @Test
+    public void seqIndexOfTest() throws Exception {
+        assertOutput("${coll?seqIndexOf(2)}",
+                "[iterator][hasNext][next][hasNext][next]1");
+        assertOutput("${coll?filter(x -> x % 2 == 0)?seqIndexOf(2)}",
+                "[iterator][hasNext][next][hasNext][next]0");
+    }
+
+    @Test
+    public void filterTest() throws Exception {
+        assertOutput("${coll?filter(x -> x % 2 == 0)?filter(x -> true)?first}",
+                "[iterator][hasNext][next][hasNext][next]2");
+    }
+
+    @Test
+    public void listTest() throws Exception {
+        // Note: #list has to fetch elements up to 4 to know if it?hasNext
+        assertOutput("<#list collLong?filter(x -> x % 2 == 0) as it>${it} ${it?hasNext?c}<#break></#list>",
+                "[iterator][hasNext][next][hasNext][next][hasNext][next][hasNext][next]2 true");
+    }
+
+    @Override
+    protected Configuration createConfiguration() throws Exception {
+        Configuration cfg = super.createConfiguration();
+        cfg.setSharedVariable("seq", new MonitoredTemplateSequenceModel(1, 2, 3));
+        cfg.setSharedVariable("coll", new MonitoredTemplateCollectionModel(1, 2, 3));
+        cfg.setSharedVariable("collLong", new MonitoredTemplateCollectionModel(1, 2, 3, 4, 5, 6));
+        cfg.setSharedVariable("collEx", new MonitoredTemplateCollectionModelEx(1, 2, 3));
+        return cfg;
+    }
+
+    public static abstract class ListContainingTemplateModel {
+        protected final List<Number> elements;
+
+        public ListContainingTemplateModel(Number... elements) {
+            this.elements = new ArrayList<Number>(Arrays.asList(elements));
+        }
+
+        public int size() {
+            logCall("size");
+            return elements.size();
+        }
+
+        protected void logCall(String callDesc) {
+            try {
+                Environment.getCurrentEnvironment().getOut().write("[" + callDesc + "]");
+            } catch (IOException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+    }
+
+    public static class MonitoredTemplateSequenceModel extends ListContainingTemplateModel
+            implements TemplateSequenceModel {
+        public MonitoredTemplateSequenceModel(Number... elements) {
+            super(elements);
+        }
+
+        public TemplateModel get(int index) throws TemplateModelException {
+            logCall("get " + index);
+            return new SimpleNumber(elements.get(index));
+        }
+
+    }
+
+    public static class MonitoredTemplateCollectionModel extends ListContainingTemplateModel
+            implements TemplateCollectionModel {
+        public MonitoredTemplateCollectionModel(Number... elements) {
+            super(elements);
+        }
+
+        public TemplateModelIterator iterator() throws TemplateModelException {
+            logCall("iterator");
+            final Iterator<Number> iterator = elements.iterator();
+            return new TemplateModelIterator() {
+                public TemplateModel next() throws TemplateModelException {
+                    logCall("next");
+                    return new SimpleNumber(iterator.next());
+                }
+
+                public boolean hasNext() throws TemplateModelException {
+                    logCall("hasNext");
+                    return iterator.hasNext();
+                }
+            };
+        }
+    }
+
+    public static class MonitoredTemplateCollectionModelEx extends MonitoredTemplateCollectionModel
+            implements TemplateCollectionModelEx {
+        public MonitoredTemplateCollectionModelEx(Number... elements) {
+            super(elements);
+        }
+
+        public boolean isEmpty() throws TemplateModelException {
+            logCall("isEmpty");
+            return elements.isEmpty();
+        }
+    }
+
+}
diff --git a/src/test/java/freemarker/core/ListWithStreamLikeBuiltinsTest.java b/src/test/java/freemarker/core/ListWithStreamLikeBuiltinsTest.java
index 1438304..f025e9a 100644
--- a/src/test/java/freemarker/core/ListWithStreamLikeBuiltinsTest.java
+++ b/src/test/java/freemarker/core/ListWithStreamLikeBuiltinsTest.java
@@ -36,7 +36,7 @@ public class ListWithStreamLikeBuiltinsTest extends TemplateTest {
 
     @Test
     public void testLambdaScope() throws Exception {
-        // The loop variables aren't visible during the lazy processing done for the elements
+        // Loop variables aren't visible during the lazy result generation:
         assertOutput("<#list (1..3)?map(p -> p * 10 + it!'-') as it>${it}<#sep>, </#list>",
                 "10-, 20-, 30-");
         assertOutput("<#list (1..3)?map(p -> p * 10 + it_has_next!'-') as it>${it}<#sep>, </#list>",


[freemarker] 02/03: Made ?size smarter when its result is compared to an integer literal. Specifically, for ?filter-ed sequences (or for the result of similar future Stream-API-like built-ins) it won't count more elements then it's necessary to tell the comparison result. Also, for a TemplateCollectionModelEx, it calls isEmpty() instead of size(), if for the comparison result we only have to know if we have more than 0 elements.

Posted by dd...@apache.org.
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 e7d91e63596c97a59568af78467f1854b3b11a2f
Author: ddekany <dd...@apache.org>
AuthorDate: Sun Mar 31 18:17:04 2019 +0200

    Made ?size smarter when its result is compared to an integer literal. Specifically, for ?filter-ed sequences (or for the result of similar future Stream-API-like built-ins) it won't count more elements then it's necessary to tell the comparison result. Also, for a TemplateCollectionModelEx, it calls isEmpty() instead of size(), if for the comparison result we only have to know if we have more than 0 elements.
---
 src/main/java/freemarker/core/BuiltIn.java         |  5 +-
 .../freemarker/core/BuiltInsForMultipleTypes.java  | 37 +++++++++++-
 .../java/freemarker/core/ComparisonExpression.java | 16 +++++
 src/main/java/freemarker/core/EvalUtil.java        | 12 ++++
 src/main/java/freemarker/core/IteratorBlock.java   |  6 +-
 src/main/java/freemarker/core/MiscUtil.java        |  7 +++
 src/manual/en_US/book.xml                          | 15 +++++
 ...ilyGeneratedSeqTargetSupportInBuiltinsTest.java | 69 +++++++++++++++++++++-
 8 files changed, 158 insertions(+), 9 deletions(-)

diff --git a/src/main/java/freemarker/core/BuiltIn.java b/src/main/java/freemarker/core/BuiltIn.java
index 0679e21..9e3dd3c 100644
--- a/src/main/java/freemarker/core/BuiltIn.java
+++ b/src/main/java/freemarker/core/BuiltIn.java
@@ -388,8 +388,9 @@ abstract class BuiltIn extends Expression implements Cloneable {
         bi.key = key;
         bi.target = target;
         if (bi.isLazilyGeneratedSequenceModelTargetSupported()) {
-            if (target instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
-                ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) target)
+            Expression cleanedTarget = MiscUtil.peelParentheses(target);
+            if (cleanedTarget instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
+                ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) cleanedTarget)
                         .setLazyResultGenerationAllowed(true);
             }
         }
diff --git a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
index 61f8f3f..b690cf5 100644
--- a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
+++ b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
@@ -48,6 +48,7 @@ import freemarker.template.TemplateScalarModel;
 import freemarker.template.TemplateSequenceModel;
 import freemarker.template.TemplateTransformModel;
 import freemarker.template._TemplateAPI;
+import freemarker.template.utility.NumberUtil;
 
 /**
  * A holder for builtins that didn't fit into any other category.
@@ -488,12 +489,16 @@ class BuiltInsForMultipleTypes {
             return true;
         }
 
+        private int countingLimit;
+
         @Override
         TemplateModel _eval(Environment env) throws TemplateException {
             TemplateModel model = target.eval(env);
 
             final int size;
-            if (model instanceof TemplateSequenceModel) {
+            if (countingLimit == 1 && model instanceof TemplateCollectionModelEx) {
+                size = ((TemplateCollectionModelEx) model).isEmpty() ? 0 : 1;
+            } else if (model instanceof TemplateSequenceModel) {
                 size = ((TemplateSequenceModel) model).size();
             } else if (model instanceof TemplateCollectionModelEx) {
                 size = ((TemplateCollectionModelEx) model).size();
@@ -508,8 +513,11 @@ class BuiltInsForMultipleTypes {
                 // expected by the template author, and we just made that calculation less wasteful here.
                 TemplateModelIterator iterator = ((LazilyGeneratedSequenceModel) model).iterator();
                 int counter = 0;
-                while (iterator.hasNext()) {
+                countElements: while (iterator.hasNext()) {
                     counter++;
+                    if (counter == countingLimit) {
+                        break countElements;
+                    }
                     iterator.next();
                 }
                 size = counter;
@@ -526,6 +534,31 @@ class BuiltInsForMultipleTypes {
             }
             return new SimpleNumber(size);
         }
+
+        /**
+         * Enables an optimization trick when the result of the built-in will be compared with a number literal.
+         * For example, in the case of {@code things?size != 0} we only need to check of the target is non-empty, which
+         * is often more efficient than telling exactly how many elements it has.
+         */
+        void setCountingLimit(int cmpOperator, NumberLiteral rightOperand) {
+            int cmpInt;
+            try {
+                cmpInt = NumberUtil.toIntExact(rightOperand.getAsNumber());
+            } catch (ArithmeticException e) {
+                // As we can't know what the ArithmeticEngine will be on runtime, we won't risk this for non-integers.
+                return;
+            }
+
+            switch (cmpOperator) {
+                case EvalUtil.CMP_OP_EQUALS: countingLimit = cmpInt + 1; break;
+                case EvalUtil.CMP_OP_NOT_EQUALS: countingLimit = cmpInt + 1; break;
+                case EvalUtil.CMP_OP_LESS_THAN: countingLimit = cmpInt; break;
+                case EvalUtil.CMP_OP_GREATER_THAN: countingLimit = cmpInt + 1; break;
+                case EvalUtil.CMP_OP_LESS_THAN_EQUALS: countingLimit = cmpInt + 1; break;
+                case EvalUtil.CMP_OP_GREATER_THAN_EQUALS: countingLimit = cmpInt; break;
+                default: throw new BugException("Unsupported comparator operator code: " + cmpOperator);
+            }
+        }
     }
     
     static class stringBI extends BuiltIn {
diff --git a/src/main/java/freemarker/core/ComparisonExpression.java b/src/main/java/freemarker/core/ComparisonExpression.java
index f647ce4..b33c5bc 100644
--- a/src/main/java/freemarker/core/ComparisonExpression.java
+++ b/src/main/java/freemarker/core/ComparisonExpression.java
@@ -51,6 +51,22 @@ final class ComparisonExpression extends BooleanExpression {
         } else {
             throw new BugException("Unknown comparison operator " + opString);
         }
+
+        Expression cleanedLeft = MiscUtil.peelParentheses(left);
+        Expression cleanedRight = MiscUtil.peelParentheses(right);
+        if (cleanedLeft instanceof BuiltInsForMultipleTypes.sizeBI) {
+            if (cleanedRight instanceof NumberLiteral) {
+                ((BuiltInsForMultipleTypes.sizeBI) cleanedLeft).setCountingLimit(
+                        operation, (NumberLiteral) cleanedRight
+                );
+            }
+        } else if (cleanedRight instanceof BuiltInsForMultipleTypes.sizeBI) {
+            if (cleanedLeft instanceof NumberLiteral) {
+                ((BuiltInsForMultipleTypes.sizeBI) cleanedRight).setCountingLimit(
+                        EvalUtil.mirrorCmpOperator(operation), (NumberLiteral) cleanedLeft
+                );
+            }
+        }
     }
 
     /*
diff --git a/src/main/java/freemarker/core/EvalUtil.java b/src/main/java/freemarker/core/EvalUtil.java
index ebde9c3..0fa7a48 100644
--- a/src/main/java/freemarker/core/EvalUtil.java
+++ b/src/main/java/freemarker/core/EvalUtil.java
@@ -343,6 +343,18 @@ class EvalUtil {
         }
     }
 
+    static int mirrorCmpOperator(int operator) {
+        switch (operator) {
+            case CMP_OP_EQUALS: return CMP_OP_EQUALS;
+            case CMP_OP_NOT_EQUALS: return CMP_OP_NOT_EQUALS;
+            case CMP_OP_LESS_THAN: return CMP_OP_GREATER_THAN;
+            case CMP_OP_GREATER_THAN: return CMP_OP_LESS_THAN;
+            case CMP_OP_LESS_THAN_EQUALS: return CMP_OP_GREATER_THAN_EQUALS;
+            case CMP_OP_GREATER_THAN_EQUALS: return CMP_OP_LESS_THAN_EQUALS;
+            default: throw new BugException("Unsupported comparator operator code: " + operator);
+        }
+    }
+
     /**
      * Converts a value to plain text {@link String}, or a {@link TemplateMarkupOutputModel} if that's what the
      * {@link TemplateValueFormat} involved produces.
diff --git a/src/main/java/freemarker/core/IteratorBlock.java b/src/main/java/freemarker/core/IteratorBlock.java
index a51401a..c734cbe 100644
--- a/src/main/java/freemarker/core/IteratorBlock.java
+++ b/src/main/java/freemarker/core/IteratorBlock.java
@@ -83,8 +83,10 @@ final class IteratorBlock extends TemplateElement {
         this.hashListing = hashListing;
         this.forEach = forEach;
 
-        if (listedExp instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
-            ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) listedExp).setLazyResultGenerationAllowed(true);
+        Expression cleanedListExp = MiscUtil.peelParentheses(listedExp);
+        if (cleanedListExp instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
+            ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) cleanedListExp)
+                    .setLazyResultGenerationAllowed(true);
             fetchElementsOutsideLoopVarContext = true;
         } else {
             fetchElementsOutsideLoopVarContext = false;
diff --git a/src/main/java/freemarker/core/MiscUtil.java b/src/main/java/freemarker/core/MiscUtil.java
index 770e394..505edb4 100644
--- a/src/main/java/freemarker/core/MiscUtil.java
+++ b/src/main/java/freemarker/core/MiscUtil.java
@@ -64,5 +64,12 @@ class MiscUtil {
         });
         return res;
     }
+
+    static Expression peelParentheses(Expression exp) {
+        while (exp instanceof ParentheticalExpression) {
+            exp = ((ParentheticalExpression) exp).getNestedExpression();
+        }
+        return exp;
+    }
     
 }
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index 0633247..24ceada 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -27963,6 +27963,21 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
             </listitem>
 
             <listitem>
+              <para>If the result of
+              <literal><replaceable>seq</replaceable>?size</literal> is
+              compared to an integer literal in a template, like in
+              <literal><replaceable>seq</replaceable>?size != 0</literal>, or
+              <literal><replaceable>seq</replaceable>?size &lt; 1</literal>,
+              and to decide the answer it's enough to know if
+              <literal><replaceable>seq</replaceable></literal> is an empty
+              sequence or collection (i.e., the exact size isn't needed), and
+              said value implements
+              <literal>TemplateCollectionModelEx</literal>, FreeMarker will
+              call <literal>TemplateCollectionModelEx.isEmpty()</literal>
+              instead of <literal>size()</literal>.</para>
+            </listitem>
+
+            <listitem>
               <para>Added
               <literal>TemplateModelUtils.wrapAsHashUnion(ObjectWrapper,
               List&lt;?&gt;)</literal> and
diff --git a/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java b/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java
index 37cefa8..36af95f 100644
--- a/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java
+++ b/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java
@@ -43,7 +43,7 @@ import freemarker.test.TemplateTest;
 public class LazilyGeneratedSeqTargetSupportInBuiltinsTest extends TemplateTest {
 
     @Test
-    public void sizeTest() throws Exception {
+    public void sizeBasicsTest() throws Exception {
         assertOutput("${seq?size}",
                 "[size]3");
         assertOutput("${collEx?size}",
@@ -63,6 +63,56 @@ public class LazilyGeneratedSeqTargetSupportInBuiltinsTest extends TemplateTest
     }
 
     @Test
+    public void sizeComparisonTest() throws Exception {
+        // These actually aren't related to lazy generation...
+        assertOutput("${collEx?size}",
+                "[size]3");
+        assertOutput("${collEx?size != 0}",
+                "[isEmpty]true");
+        assertOutput("${0 != collEx?size}",
+                "[isEmpty]true");
+        assertOutput("${collEx?size == 0}",
+                "[isEmpty]false");
+        assertOutput("${0 == collEx?size}",
+                "[isEmpty]false");
+        assertOutput("${(collEx?size >= 1)}",
+                "[isEmpty]true");
+        assertOutput("${1 <= collEx?size}",
+                "[isEmpty]true");
+        assertOutput("${collEx?size <= 0}",
+                "[isEmpty]false");
+        assertOutput("${(0 >= collEx?size)}",
+                "[isEmpty]false");
+        assertOutput("${collEx?size > 0}",
+                "[isEmpty]true");
+        assertOutput("${0 < collEx?size}",
+                "[isEmpty]true");
+        assertOutput("${collEx?size < 1}",
+                "[isEmpty]false");
+        assertOutput("${1 > collEx?size}",
+                "[isEmpty]false");
+        assertOutput("${collEx?size == 1}",
+                "[size]false");
+        assertOutput("${1 == collEx?size}",
+                "[size]false");
+
+        // Now the lazy generation things:
+        assertOutput("${collLong?filter(x -> true)?size}",
+                "[iterator]" +
+                        "[hasNext][next][hasNext][next][hasNext][next]" +
+                        "[hasNext][next][hasNext][next][hasNext][next][hasNext]6");
+        // Note: "[next]" is added by ?filter, as it has to know if the element matches the predicate.
+        assertOutput("${collLong?filter(x -> true)?size != 0}",
+                "[iterator][hasNext][next]true");
+        assertOutput("${collLong?filter(x -> true)?size != 1}",
+                "[iterator][hasNext][next][hasNext][next]true");
+        assertOutput("${collLong?filter(x -> true)?size == 1}",
+                "[iterator][hasNext][next][hasNext][next]false");
+        assertOutput("${collLong?filter(x -> true)?size < 3}",
+                "[iterator][hasNext][next][hasNext][next][hasNext][next]false");
+    }
+
+    @Test
     public void firstTest() throws Exception {
         assertOutput("${coll?first}",
                 "[iterator][hasNext][next]1");
@@ -87,13 +137,26 @@ public class LazilyGeneratedSeqTargetSupportInBuiltinsTest extends TemplateTest
     @Test
     public void listTest() throws Exception {
         // Note: #list has to fetch elements up to 4 to know if it?hasNext
-        assertOutput("<#list collLong?filter(x -> x % 2 == 0) as it>${it} ${it?hasNext?c}<#break></#list>",
-                "[iterator][hasNext][next][hasNext][next][hasNext][next][hasNext][next]2 true");
+        String commonSourceExp = "collLong?filter(x -> x % 2 == 0)";
+        for (String sourceExp : new String[] {commonSourceExp, "(" + commonSourceExp + ")"}) {
+            assertOutput("<#list " + sourceExp + " as it>${it} ${it?hasNext}<#break></#list>",
+                    "[iterator][hasNext][next][hasNext][next][hasNext][next][hasNext][next]2 true");
+        }
     }
 
+    @Test
+    public void biTargetParenthesisTest() throws Exception {
+        assertOutput("${(coll?filter(x -> x % 2 == 0))?first}",
+                "[iterator][hasNext][next][hasNext][next]2");
+    }
+
+
+
     @Override
     protected Configuration createConfiguration() throws Exception {
         Configuration cfg = super.createConfiguration();
+        cfg.setIncompatibleImprovements(Configuration.VERSION_2_3_29);
+        cfg.setBooleanFormat("c");
         cfg.setSharedVariable("seq", new MonitoredTemplateSequenceModel(1, 2, 3));
         cfg.setSharedVariable("coll", new MonitoredTemplateCollectionModel(1, 2, 3));
         cfg.setSharedVariable("collLong", new MonitoredTemplateCollectionModel(1, 2, 3, 4, 5, 6));


[freemarker] 03/03: (Added some version history entry for ?filter/?map and local lambdas... documentation is still missing)

Posted by dd...@apache.org.
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 700e87ec201f5251ac4016371a4de3438d2e125e
Author: ddekany <dd...@apache.org>
AuthorDate: Mon Apr 1 00:33:50 2019 +0200

    (Added some version history entry for ?filter/?map and local lambdas... documentation is still missing)
---
 src/manual/en_US/book.xml | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index 24ceada..22ba798 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -27899,6 +27899,25 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
 
           <itemizedlist>
             <listitem>
+              <para>Added new built-ins:
+              <literal>?filter(<replaceable>predicate</replaceable>)</literal>
+              and <literal>?map(<replaceable>mapper</replaceable>)</literal>.
+              These allow using lambda expression, like
+              <literal>users?filter(user -&gt; user.superuser)</literal> or
+              <literal>users?map(user -&gt; user.name)</literal>, or accept a
+              functions/method as parameter. (Lambda expressions are also new
+              in this release, but they can only be used in said built-ins, so
+              they aren't like in Java for example.) These built-ins are
+              generally eager, that is, they immediately build a new sequence.
+              However at selected places, most notably when used as
+              <literal>&lt;#list <replaceable>...</replaceable>&gt;</literal>
+              directive parameter, they are working in lazy mode instead, that
+              is, they won't built the a new sequence, just stream through the
+              existing one and apply the filter or mapping element by element.
+              [TODO: document and link documentation]</para>
+            </listitem>
+
+            <listitem>
               <para>Added new built-ins for truncating text.
               <literal><replaceable>string</replaceable>?truncate(<replaceable>length</replaceable>)</literal>
               truncates the text to the given length, and by default adds