Skip to content

Implement EagerIfTag#553

Merged
jasmith-hs merged 7 commits into
masterfrom
eager-if
Dec 9, 2020
Merged

Implement EagerIfTag#553
jasmith-hs merged 7 commits into
masterfrom
eager-if

Conversation

@jasmith-hs

@jasmith-hs jasmith-hs commented Nov 30, 2020

Copy link
Copy Markdown
Contributor

Part of #532

This PR adds the functionality necessary for eager execution with if tags. Unless tags are handled the same when executing eagerly as they are a negated if tag, and the branch paths are evaluated in the same way.

If the correct path cannot be determined because of a DeferredValueException, then the eagerInterpret() method gets called and all branches of the if tree get executed in a speculative fashion. The children are executed in a child context that is set to protectedMode = true. This protected mode prevents certain things from happening that would change the state of the context. When one of these occurs (such as replacing or updating a value in the interpreter.getContext()), the previous value gets reconstructed. See #545 as this logic is generic and has been introduced already.

Also, there is some logic to prune branches that certainly won't be executed. This is possible when there are ElseIfTags present so we don't need to include code unnecessarily.


A quick example:

{% set foo,bar = 'is foo','is bar' %}
{% if contact.registered %}
{{ foo }}
{% else %}
{{ bar }}
{% endif %}

This is a simple case, where eagerly executing the if statement (assuming contact is deferred) would cause the output to be:

{% if contact.registered %}
is foo
{% else %}
is bar
{% endif %}

A second pass with the contact set would either give is foo or is bar, without needing to preserve the context. This is even more beneficial when rather than evaluating simple expressions, complex functions (possibly calling APIs) can be eagerly evaluated within these branches.

cc @jboulter @Joeoh

public String renderChildren(TagNode tagNode, JinjavaInterpreter interpreter) {
// If the branch is impossible, it should be removed
boolean definitelyDrop = shouldDropBranch(tagNode, interpreter);
// If the branch would definitely get executed, change it to and else

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is a little unclear to me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I have a typo in it. It's saying that if an elseif branch would definitely get executed, it can be changed to an else.
Ex.

{% if deferred %}
1.
{% elseif foo > 0 %}
2.
{% else %}
3.
{% endif %}

if foo == 2, then it could get output like:

{% if deferred %}
1.
{% else %}
2.
{% endif %}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the opposite for definitelyDrop. If there were to be a tag like: {% elseif foo == 0 %}, it could get dropped as it's impossible.

@jasmith-hs jasmith-hs mentioned this pull request Dec 1, 2020
32 tasks
if (definitelyExecuted) {
break;
}
definitelyDrop =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we drop a branch, what happens to the whitespace around it? I could see this changing the output of templates because of dropped statements and whitespace (not that it's generally useful).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good question. The trimming gets handled when the tree gets parsed. Dropping a statement here is done similarly to picking the correct branch in the normal IfTag (and then not executing any of the others). Since no newlines are being added from the EagerIfTag's logic, the resulting whitespace is accurate.

The TreeParser takes the trim characters from a TagToken and moves them over to the TextToken children so we the whitespace trimming doesn't get handled in the tag node logic.

Base automatically changed from eager-expression-node to master December 9, 2020 17:27
@jasmith-hs jasmith-hs merged commit a47abf5 into master Dec 9, 2020
@jasmith-hs jasmith-hs deleted the eager-if branch December 9, 2020 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants