Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed optimization of filters and misoptimizations with changed eval context #476

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 29 additions & 62 deletions jinja2/nodes.py
Expand Up @@ -94,16 +94,6 @@ def revert(self, old):
self.__dict__.update(old)


def get_eval_context(node, ctx):
if ctx is None:
if node.environment is None:
raise RuntimeError('if no eval context is passed, the '
'node must have an attached '
'environment.')
return EvalContext(node.environment)
return ctx


class Node(with_metaclass(NodeType, object)):
"""Baseclass for all Jinja2 nodes. There are a number of nodes available
of different types. There are four major types:
Expand Down Expand Up @@ -356,14 +346,10 @@ class Expr(Node):
"""Baseclass for all expressions."""
abstract = True

def as_const(self, eval_ctx=None):
def as_const(self, eval_ctx):
"""Return the value of the expression as constant or raise
:exc:`Impossible` if this was not possible.

An :class:`EvalContext` can be provided, if none is given
a default context is created which requires the nodes to have
an attached environment.

.. versionchanged:: 2.4
the `eval_ctx` parameter was added.
"""
Expand All @@ -380,8 +366,7 @@ class BinExpr(Expr):
operator = None
abstract = True

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
# intercepted operators cannot be folded at compile time
if self.environment.sandboxed and \
self.operator in self.environment.intercepted_binops:
Expand All @@ -399,8 +384,7 @@ class UnaryExpr(Expr):
operator = None
abstract = True

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
# intercepted operators cannot be folded at compile time
if self.environment.sandboxed and \
self.operator in self.environment.intercepted_unops:
Expand Down Expand Up @@ -440,7 +424,7 @@ class Const(Literal):
"""
fields = ('value',)

def as_const(self, eval_ctx=None):
def as_const(self, eval_ctx):
return self.value

@classmethod
Expand All @@ -459,8 +443,7 @@ class TemplateData(Literal):
"""A constant template string."""
fields = ('data',)

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
if eval_ctx.volatile:
raise Impossible()
if eval_ctx.autoescape:
Expand All @@ -475,8 +458,7 @@ class Tuple(Literal):
"""
fields = ('items', 'ctx')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return tuple(x.as_const(eval_ctx) for x in self.items)

def can_assign(self):
Expand All @@ -490,8 +472,7 @@ class List(Literal):
"""Any list literal such as ``[1, 2, 3]``"""
fields = ('items',)

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return [x.as_const(eval_ctx) for x in self.items]


Expand All @@ -501,26 +482,23 @@ class Dict(Literal):
"""
fields = ('items',)

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return dict(x.as_const(eval_ctx) for x in self.items)


class Pair(Helper):
"""A key, value pair for dicts."""
fields = ('key', 'value')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return self.key.as_const(eval_ctx), self.value.as_const(eval_ctx)


class Keyword(Helper):
"""A key, value pair for keyword arguments where key is a string."""
fields = ('key', 'value')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return self.key, self.value.as_const(eval_ctx)


Expand All @@ -530,8 +508,7 @@ class CondExpr(Expr):
"""
fields = ('test', 'expr1', 'expr2')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
if self.test.as_const(eval_ctx):
return self.expr1.as_const(eval_ctx)

Expand All @@ -551,9 +528,8 @@ class Filter(Expr):
"""
fields = ('node', 'name', 'args', 'kwargs', 'dyn_args', 'dyn_kwargs')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
if eval_ctx.volatile or self.node is None:
def as_const(self, eval_ctx):
if self.node is None:
raise Impossible()
# we have to be careful here because we call filter_ below.
# if this variable would be called filter, 2to3 would wrap the
Expand All @@ -563,9 +539,10 @@ def as_const(self, eval_ctx=None):
filter_ = self.environment.filters.get(self.name)
if filter_ is None or getattr(filter_, 'contextfilter', False):
raise Impossible()
obj = self.node.as_const(eval_ctx)
args = [x.as_const(eval_ctx) for x in self.args]
args = [x.as_const(eval_ctx) for x in [self.node] + self.args]
if getattr(filter_, 'evalcontextfilter', False):
if eval_ctx.volatile:
raise Impossible()
args.insert(0, eval_ctx)
elif getattr(filter_, 'environmentfilter', False):
args.insert(0, self.environment)
Expand All @@ -581,7 +558,7 @@ def as_const(self, eval_ctx=None):
except Exception:
raise Impossible()
try:
return filter_(obj, *args, **kwargs)
return filter_(*args, **kwargs)
except Exception:
raise Impossible()

Expand All @@ -602,10 +579,7 @@ class Call(Expr):
"""
fields = ('node', 'args', 'kwargs', 'dyn_args', 'dyn_kwargs')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
if eval_ctx.volatile:
raise Impossible()
def as_const(self, eval_ctx):
obj = self.node.as_const(eval_ctx)

# don't evaluate context functions
Expand All @@ -614,6 +588,8 @@ def as_const(self, eval_ctx=None):
if getattr(obj, 'contextfunction', False):
raise Impossible()
elif getattr(obj, 'evalcontextfunction', False):
if eval_ctx.volatile:
raise Impossible()
args.insert(0, eval_ctx)
elif getattr(obj, 'environmentfunction', False):
args.insert(0, self.environment)
Expand All @@ -639,8 +615,7 @@ class Getitem(Expr):
"""Get an attribute or item from an expression and prefer the item."""
fields = ('node', 'arg', 'ctx')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
if self.ctx != 'load':
raise Impossible()
try:
Expand All @@ -659,11 +634,10 @@ class Getattr(Expr):
"""
fields = ('node', 'attr', 'ctx')

def as_const(self, eval_ctx=None):
def as_const(self, eval_ctx):
if self.ctx != 'load':
raise Impossible()
try:
eval_ctx = get_eval_context(self, eval_ctx)
return self.environment.getattr(self.node.as_const(eval_ctx),
self.attr)
except Exception:
Expand All @@ -679,8 +653,7 @@ class Slice(Expr):
"""
fields = ('start', 'stop', 'step')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
def const(obj):
if obj is None:
return None
Expand All @@ -694,8 +667,7 @@ class Concat(Expr):
"""
fields = ('nodes',)

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return ''.join(text_type(x.as_const(eval_ctx)) for x in self.nodes)


Expand All @@ -705,8 +677,7 @@ class Compare(Expr):
"""
fields = ('expr', 'ops')

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
result = value = self.expr.as_const(eval_ctx)
try:
for op in self.ops:
Expand Down Expand Up @@ -769,17 +740,15 @@ class And(BinExpr):
"""Short circuited AND."""
operator = 'and'

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return self.left.as_const(eval_ctx) and self.right.as_const(eval_ctx)


class Or(BinExpr):
"""Short circuited OR."""
operator = 'or'

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return self.left.as_const(eval_ctx) or self.right.as_const(eval_ctx)


Expand Down Expand Up @@ -845,8 +814,7 @@ class MarkSafe(Expr):
"""Mark the wrapped expression as safe (wrap it as `Markup`)."""
fields = ('expr',)

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
return Markup(self.expr.as_const(eval_ctx))


Expand All @@ -858,8 +826,7 @@ class MarkSafeIfAutoescape(Expr):
"""
fields = ('expr',)

def as_const(self, eval_ctx=None):
eval_ctx = get_eval_context(self, eval_ctx)
def as_const(self, eval_ctx):
if eval_ctx.volatile:
raise Impossible()
expr = self.expr.as_const(eval_ctx)
Expand Down
21 changes: 19 additions & 2 deletions jinja2/optimizer.py
Expand Up @@ -31,6 +31,7 @@ class Optimizer(NodeTransformer):

def __init__(self, environment):
self.environment = environment
self.eval_ctx = nodes.EvalContext(environment)

def visit_If(self, node):
"""Eliminate dead code."""
Expand All @@ -39,7 +40,7 @@ def visit_If(self, node):
if node.find(nodes.Block) is not None:
return self.generic_visit(node)
try:
val = self.visit(node.test).as_const()
val = self.visit(node.test).as_const(self.eval_ctx)
except nodes.Impossible:
return self.generic_visit(node)
if val:
Expand All @@ -51,11 +52,27 @@ def visit_If(self, node):
result.extend(self.visit_list(node))
return result

def visit_EvalContextModifier(self, node):
for keyword in node.options:
try:
value = keyword.value.as_const(self.eval_ctx)
except nodes.Impossible:
self.eval_ctx.volatile = True
else:
setattr(self.eval_ctx, keyword.key, value)
return self.generic_visit(node)

def visit_ScopedEvalContextModifier(self, node):
backup = self.eval_ctx.save()
node = self.visit_EvalContextModifier(node)
self.eval_ctx.revert(backup)
return node

def fold(self, node):
"""Do constant folding."""
node = self.generic_visit(node)
try:
return nodes.Const.from_untrusted(node.as_const(),
return nodes.Const.from_untrusted(node.as_const(self.eval_ctx),
lineno=node.lineno,
environment=self.environment)
except nodes.Impossible:
Expand Down