Skip to content

Commit

Permalink
Store shadowed AST chunks on host nodes using a Prop.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 694634080
  • Loading branch information
12wrigja authored and copybara-github committed Nov 8, 2024
1 parent bca5f22 commit 35edd47
Show file tree
Hide file tree
Showing 12 changed files with 256 additions and 10 deletions.
36 changes: 36 additions & 0 deletions src/com/google/javascript/jscomp/AstValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,7 @@ private void validateName(Node n) {
validateProperties(n);
validateChildCount(n);
validateTypeInformation(n);
validateShadowContentIfPresent(n);
}

private void validateOptionalName(Node n) {
Expand All @@ -1124,6 +1125,41 @@ private void validateOptionalName(Node n) {
}
}

private void validateShadowContentIfPresent(Node n) {
Node shadow = n.getClosureUnawareShadow();
if (shadow == null) {
return;
}
if (!shadow.isRoot()) {
violation("Shadow reference node is not a ROOT node", shadow);
return;
}
Node shadowScript = shadow.getFirstChild();
if (shadowScript == null || !shadowScript.isScript()) {
violation("Shadow root node's child is not a script node", shadowScript);
return;
}
if (shadowScript.getChildCount() != 1) {
violation("Shadow SCRIPT node child has more than one child", shadowScript);
return;
}
Node exprResult = shadowScript.getFirstChild();
if (exprResult == null || !exprResult.isExprResult()) {
violation("Shadow SCRIPT node child is not an expr result node", exprResult);
return;
}
if (exprResult.getChildCount() != 1) {
violation("Shadow EXPR_RESULT node should have exactly one child", exprResult);
return;
}
Node shadowJsFunction = exprResult.getFirstChild();
if (!shadowJsFunction.isFunction()) {
violation("Shadow node EXPR_RESULT child is not a function", shadowJsFunction);
return;
}
validateFunctionExpression(shadowJsFunction);
}

private void validateEmptyName(Node n) {
validateNodeType(Token.NAME, n);
validateEmptyString(n);
Expand Down
21 changes: 21 additions & 0 deletions src/com/google/javascript/jscomp/ChangeVerifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ private void associateClones(Node n, Node snapshot) {
child = child.getNext();
snapshotChild = snapshotChild.getNext();
}

Node shadowChild = n.getClosureUnawareShadow();
Node snapshotShadowChild = snapshot.getClosureUnawareShadow();
if (shadowChild != null && snapshotShadowChild != null) {
associateClones(shadowChild, snapshotShadowChild);
} else if (shadowChild != null || snapshotShadowChild != null) {
throw new IllegalStateException("invalid Shadow node state");
}
}

/** Checks that the scope roots marked as changed have indeed changed */
Expand All @@ -90,6 +98,10 @@ public void visit(Node oldNode) {
if (NodeUtil.isChangeScopeRoot(oldNode)) {
snapshotScopeNodes.add(oldNode);
}
Node shadow = oldNode.getClosureUnawareShadow();
if (shadow != null) {
NodeUtil.visitPreOrder(shadow.getFirstFirstChild(), this);
}
}
});

Expand All @@ -111,6 +123,15 @@ public void visit(Node n) {
verifyNodeChange(passNameMsg, n, clone);
}
}
Node shadow = n.getClosureUnawareShadow();
if (shadow != null) {
checkState(shadow.isRoot(), shadow);
checkState(shadow.getChildCount() == 1, shadow);
Node script = shadow.getFirstChild();
checkState(script.isScript(), script);
checkState(script.getChildCount() == 1, script);
NodeUtil.visitPreOrder(script.getFirstChild(), this);
}
}
});

Expand Down
5 changes: 5 additions & 0 deletions src/com/google/javascript/jscomp/NodeUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -5913,6 +5913,11 @@ public static void markFunctionsDeleted(Node node, AbstractCompiler compiler) {
for (Node child = node.getFirstChild(); child != null; child = child.getNext()) {
markFunctionsDeleted(child, compiler);
}

Node shadowed = node.getClosureUnawareShadow();
if (shadowed != null) {
markFunctionsDeleted(shadowed, compiler);
}
}

/** Returns the list of scope nodes which are parents of the provided list of scope nodes. */
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/PassFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public static PassFactory createEmptyPass(String name) {
}

/** Creates a new compiler pass to be run. */
final CompilerPass create(AbstractCompiler compiler) {
public final CompilerPass create(AbstractCompiler compiler) {
return getInternalFactory().apply(compiler);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,21 @@ private Node visit(AstNode astNode, FeatureContext context, @Nullable Node sourc
this.recordScriptFeatures(context, n);

FeatureContext newContext = contextFor(context, n);
if (Node.hasBitSet(properties, NodeProperty.CLOSURE_UNAWARE_SHADOW.getNumber())) {
AstNode serializedShadowChild = astNode.getChild(0);
Node shadowedCode = this.visit(serializedShadowChild, newContext, sourceFileTemplate);
this.owner().setOriginalNameIfPresent(serializedShadowChild, shadowedCode);
// The shadowed code is only the "source" parts of the shadow structure, and does not
// include the synthetic code that is needed for the compiler to consider it a valid
// standalone AST. We recreate that here.
// This must be kept in sync with the shadow structure created by TypedAstSerializer.
Node shadowRoot = IR.root(IR.script(IR.exprResult(shadowedCode)));
shadowRoot.getFirstChild().setStaticSourceFileFrom(sourceFileTemplate);
shadowRoot.getFirstFirstChild().setStaticSourceFileFrom(sourceFileTemplate);
n.setClosureUnawareShadow(shadowRoot);
return n;
}

int children = astNode.getChildCount();
for (int i = 0; i < children; i++) {
AstNode child = astNode.getChild(i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,13 @@ public void visit(NodeTraversal t, Node n, Node parent) {
if (type != null) {
typePointersByJstype.computeIfAbsent(type, jstypeReconserializer::serializeType);
}
Node shadow = n.getClosureUnawareShadow();
if (shadow != null) {
// Shadow roots are structured as
// ROOT -> SCRIPT -> EXPR_RESULT -> FUNCTION
NodeTraversal.traverse(
compiler, shadow.getFirstFirstChild().getFirstChild(), new TypeSearchCallback());
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,19 @@ private AstNode visit(Node n) {
for (Node child = n.getFirstChild(); child != null; child = child.getNext()) {
builder.addChild(visit(child));
}
Node shadowedCode = n.getClosureUnawareShadow();
if (shadowedCode != null) {
// For Closure shadow hosts, the ASTNode will get a boolean property bit set
// (CLOSURE_UNAWARE_SHADOW) that indicates this child ASTNode is not a normal child, but is
// the contents of the shadow.
// Shadow roots are structured as
// ROOT -> SCRIPT -> EXPR_RESULT -> FUNCTION
// We avoid including the ROOT -> SCRIPT -> EXPR_RESULT structure in the serialization as it
// is all synthetic code that would unnecessarily bloat the TypedAST and is instead recreated
// upon deserialization.
// The child ASTNode is just the FUNCTION.
builder.addChild(visit(shadowedCode.getFirstFirstChild().getFirstChild()));
}

if (sourceFile != 0) {
subtreeSourceFiles.removeLast();
Expand Down
58 changes: 56 additions & 2 deletions src/com/google/javascript/rhino/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ enum Prop {
// Only present in the "synthetic externs file". Builds initialized using a
// "TypedAST filesystem" will delete any such declarations present in a different compilation
// shard
SYNTHESIZED_UNFULFILLED_NAME_DECLARATION
SYNTHESIZED_UNFULFILLED_NAME_DECLARATION,
// This prop holds a reference to a closure-unaware sub-AST.
CLOSURE_UNAWARE_SHADOW,
}

// Avoid cloning "values" repeatedly in hot code, we save it off now.
Expand Down Expand Up @@ -606,6 +608,22 @@ public final Node getOnlyChild() {
return this == parent.first ? null : previous;
}

public final void setClosureUnawareShadow(@Nullable Node shadowRoot) {
checkState(this.first == null, "Cannot set shadow root on a node with children");
checkState(this.token == Token.NAME, "Only NAME nodes can be used as shadows");
this.putProp(Prop.CLOSURE_UNAWARE_SHADOW, shadowRoot);
}

public final @Nullable Node getClosureUnawareShadow() {
if (this.token != Token.NAME) {
// As checked in setClosureUnawareShadow, only NAME nodes can have shadow content.
// We short-circuit the getProp call here because that can be very expensive for nodes that
// have long proplists (and we know it wouldn't find this prop anyways).
return null;
}
return (Node) this.getProp(Prop.CLOSURE_UNAWARE_SHADOW);
}

/**
* Gets the ith child, note that this is O(N) where N is the number of children.
*
Expand Down Expand Up @@ -1040,6 +1058,13 @@ public void validateProperties(Consumer<String> violationMessageConsumer) {
"Expected all synthetic unfulfilled declarations to be `var <name>`");
}
break;
case CLOSURE_UNAWARE_SHADOW:
PropListItem shadowProp = lookupProperty(Prop.CLOSURE_UNAWARE_SHADOW);
if (!(shadowProp instanceof Node.ObjectPropListItem)
|| !(shadowProp.getObjectValue() instanceof Node)) {
violationMessageConsumer.accept("CLOSURE_UNAWARE_SHADOW property must point to a Node");
}
break;
default:
// No validation is currently done for other properties
break;
Expand Down Expand Up @@ -1119,7 +1144,7 @@ static boolean hasNodePropertyBitSet(long bitset, NodeProperty prop) {
return (bitset & nodePropertyToBit(prop)) != 0;
}

static boolean hasBitSet(long bitset, int bit) {
public static final boolean hasBitSet(long bitset, int bit) {
return (bitset & 1L << bit) != 0;
}

Expand All @@ -1146,6 +1171,14 @@ public final long serializeProperties() {
case SIDE_EFFECT_FLAGS:
propSet = setNodePropertySideEffectFlags(propSet, propListItem.getIntValue());
break;
case CLOSURE_UNAWARE_SHADOW:
// This is a bit of an unusual case, because the CLOSURE_UNAWARE_SHADOW Prop is a Node
// pointer, not a boolean.
// However, it is treated as a boolean property in the TypedAST representation as a signal
// that the child ASTNode is shadowed code and not a normal child node.
// We check for this bit when building in ScriptNodeDeserializer.
propSet = setNodePropertyBit(propSet, NodeProperty.CLOSURE_UNAWARE_SHADOW);
break;
default:
if (propListItem instanceof Node.IntPropListItem) {
NodeProperty nodeProperty = PropTranslator.serialize(prop);
Expand Down Expand Up @@ -1218,6 +1251,13 @@ public final void deserializeProperties(long propSet) {
case THROWS:
sideEffectFlags |= SideEffectFlags.THROWS;
break;
case CLOSURE_UNAWARE_SHADOW:
// Ignore this - we need the deserialized child to actually set the shadow.
// We'll check for this higher up in the call stack.
// If we don't ignore this, then a boolean prop is created (and then later clobbered)
// representing this property, which seems silly at best and potentially dangerous at
// worst.
break;
default:
// All other properties are booleans that are 1-to-1 equivalent with Node properties.
Prop prop = PropTranslator.deserialize(nodeProperty);
Expand Down Expand Up @@ -1396,6 +1436,10 @@ private void toString(
byte[] keys = getSortedPropTypes();
for (int i = 0; i < keys.length; i++) {
Prop type = PROP_VALUES[keys[i]];
if (type == Prop.CLOSURE_UNAWARE_SHADOW) {
sb.append(" [is_shadow_host]");
continue;
}
PropListItem x = lookupProperty(type);
sb.append(" [");
sb.append(Ascii.toLowerCase(String.valueOf(type)));
Expand Down Expand Up @@ -1534,6 +1578,10 @@ private static void toStringTreeHelper(Node n, int level, Appendable sb) throws
for (Node cursor = n.first; cursor != null; cursor = cursor.next) {
toStringTreeHelper(cursor, level + 1, sb);
}
Node shadow = n.getClosureUnawareShadow();
if (shadow != null) {
toStringTreeHelper(shadow, level + 1, sb);
}
}

public final void appendJsonTree(Appendable appendable) throws IOException {
Expand Down Expand Up @@ -2460,6 +2508,12 @@ public final Node cloneTree(boolean cloneTypeExprs) {
lastChild.next = null;
result.first = firstChild;
}

Node shadow = this.getClosureUnawareShadow();
if (shadow != null) {
result.setClosureUnawareShadow(shadow.cloneTree(cloneTypeExprs));
}

return result;
}

Expand Down
4 changes: 4 additions & 0 deletions src/com/google/javascript/rhino/PropTranslator.java
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ private static final void setProps() {
return NodeProperty.CONSTANT_VAR_FLAGS;
case SYNTHESIZED_UNFULFILLED_NAME_DECLARATION:
return NodeProperty.SYNTHESIZED_UNFULFILLED_NAME_DECLARATION;
case CLOSURE_UNAWARE_SHADOW:
case SIDE_EFFECT_FLAGS:
case DECLARED_TYPE_EXPR:
case FEATURE_SET:
Expand Down Expand Up @@ -200,6 +201,9 @@ private static final void checkUnexpectedNullProtoProps() {
case THROWS:
// these are used for the SIDE_EFFECT_FLAGS bit field property
break;
case CLOSURE_UNAWARE_SHADOW:
// this is a special case where the property is a Node pointer, not a boolean
break;
default:
// everything else should be a 1-to-1 match with a boolean property
checkState(
Expand Down
5 changes: 5 additions & 0 deletions src/com/google/javascript/rhino/typed_ast/typed_ast.proto
Original file line number Diff line number Diff line change
Expand Up @@ -407,4 +407,9 @@ enum NodeProperty {
MUTATES_THIS = 47;
MUTATES_ARGUMENTS = 48;
THROWS = 49;

// Indicates whether this node is shadowing closure-unware code. In the
// TypedAST, the singular child AstNode is the shadowed code, not a normal
// child node.
CLOSURE_UNAWARE_SHADOW = 50;
}
29 changes: 29 additions & 0 deletions test/com/google/javascript/jscomp/AstValidatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1682,4 +1682,33 @@ private void testFeatureValidation(String code, Feature feature) {
script.putProp(Node.FEATURE_SET, currentFeatures.without(feature));
expectInvalid(script, Check.SCRIPT);
}

@Test
public void testShadowContent_validatesSingularExpectedStructure() {
// Since we're building the AST by hand, there won't be any types on it.
typeInfoValidationMode = TypeInfoValidation.NONE;

Node shadowHost = IR.name("f");
Node f = IR.exprResult(shadowHost);
expectValid(f, Check.STATEMENT);

shadowHost.setClosureUnawareShadow(IR.name("x"));
expectInvalid(f, Check.STATEMENT, "Shadow reference node is not a ROOT node");

shadowHost.setClosureUnawareShadow(IR.root());
expectInvalid(f, Check.STATEMENT, "Shadow root node's child is not a script node");

shadowHost.setClosureUnawareShadow(
IR.root(IR.script(IR.exprResult(IR.name("x")), IR.exprResult(IR.name("y")))));
expectInvalid(f, Check.STATEMENT, "Shadow SCRIPT node child has more than one child");

shadowHost.setClosureUnawareShadow(IR.root(this.parseValidScript("function foo(){}").detach()));
expectInvalid(f, Check.STATEMENT, "Shadow SCRIPT node child is not an expr result node");

shadowHost.setClosureUnawareShadow(IR.root(this.parseValidScript("(x++)").detach()));
expectInvalid(f, Check.STATEMENT, "Shadow node EXPR_RESULT child is not a function");

shadowHost.setClosureUnawareShadow(IR.root(this.parseValidScript("(function(){})").detach()));
expectValid(f, Check.STATEMENT);
}
}
Loading

0 comments on commit 35edd47

Please sign in to comment.