Train: #728 (parity batch 2 group A) on master a074f49

CMakeLists.txt

@@ -231,6 +231,7 @@ set(JINJA2CPP_SOURCES
     src/filesystem_handler.cpp
     src/filter_factories.cpp
     src/filters.cpp
+    src/function_base.cpp
     src/json_reader.cpp
     src/generic_list.cpp
     src/global_functions.cpp

docs/tasks/0306-filter-test-argument-unpacking.md

@@ -1,5 +1,5 @@
 ---
-status: open
+status: in-progress
 priority: low
 area: parity
 depends: [0220]

docs/tasks/0308-divisibleby-non-number.md

@@ -1,5 +1,5 @@
 ---
-status: open
+status: in-progress
 priority: low
 area: parity
 depends: []

docs/tasks/0310-undefined-filter-argument-is-omitted.md

@@ -1,5 +1,5 @@
 ---
-status: open
+status: in-progress
 priority: low
 area: parity
 touches: [src/function_base.h, src/string_converter_filter.cpp]

src/expression_evaluator.h

@@ -118,12 +118,10 @@ struct CallParamsInfo
     std::vector<NodeRef<Expression>> posParams;
 };
 
-// The arguments of a call as parsed: f(*args, **kwargs) is unpacked when the call is evaluated,
-// null when not written. Only calls take them, so filters and tests keep CallParamsInfo
-struct CallArgsInfo
+// The arguments of a call, a filter or a test as parsed: f(*args, **kwargs) is unpacked when
+// the call is evaluated, null when not written. A call without them is its CallParamsInfo
+struct CallArgsInfo : CallParamsInfo
 {
-    OrderedMap<std::string, NodeRef<Expression>> kwParams;
-    std::vector<NodeRef<Expression>> posParams;
     NodeRef<Expression> dynArgs;
     NodeRef<Expression> dynKwargs;
 };

src/expression_parser.cpp

@@ -3,6 +3,7 @@
 #include "engine_errors.h"
 #include "error_handling.h"
 #include "expression_evaluator.h"
+#include "filters.h"
 #include "internal_value.h"
 #include "lexer.h"
 #include "loop_attr.h"
@@ -13,6 +14,7 @@
 #include "render_context.h"
 #include "renderer.h"
 #include "small_vector.h"
+#include "testers.h"
 #include "value_visitors.h"
 
 #include <jinja2cpp/error_info.h>
@@ -709,11 +711,11 @@ ExpressionParser::ParseResult<NodeRef<Expression>> ExpressionParser::ParseTest(L
         return MakeParseError(ErrorCode::ExpectedIdentifier, nameTok);
     }
 
-    CallParamsInfo params;
+    CallArgsInfo params;
     const auto& argTok = lexer.PeekNextToken();
     if (lexer.EatIfEqual('('))
     {
-        auto parsedParams = ParseCallParams(lexer);
+        auto parsedParams = ParseCallArgs(lexer);
         if (!parsedParams)
         {
             return MakeUnexpected(parsedParams.error());
@@ -763,7 +765,16 @@ ExpressionParser::ParseResult<NodeRef<Expression>> ExpressionParser::ParseTest(L
     try
     {
         bool unknown = false;
-        result = IsExpression::Make(m_nodes, valueRef, name, params, FindRegisteredTester(name), &unknown, !m_volatile);
+        if (params.dynArgs || params.dynKwargs)
+        {
+            // Bound to the test each time it runs (docs/tasks/0306)
+            const auto test = CreateUnpackingTester(m_nodes, name, params, FindRegisteredTester(name), &unknown);
+            result = m_nodes.Make<IsExpression>(valueRef, test, false);
+        }
+        else
+        {
+            result = IsExpression::Make(m_nodes, valueRef, name, params, FindRegisteredTester(name), &unknown, !m_volatile);
+        }
         if (unknown)
         {
             m_unknown.Add(nameTok, true);
@@ -957,21 +968,11 @@ std::optional<CallArgKind> ClassifyUnpacked(const Token& tok, const CallArgsInfo
     return params.dynKwargs || params.dynArgs ? std::nullopt : std::optional(CallArgKind::Args);
 }
 
-bool HasUnpacked(const CallParamsInfo& /*params*/)
-{
-    return false;
-}
-
 bool HasUnpacked(const CallArgsInfo& params)
 {
     return params.dynArgs || params.dynKwargs;
 }
 
-bool HasUnpackedKeywords(const CallParamsInfo& /*params*/)
-{
-    return false;
-}
-
 bool HasUnpackedKeywords(const CallArgsInfo& params)
 {
     return static_cast<bool>(params.dynKwargs);
@@ -981,15 +982,11 @@ bool HasUnpackedKeywords(const CallArgsInfo& params)
 // arguments, then keywords and one `*expr`, then one `**expr`. Nothing where it is out of that
 // order ("invalid syntax for function call expression") or a keyword is repeated; a keyword's
 // name goes to `name`
-template<typename Params>
-std::optional<CallArgKind> ClassifyCallArg(const Token& tok, LexScanner& lexer, const Params& params, std::string& name)
+std::optional<CallArgKind> ClassifyCallArg(const Token& tok, LexScanner& lexer, const CallArgsInfo& params, std::string& name)
 {
-    if constexpr (std::is_same_v<Params, CallArgsInfo>)
+    if (tok == '*' || tok == Token::MulMul)
     {
-        if (tok == '*' || tok == Token::MulMul)
-        {
-            return ClassifyUnpacked(tok, params);
-        }
+        return ClassifyUnpacked(tok, params);
     }
     if (tok == Token::Identifier && lexer.PeekNextToken() == '=')
     {
@@ -1001,20 +998,10 @@ std::optional<CallArgKind> ClassifyCallArg(const Token& tok, LexScanner& lexer,
 }
 } // namespace
 
-ExpressionParser::ParseResult<CallParamsInfo> ExpressionParser::ParseCallParams(LexScanner& lexer)
-{
-    return ParseArguments<CallParamsInfo>(lexer);
-}
-
+// The arguments of a call, a filter or a test, `*args` and `**kwargs` included
 ExpressionParser::ParseResult<CallArgsInfo> ExpressionParser::ParseCallArgs(LexScanner& lexer)
 {
-    return ParseArguments<CallArgsInfo>(lexer);
-}
-
-template<typename Params>
-ExpressionParser::ParseResult<Params> ExpressionParser::ParseArguments(LexScanner& lexer)
-{
-    Params result;
+    CallArgsInfo result;
 
     if (lexer.EatIfEqual(')'))
     {
@@ -1054,7 +1041,7 @@ ExpressionParser::ParseResult<Params> ExpressionParser::ParseArguments(LexScanne
         {
             result.kwParams[paramName] = *valueExpr;
         }
-        else if constexpr (std::is_same_v<Params, CallArgsInfo>)
+        else
         {
             (*kind == CallArgKind::Args ? result.dynArgs : result.dynKwargs) = *valueExpr;
         }
@@ -1299,24 +1286,31 @@ ExpressionParser::ParseResult<NodeRef<ExpressionFilter>> ExpressionParser::Parse
             }
 
             std::string name = lexer.GetAsString(tok);
-            ParseResult<CallParamsInfo> params;
-
-            if (lexer.NextToken() == '(')
-            {
-                params = ParseCallParams(lexer);
-            }
-            else
+            const bool hasArgs = lexer.NextToken() == '(';
+            if (!hasArgs)
             {
                 lexer.ReturnToken();
             }
+            // Made in place: assigning an expected swaps the arguments
+            auto params = hasArgs ? ParseCallArgs(lexer) : ParseResult<CallArgsInfo>();
 
             if (!params)
             {
                 return MakeUnexpected(params.error());
             }
 
             bool unknown = false;
-            auto filter = ExpressionFilter::Make(m_nodes, name, *params, FindRegisteredFilter(name), &unknown, !m_volatile);
+            NodeRef<ExpressionFilter> filter;
+            if (params->dynArgs || params->dynKwargs)
+            {
+                // Bound to the filter each time it runs (docs/tasks/0306); such a call is not folded
+                const auto unpacking = CreateUnpackingFilter(m_nodes, name, *params, FindRegisteredFilter(name), &unknown);
+                filter = m_nodes.Make<ExpressionFilter>(unpacking, ArenaText(), false);
+            }
+            else
+            {
+                filter = ExpressionFilter::Make(m_nodes, name, *params, FindRegisteredFilter(name), &unknown, !m_volatile);
+            }
             if (unknown)
             {
                 m_unknown.Add(tok, false);

src/expression_parser.h

@@ -150,10 +150,7 @@ class ExpressionParser
     ParseResult<NodeRef<Expression>> ParseFullExpression(LexScanner& lexer, bool includeIfPart = true);
     // Jinja2's parse_tuple without parentheses: 'a, b' is a tuple, 'a' stays an expression
     ParseResult<NodeRef<Expression>> ParseTupleOrExpression(LexScanner& lexer, bool includeIfPart = true);
-    // The arguments of a filter or test, which bind them at Load and so take no `*args` or
-    // `**kwargs` yet (docs/tasks/0306)
-    ParseResult<CallParamsInfo> ParseCallParams(LexScanner& lexer);
-    // The arguments of a call, `*args` and `**kwargs` included
+    // The arguments of a call, a filter or a test, `*args` and `**kwargs` included
     ParseResult<CallArgsInfo> ParseCallArgs(LexScanner& lexer);
     ParseResult<NodeRef<ExpressionFilter>> ParseFilterExpression(LexScanner& lexer);
     // Settings::finalize as a callable; undefined if it is not set
@@ -196,8 +193,6 @@ class ExpressionParser
     ParseResult<NodeRef<Expression>> ParseBracedExpressionOrTuple(LexScanner& lexer);
     ParseResult<NodeRef<Expression>> ParseDictionary(LexScanner& lexer);
     ParseResult<NodeRef<Expression>> ParseTuple(LexScanner& lexer);
-    template<typename Params>
-    ParseResult<Params> ParseArguments(LexScanner& lexer);
     ParseResult<NodeRef<Expression>> ParseCall(LexScanner& lexer, NodeRef<Expression> valueRef);
     ParseResult<NodeRef<Expression>> ParseSubscript(LexScanner& lexer, NodeRef<Expression> valueRef);
     NodeRef<SubscriptExpression> MakeSubscript(NodeRef<Expression> value, const std::string& attrName);

src/filter_factories.cpp

@@ -4,6 +4,8 @@
 #include "filters.h"
 
 #include "expression_evaluator.h"
+#include "engine_errors.h"
+#include "function_base.h"
 #include "internal_value.h"
 #include "node_arena.h"
 #include "render_context.h"
@@ -13,10 +15,12 @@
 #include <jinja2cpp/value.h>
 
 #include <algorithm>
+#include <cstddef>
 #include <memory>
 #include <optional>
 #include <string>
 #include <string_view>
+#include <type_traits>
 #include <utility>
 #include <vector>
 
@@ -131,7 +135,7 @@ const FilterEntry* FindBuiltinFilter(std::string_view filterName)
 }
 } // namespace
 
-FilterPtr CreateFilter(std::string_view filterName, const CallParamsInfo& params)
+FilterPtr CreateFilter(std::string_view filterName, FunctionArgs params)
 {
     if (const auto* entry = FindBuiltinFilter(filterName))
     {
@@ -140,7 +144,7 @@ FilterPtr CreateFilter(std::string_view filterName, const CallParamsInfo& params
     return std::make_shared<filters::UserDefinedFilter>(std::string(filterName), params);
 }
 
-NodeRef<ExpressionFilter::IExpressionFilter> CreateFilter(NodeArena& nodes, std::string_view filterName, const CallParamsInfo& params, bool* unknown)
+NodeRef<ExpressionFilter::IExpressionFilter> CreateFilter(NodeArena& nodes, std::string_view filterName, FunctionArgs params, bool* unknown)
 {
     if (const auto* entry = FindBuiltinFilter(filterName))
     {
@@ -153,7 +157,7 @@ NodeRef<ExpressionFilter::IExpressionFilter> CreateFilter(NodeArena& nodes, std:
     return nodes.MakeObject<IExpressionFilter, filters::UserDefinedFilter>(std::string(filterName), params);
 }
 
-FilterPtr CreateFilter(const std::string& filterName, const CallParamsInfo& params, RenderContext& context)
+FilterPtr CreateFilter(const std::string& filterName, FunctionArgs params, RenderContext& context)
 {
     auto* env = context.GetEnv();
     auto registered = env ? env->FindFilter(filterName) : std::optional<UserCallable>();
@@ -165,5 +169,103 @@ FilterPtr CreateFilter(const std::string& filterName, const CallParamsInfo& para
     return std::make_shared<filters::UserDefinedFilter>(filterName, params, std::move(callable));
 }
 
+NodeRef<ExpressionFilter::IExpressionFilter> CreateUnpackingFilter(NodeArena& nodes, const std::string& filterName, const CallArgsInfo& params, InternalValue registered, bool* unknown)
+{
+    if (unknown && !GetIf<Callable>(&registered) && !FindBuiltinFilter(filterName))
+    {
+        *unknown = true;
+    }
+    // The arguments first: an arena makes one object at a time
+    const auto args = ArenaCallParams::Make(nodes, params);
+    return nodes.MakeObject<IExpressionFilter, filters::UnpackingFilter>(filterName, args, std::move(registered));
+}
+
+namespace filters
+{
+// The constructors of the filters that forward their extra arguments are templates over the
+// expressions and the values of a call (FunctionArgs::Visit). Here, out of filters.cpp: there they
+// changed GCC's inlining of the filters' render paths enough to cost up to 1% (docs/tasks/0306)
+Map::Map(const FilterParams& params)
+{
+    // map(attribute='x', default=d) reads the attribute of each item; it binds no filter
+    const bool byAttribute = params.Visit([this](const auto& args) {
+        const auto attributeIt = args.kwParams.find("attribute");
+        if (!args.posParams.empty() || attributeIt == args.kwParams.end())
+        {
+            return false;
+        }
+        std::decay_t<decltype(args)> mapping;
+        mapping.kwParams["name"] = attributeIt->second;
+        for (const auto& [name, value] : args.kwParams)
+        {
+            if (name == "default")
+            {
+                mapping.kwParams["default"] = value;
+            }
+            else if (name != "attribute")
+            {
+                // Jinja2's prepare_map raises FilterArgumentError
+                SetArgumentsError("got an unexpected keyword argument '" + name + "'");
+                break;
+            }
+        }
+        m_mappingParams = ForwardedArgs(std::move(mapping));
+        return true;
+    });
+    if (byAttribute)
+    {
+        m_byAttribute = true;
+        return;
+    }
+
+    static const auto args = MakeArgumentsTable({ { "filter", true } });
+    ParseParams(args, params, ExtraArgs::Accept, &m_mappingParams);
+}
+
+Tester::Tester(const FilterParams& params, Tester::Mode mode)
+    : m_mode(mode)
+{
+    // Python's select_or_reject takes the attribute and the test name by position and passes
+    // the other arguments, keywords included, to the test; without a test it calls bool() and
+    // drops them
+    const bool isAttrMode = mode == SelectAttrMode || mode == RejectAttrMode;
+    const std::size_t leadCount = std::min(params.PosCount(), isAttrMode ? std::size_t{ 2 } : std::size_t{ 1 });
+    m_hasAttribute = isAttrMode && leadCount > 0;
+
+    params.Visit([&](const auto& args) {
+        const auto split = args.posParams.begin() + static_cast<std::ptrdiff_t>(leadCount);
+        std::decay_t<decltype(args)> leadParams;
+        leadParams.posParams.assign(args.posParams.begin(), split);
+        if (isAttrMode)
+        {
+            static const auto table = MakeArgumentsTable({ { "attribute", false }, { "tester", false } });
+            ParseParams(table, leadParams);
+        }
+        else
+        {
+            static const auto table = MakeArgumentsTable({ { "tester", false } });
+            ParseParams(table, leadParams);
+        }
+
+        std::decay_t<decltype(args)> testingParams;
+        testingParams.posParams.assign(split, args.posParams.end());
+        testingParams.kwParams = args.kwParams;
+        m_testingParams = ForwardedArgs(std::move(testingParams));
+    });
+}
+
+InternalValue UnpackingFilter::Filter(const InternalValue& baseVal, RenderContext& context)
+{
+    const auto args = helpers::EvaluateCallParams(m_params, context);
+    // Filters added to the environment take precedence over the builtins (ExpressionFilter::Make)
+    const FilterPtr filter = GetIf<Callable>(&m_registered) ? std::make_shared<UserDefinedFilter>(m_filterName, args, m_registered) : CreateFilter(m_filterName, args);
+    if (auto error = filter->GetArgumentsError(); !error.empty())
+    {
+        throw TemplateRuntimeError(m_filterName + "() " + error);
+    }
+    return filter->Filter(baseVal, context);
+}
+} // namespace filters
+
 } // namespace v2
 } // namespace jinja2