Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/tasks/0305-builtin-class-repr.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,12 @@ not an attribute, so `range.name` stays undefined), print it in `ValueRendererBa
it in `xmlattr`.

**Done when.** No line of `test/parity/divergences/` names 0305.

**Done.** `Callable::SetRepr` gives range, dict, cycler, joiner and namespace their class text
(global_functions.cpp `MakeClass`); `ValueRendererBase` prints it, and so do `xmlattr`,
`pprint`, `~`, `%` and `format`, which print through it. `loop.cycle` and `loop.changed` print
as bound methods through `Callable::SetBoundMethod(self, method, receiver)`: the receiver is the
`LoopCycleCell` the callables share with their loop (statements.cpp), which reads the live loop
while its frame holds it and keeps the last position and length after, so a kept callable does
not keep the loop (that would collect a filtered loop's rest at its end). The length after a
`break` out of a lazily filtered loop is unknown then: 0366.
10 changes: 10 additions & 0 deletions docs/tasks/0309-repr-of-template-modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,13 @@ and repr() is `<TemplateModule 'name'>` (see 0284 for the module object).

**Done when.** `filters.pprint_global_and_module` matches and its line is gone from
`test/parity/divergences/filters.txt`.

**Done.** `pprint` prints a global's class through 0305's `Callable::SetRepr`. `import ... as m`
binds a `ModuleMapAdapter` (internal_value.cpp, `CreateModuleAdapter`, made by
`ImportedModule::MakeObject`): an object of class `TemplateModule` whose repr argument is the
template's name and whose `IMapAccessor::GetStrValue` is the output of the module's top level,
which `ImportedModule::Render` now keeps. `{{ m }}` prints that output, unescaped under
autoescape as Jinja2's `__html__` gives it (markup.h); `pprint`, `[m]` and `string` give
`<TemplateModule 'lib'>`, and being an object, `m|length`, `for k in m`, `m|list`, `m.items()`,
`m|dictsort` and `m|tojson` fail as in Jinja2 (loader `module_object_*` cases). `m is escaped`
stays False: 0367.
25 changes: 25 additions & 0 deletions docs/tasks/0366-bound-loop-method-repr-after-break.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
status: open
priority: low
area: parity
depends: [0305]
touches: [src/statements.cpp#LoopCycleCell]
---
# A kept `loop.cycle` prints `?` for the length of a lazily filtered loop it broke out of

**Problem.** Found doing 0305. A `loop.cycle` or `loop.changed` callable prints as Jinja2's bound
method, `<bound method LoopContext.cycle of <LoopContext 1/2>>`. The callables share a cell with
their loop (`LoopCycleCell`), not the loop itself: a callable that kept the loop would make the
loop collect the rest of its items when it ends (the `use_count` check in `ForStatement`), which
runs the loop filter for items Jinja2 never filters (`statements.loop_cycle_stored_filtered_break`
raises in the filter then). So a callable kept past a `break` out of a loop whose length was not
known yet, a lazily filtered one (`for x in l if ...`), has no items left to count, and prints the
length as `?`. Jinja2 counts them when the repr is taken, which also runs the filter then (and
raises there if the filter does). Pinned by `statements.loop_cycle_repr_after_break_lazy`.

**Proposal.** Keep what the loop needs to fetch its rest (its enumerator over a list the cell
keeps alive, and the filter's expression with a context of its own) in the cell when a callable
shares it at the loop's end, and collect the rest on the first repr. Only worth it if a template
in the wild prints such a callable; otherwise mark the line deliberate.

**Done when** the line is gone from `test/parity/divergences/statements.txt` or marked deliberate.
26 changes: 26 additions & 0 deletions docs/tasks/0367-module-object-is-escaped.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
status: open
priority: low
area: parity
depends: [0309]
touches: [src/testers.cpp#IsEscapedMode, src/filters.cpp#Join, src/serialize_filters.cpp#XmlAttrFilter]
---
# `m is escaped` of an imported module

**Problem.** Found doing 0309. `{% import 'lib' as m %}` binds Jinja2's `TemplateModule`, whose
`__html__` gives its output as Markup. Since 0309 the module object prints that way under
autoescape (markup.h, `IMapAccessor::GetStrValue`), but the `escaped` test (testers.cpp,
`IsEscapedMode`) asks only Markup strings and host objects, so `{{ m is escaped }}` is False where
Jinja2 says True. Pinned by `loader.module_object_is_escaped`. testers.cpp belonged to parity
batch 2 group A when this was found, so it was not fixed in 0309's PR.

Two more sites take str() of the module where Jinja2 takes `__html__` under autoescape (found by
the 0309 verifier, no corpus case yet): `{{ [m]|join }}` escapes the output twice
(`&amp;lt;i&amp;gt;`, Jinja2 `&lt;i&gt;`), and `{{ dict(a=m)|xmlattr }}` escapes it once
(`a="A&lt;b&gt;B"`, Jinja2 `a="A<b>B"`).

**Proposal.** In `IsEscapedMode`, and in join's and xmlattr's Markup checks, treat a map whose
`GetStrValue()` is set as having `__html__`, as `host::IsMarkupLike` does for host objects; add a
corpus case for each.

**Done when** the line is gone from `test/parity/divergences/loader.txt`.
19 changes: 14 additions & 5 deletions src/global_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ Callable MakeFunction(Callable::ExpressionCallable fn)
return Callable(Callable::GlobalFunc, std::move(fn));
}

// A global that is a class in Jinja2, printed as Python prints the class (docs/tasks/0305)
Callable MakeClass(Callable::ExpressionCallable fn, const char* repr)
{
auto result = MakeFunction(std::move(fn));
result.SetRepr(repr);
return result;
}

ParsedArguments ParseArgs(const std::initializer_list<ArgumentInfo>& argsInfo, const CallParams& params, const char* fnName)
{
bool isSucceeded = true;
Expand Down Expand Up @@ -563,11 +571,12 @@ void SetupI18nGlobals(InternalValueMap& globalParams)

void SetupGlobals(InternalValueMap& globalParams)
{
globalParams.emplace("range", MakeFunction(CallRange));
globalParams.emplace("dict", MakeFunction(CallDict));
globalParams.emplace("cycler", MakeFunction(CallCycler));
globalParams.emplace("joiner", MakeFunction(CallJoiner));
globalParams.emplace("namespace", MakeFunction(CallNamespace));
globalParams.emplace("range", MakeClass(CallRange, "<class 'range'>"));
globalParams.emplace("dict", MakeClass(CallDict, "<class 'dict'>"));
globalParams.emplace("cycler", MakeClass(CallCycler, "<class 'jinja2.utils.Cycler'>"));
globalParams.emplace("joiner", MakeClass(CallJoiner, "<class 'jinja2.utils.Joiner'>"));
globalParams.emplace("namespace", MakeClass(CallNamespace, "<class 'jinja2.utils.Namespace'>"));
// A function, whose repr names its address: it prints nothing
globalParams.emplace("lipsum", MakeFunction([](const CallParams& params, RenderContext& context) {
// Every render has a callback (TemplateImpl::Render)
return CallLipsum(params, context.GetRendererCallback()->GetRandomEngine());
Expand Down
23 changes: 23 additions & 0 deletions src/internal_value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3113,6 +3113,24 @@ class ObjectMapAdapter : public InternalValueMapAdapter<ByVal, true>
std::optional<InternalValue> m_reprArgument;
};

// Jinja2's TemplateModule: <TemplateModule 'name'> as repr, its template's output as str()
class ModuleMapAdapter final : public ObjectMapAdapter
{
public:
ModuleMapAdapter(InternalValueMap&& values, InternalValue name, InternalValue output)
: ObjectMapAdapter(std::move(values), "TemplateModule", std::move(name))
, m_output(std::move(output))
{
}

[[nodiscard]] const InternalValue* GetStrValue() const override { return &m_output; }
[[nodiscard]] GenericMap CreateGenericMap() const override { return GenericMap(std::make_shared<const ModuleMapAdapter>(*this)); }

private:
// A string, which holds no other value
InternalValue m_output;
};

InternalValue Value2IntValue(const Value& val)
{
return std::visit(visitors::InputValueConvertor(false, true), val.GetData());
Expand Down Expand Up @@ -3755,6 +3773,11 @@ MapAdapter CreateObjectAdapter(InternalValueMap&& values, const char* className,
return MapAdapter(MakeRef<ObjectMapAdapter>(std::move(values), className, std::move(reprArgument)));
}

MapAdapter CreateModuleAdapter(InternalValueMap&& values, InternalValue name, InternalValue output)
{
return MapAdapter(MakeRef<ModuleMapAdapter>(std::move(values), std::move(name), std::move(output)));
}

MapAdapter CreateMapAdapter(const InternalValueMap* values)
{
return MapAdapter(MakeRef<InternalValueMapAdapter<ByRef, false>>(*values));
Expand Down
44 changes: 44 additions & 0 deletions src/internal_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,9 @@ struct IMapAccessor : ValueObject
// The value an object prints with as <Class value> (`self`, <TemplateReference 'name'>);
// null for one that prints as <module.Class object at 0x...>
[[nodiscard]] virtual const InternalValue* GetReprArgument() const { return nullptr; }
// The text str() of an object prints when it is not its repr (a template module's output,
// which is also its __html__); null for the others
[[nodiscard]] virtual const InternalValue* GetStrValue() const { return nullptr; }
// A for loop's `loop` object puts the attribute `attr` in `value` and returns true when
// it can be read without the generic lookup (`loop.index`, 0117 P1); others return false
virtual bool GetLoopAttr(LoopAttr /*attr*/, InternalValue& /*value*/) const { return false; }
Expand Down Expand Up @@ -914,6 +917,7 @@ class MapRef
[[nodiscard]] bool IsNamespace() const { return m_accessor != nullptr && m_accessor->IsNamespace(); }
[[nodiscard]] const char* GetObjectClass() const { return m_accessor ? m_accessor->GetObjectClass() : nullptr; }
[[nodiscard]] const InternalValue* GetReprArgument() const { return m_accessor ? m_accessor->GetReprArgument() : nullptr; }
[[nodiscard]] const InternalValue* GetStrValue() const { return m_accessor ? m_accessor->GetStrValue() : nullptr; }
// NOLINTNEXTLINE(modernize-use-nodiscard): see IMapAccessor::SetValue
bool SetValue(std::string name, const InternalValue& val) const { return m_accessor != nullptr && m_accessor->SetValue(std::move(name), val); }
[[nodiscard]] bool ShouldExtendLifetime() const { return m_accessor != nullptr && m_accessor->ShouldExtendLifetime(); }
Expand Down Expand Up @@ -1587,6 +1591,9 @@ MapAdapter CreateNamespaceAdapter(TemplateDict&& values);
MapAdapter CreateObjectAdapter(InternalValueMap&& values, const char* className);
// An object of `className` that prints as <className repr(reprArgument)>
MapAdapter CreateObjectAdapter(InternalValueMap&& values, const char* className, InternalValue reprArgument);
// Jinja2's TemplateModule of an imported template named `name`: `values` are its exports,
// `output` the text str() prints (docs/tasks/0309)
MapAdapter CreateModuleAdapter(InternalValueMap&& values, InternalValue name, InternalValue output);
MapAdapter CreateMapAdapter(const InternalValueMap* values);
// Shares the map: the adapter keeps it alive (a loop object kept past its loop)
MapAdapter CreateMapAdapter(std::shared_ptr<InternalValueMap> values);
Expand Down Expand Up @@ -1817,6 +1824,24 @@ inline bool IsStringView(const InternalValue& val)
}


// The object a bound method is bound to, as the method's repr prints it. It holds no
// template value, so the cycle collector need not see it
class IBoundReceiver
{
public:
IBoundReceiver() = default;
IBoundReceiver(const IBoundReceiver&) = delete;
IBoundReceiver(IBoundReceiver&&) = delete;
IBoundReceiver& operator=(const IBoundReceiver&) = delete;
IBoundReceiver& operator=(IBoundReceiver&&) = delete;
virtual ~IBoundReceiver() = default;

// The Python class of the object (LoopContext)
[[nodiscard]] virtual const char* GetClassName() const = 0;
// The repr of the object (<LoopContext 1/2>)
[[nodiscard]] virtual std::string Repr() const = 0;
};

class Callable
{
public:
Expand Down Expand Up @@ -1896,6 +1921,23 @@ class Callable
m_boundSelf = self;
m_boundMethod = method;
}
// As above, with the object the method is bound to, which its repr prints:
// <bound method LoopContext.cycle of <LoopContext 1/2>> (docs/tasks/0305)
void SetBoundMethod(const void* self, const char* method, std::shared_ptr<const IBoundReceiver> receiver)
{
SetBoundMethod(self, method);
m_boundReceiver = std::move(receiver);
}
// The object of a bound method's repr; null for other callables
[[nodiscard]] const std::shared_ptr<const IBoundReceiver>& GetBoundReceiver() const { return m_boundReceiver; }
// The name of a bound method; null for other callables
[[nodiscard]] const char* GetBoundMethod() const { return m_boundMethod; }

// The str() and repr() of a callable whose Python counterpart prints without an address,
// such as the classes among the globals (<class 'range'>); a static text, not an
// attribute, so range.name stays undefined (docs/tasks/0305)
void SetRepr(const char* repr) { m_repr = repr; }
[[nodiscard]] const char* GetRepr() const { return m_repr; }

// Python compares functions and macros by identity: the copies of one callable share its
// function, and a user callable converted from the same jinja2::UserCallable is the same
Expand All @@ -1910,6 +1952,8 @@ class Callable
std::shared_ptr<const InternalValueMap> m_attributes;
const void* m_boundSelf = nullptr;
const char* m_boundMethod = nullptr;
std::shared_ptr<const IBoundReceiver> m_boundReceiver;
const char* m_repr = nullptr;
};

// The function of a method bound to its receiver (methods::MakeBoundMethod): a type of its
Expand Down
5 changes: 5 additions & 0 deletions src/markup.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,11 @@ inline InternalValue MarkupEscape(const InternalValue& val, IRendererCallback* c
{
return host::Html(*host, callback->IsWideTarget());
}
// So is a template module, whose __html__ is its output as Markup (docs/tasks/0309)
if (const auto map = AsMap(val); map && map->GetStrValue())
{
return InternalValue(*map->GetStrValue()).SetMarkup();
Comment thread
rmorozov marked this conversation as resolved.
Resolved
}
// Any other value is escaped from its str()
auto text = callback->GetAsTargetString(val);
return std::visit([&escape](const auto& str) { return escape(std::basic_string_view<typename std::decay_t<decltype(str)>::value_type>(str)); }, text);
Expand Down
6 changes: 3 additions & 3 deletions src/serialize_filters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1540,13 +1540,13 @@ InternalValue XmlAttrFilter::Filter(const InternalValue& baseVal, RenderContext&
auto escape = [](const std::string& str) { return EscapeHtml(std::string_view(str)); };

// Port of Jinja2's do_xmlattr: key="escaped str(value)" in the mapping's order, None and
// undefined values left out. A macro prints its repr; other callables have no str() here
// (docs/tasks/0305)
// undefined values left out. A macro, a class among the globals and a bound method print
// their repr; other callables have no str() here, Python's names an address (docs/tasks/0305)
std::string result;
for (auto& [keyValue, value] : map->GetEntries())
{
const auto* callable = GetIf<Callable>(&value);
if (IsEmpty(value) || (callable && callable->GetKind() != Callable::Macro))
if (IsEmpty(value) || (callable && callable->GetKind() != Callable::Macro && !callable->GetRepr() && !callable->GetBoundReceiver()))
{
continue;
}
Expand Down
Loading
Loading