Skip to content

Commit

Permalink
Simplify the gencode for idom ve logging when the logOnly parameter…
Browse files Browse the repository at this point in the history
… is used.

Move the logic of whether to construct a `NullRenderer` into the `enter` functions, now the gencode just needs to capture the return value when passing a logonly parameter.

This saves 2 `if` statements and one local variable per `velog` with a logonly parameter.  This also fixes a more basic issue where we would accidentally generate the logic for the logonly expression twice!.

Clean up a few minor issues at the same time:
 * use `undefined` instead of `null` to save time and make dead property analysis simpler
 * rename `enter`/`exit` to disambiguate them.

PiperOrigin-RevId: 707130737
  • Loading branch information
lukesandberg authored and copybara-github committed Dec 17, 2024
1 parent e780519 commit 32adf6a
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,10 @@ Statement generateMsgGroupCode(MsgFallbackGroupNode node) {
if (phNode.getParent() instanceof VeLogNode) {
VeLogNode parent = (VeLogNode) phNode.getParent();
if (parent.getChild(0) == phNode) {
GenIncrementalDomTemplateBodyVisitor.VeLogStateHolder state =
idomTemplateBodyVisitor.openVeLogNode(parent);
// It is a compiler failure to have a logOnly in a message node.
Preconditions.checkState(state.logOnlyConditional == null);
value = Statements.of(state.enterStatement, value);
value = Statements.of(idomTemplateBodyVisitor.openVeLogNode(parent), value);
}
if (parent.getChild(parent.numChildren() - 1) == phNode) {
value = Statements.of(value, idomTemplateBodyVisitor.exitVeLogNode(parent, null));
value = Statements.of(value, idomTemplateBodyVisitor.exitVeLogNode(parent));
}
}
switchBuilder.addCase(Expressions.stringLiteral(ph.getKey()), value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_ATTR;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_CLOSE;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_ELEMENT_CLOSE;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_ENTER;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_EXIT;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_ENTER_VELOG;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_EXIT_VELOG;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_KEEP_GOING;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_LOGGING_FUNCTION_ATTR;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_OPEN;
Expand All @@ -38,9 +38,6 @@
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_PUSH_KEY;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_PUSH_MANUAL_KEY;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_TEXT;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_TODEFAULT;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_TONULL;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_VERIFY_LOGONLY;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.INCREMENTAL_DOM_VISIT_HTML_COMMENT;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.SOY_IDOM_CALL_DYNAMIC_ATTRIBUTES;
import static com.google.template.soy.incrementaldomsrc.IncrementalDomRuntime.SOY_IDOM_CALL_DYNAMIC_CSS;
Expand Down Expand Up @@ -139,33 +136,6 @@
* print the function calls and changing how statements are combined.
*/
public final class GenIncrementalDomTemplateBodyVisitor extends GenJsTemplateBodyVisitor {

/**
* Class that contains the state generated from visiting the beginning of a velogging statement.
* This allows one to generate code such as
*
* <pre>
* var velog_1 = foobar;
* if (velog_1) {
* idom = idom.toNullRenderer();
* }
* idom.enter(new Metadata(...));
* ...
* if (velog_1) {
* idom = idom.toDefaultRenderer();
* }
* </pre>
*/
static class VeLogStateHolder {
Expression logOnlyConditional; // Holds the variable reference to velog_1
Statement enterStatement; // Contains the idom.enter(...) statement

public VeLogStateHolder(Expression logOnlyConditional, Statement enterStatement) {
this.logOnlyConditional = logOnlyConditional;
this.enterStatement = enterStatement;
}
}

private final Deque<SanitizedContentKind> contentKind;
private final List<Statement> staticVarDeclarations;
private final boolean generatePositionalParamsSignature;
Expand Down Expand Up @@ -1080,45 +1050,29 @@ protected Statement visitSkipNode(SkipNode node) {
@Override
protected Statement visitVeLogNode(VeLogNode node) {
List<Statement> statements = new ArrayList<>();
VeLogStateHolder state = openVeLogNode(node);
statements.add(state.enterStatement);
statements.add(openVeLogNode(node));
statements.addAll(visitChildren(node));
statements.add(exitVeLogNode(node, state.logOnlyConditional));
statements.add(exitVeLogNode(node));
return Statements.of(statements);
}

VeLogStateHolder openVeLogNode(VeLogNode node) {
Expression isLogOnly = Expressions.LITERAL_FALSE;
VariableDeclaration isLogOnlyVar;
Expression isLogOnlyReference = null;
List<Statement> stmts = new ArrayList<>();
Statement openVeLogNode(VeLogNode node) {
Expression veData = getExprTranslator().exec(node.getVeDataExpression());
if (node.getLogonlyExpression() != null) {
String idName = "velog_" + staticsCounter++;
isLogOnlyReference = id(idName);
isLogOnly = getExprTranslator().exec(node.getLogonlyExpression());
isLogOnlyVar = VariableDeclaration.builder(idName).setRhs(isLogOnly).build();
stmts.add(isLogOnlyVar);
stmts.add(
Statements.ifStatement(
INCREMENTAL_DOM_VERIFY_LOGONLY.call(isLogOnlyVar.ref()),
Statements.assign(INCREMENTAL_DOM, INCREMENTAL_DOM_TONULL.call()))
.build());
return Statements.assign(
INCREMENTAL_DOM,
INCREMENTAL_DOM_ENTER_VELOG.call(
veData, getExprTranslator().exec(node.getLogonlyExpression())));
}
Expression veData = getExprTranslator().exec(node.getVeDataExpression());
stmts.add(INCREMENTAL_DOM_ENTER.call(veData, isLogOnly).asStatement());
return new VeLogStateHolder(isLogOnlyReference, Statements.of(stmts));
return INCREMENTAL_DOM_ENTER_VELOG.call(veData).asStatement();
}

Statement exitVeLogNode(VeLogNode node, @Nullable Expression isLogOnly) {
Statement exit = INCREMENTAL_DOM_EXIT.call().asStatement();
if (isLogOnly != null) {
return Statements.of(
exit,
Statements.ifStatement(
isLogOnly, Statements.assign(INCREMENTAL_DOM, INCREMENTAL_DOM_TODEFAULT.call()))
.build());
Statement exitVeLogNode(VeLogNode node) {
var exit = INCREMENTAL_DOM_EXIT_VELOG.call();
if (node.getLogonlyExpression() != null) {
return Statements.assign(INCREMENTAL_DOM, exit);
}
return exit;
return exit.asStatement();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,11 @@ final class IncrementalDomRuntime {
public static final Expression INCREMENTAL_DOM_APPLY_ATTRS =
INCREMENTAL_DOM.dotAccess("applyAttrs");

public static final Expression INCREMENTAL_DOM_ENTER = INCREMENTAL_DOM.dotAccess("enter");
public static final Expression INCREMENTAL_DOM_ENTER_VELOG =
INCREMENTAL_DOM.dotAccess("enterVeLog");

public static final Expression INCREMENTAL_DOM_EXIT = INCREMENTAL_DOM.dotAccess("exit");

public static final Expression INCREMENTAL_DOM_VERIFY_LOGONLY =
INCREMENTAL_DOM.dotAccess("verifyLogOnly");

public static final Expression INCREMENTAL_DOM_TODEFAULT =
INCREMENTAL_DOM.dotAccess("toDefaultRenderer");

public static final Expression INCREMENTAL_DOM_TONULL =
INCREMENTAL_DOM.dotAccess("toNullRenderer");
public static final Expression INCREMENTAL_DOM_EXIT_VELOG =
INCREMENTAL_DOM.dotAccess("exitVeLog");

public static final Expression INCREMENTAL_DOM_TEXT = INCREMENTAL_DOM.dotAccess("text");
public static final Expression INCREMENTAL_DOM_PRINT = INCREMENTAL_DOM.dotAccess("print");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ private static LoggingFunctionInfo getLoggingFunctionInfo(CallParamNode paramNod
/**
* Element composition calls are deconstructed into call nodes. However, some of the attributes
* contain velogging functions. This takes those attributes and puts them on a wrapping `veAttr`
* element, which the runtime libarary then manages. So the overall DOM structure becomes
* element, which the runtime library then manages. So the overall DOM structure becomes
*
* <pre>{@code
* <velog>
Expand Down
Loading

0 comments on commit 32adf6a

Please sign in to comment.