Skip to content

Commit

Permalink
Rework the recent change that preserves cast information after inlining.
Browse files Browse the repository at this point in the history
Remove the methods from NodeUtil, as they aren't really general purpose, and add the functionality to FunctionInjector directly. Add an integration test to make sure that j2cl inlining and disambiguate-properties are working together as expected.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=138793388
  • Loading branch information
dimvar authored and blickly committed Nov 11, 2016
1 parent 7cf5873 commit e02df6b
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 62 deletions.
3 changes: 1 addition & 2 deletions src/com/google/javascript/jscomp/ConformanceRules.java
Expand Up @@ -311,8 +311,7 @@ protected boolean isAssertionCall(Node n) {
}

protected boolean isTypeImmediatelyTightened(Node n) {
Node parent = n.getParent();
return NodeUtil.wasCasted(n) || isAssertionCall(parent);
return isAssertionCall(n.getParent()) || /* casted node */ n.getTypeIBeforeCast() != null;
}

protected boolean isUsed(Node n) {
Expand Down
13 changes: 9 additions & 4 deletions src/com/google/javascript/jscomp/FunctionInjector.java
Expand Up @@ -22,7 +22,7 @@
import com.google.javascript.jscomp.ExpressionDecomposer.DecompositionType;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;

import com.google.javascript.rhino.TypeI;
import java.util.Collection;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -291,8 +291,11 @@ private Node inlineReturnValue(Reference ref, Node fnNode) {
}

// If the call site had a cast ensure it's persisted to the new expression that replaces it.
NodeUtil.maybePropagateCast(callNode, newExpression);

TypeI typeBeforeCast = callNode.getTypeIBeforeCast();
if (typeBeforeCast != null) {
newExpression.putProp(Node.TYPE_BEFORE_CAST, typeBeforeCast);
newExpression.setTypeI(callNode.getTypeI());
}
callParentNode.replaceChild(callNode, newExpression);
return newExpression;
}
Expand Down Expand Up @@ -563,7 +566,9 @@ private Node inlineFunction(
private static void removeConstantVarAnnotation(Scope scope, String name) {
Var var = scope.getVar(name);
Node nameNode = var == null ? null : var.getNameNode();
if (nameNode == null) return;
if (nameNode == null) {
return;
}

if (nameNode.getBooleanProp(Node.IS_CONSTANT_VAR)) {
nameNode.removeProp(Node.IS_CONSTANT_VAR);
Expand Down
11 changes: 0 additions & 11 deletions src/com/google/javascript/jscomp/NodeUtil.java
Expand Up @@ -4868,15 +4868,4 @@ static boolean isGetterOrSetter(Node propNode) {
static boolean isCallTo(Node n, String qualifiedName) {
return n.isCall() && n.getFirstChild().matchesQualifiedName(qualifiedName);
}

static boolean wasCasted(Node node) {
return node.getProp(Node.TYPE_BEFORE_CAST) != null;
}

static void maybePropagateCast(Node source, Node destination) {
if (wasCasted(source)) {
destination.putProp(Node.TYPE_BEFORE_CAST, source.getProp(Node.TYPE_BEFORE_CAST));
destination.setJSType(source.getJSType());
}
}
}
33 changes: 33 additions & 0 deletions test/com/google/javascript/jscomp/IntegrationTest.java
Expand Up @@ -23,6 +23,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.javascript.jscomp.CompilerOptions.DisposalCheckingPolicy;
import com.google.javascript.jscomp.CompilerOptions.J2clPassMode;
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
import com.google.javascript.jscomp.CompilerOptions.Reach;
import com.google.javascript.jscomp.deps.ModuleLoader;
Expand Down Expand Up @@ -1768,6 +1769,38 @@ public void testDeadAssignmentsElimination() {
test(options, code, "function f() { var x; 3; 4; x = 5; return x; } f();");
}

public void testPreservesCastInformation() {
// Only set the suffix instead of both prefix and suffix, because j2cl pass
// looks for that exact suffix, and IntegrationTestCase adds an input
// id number between prefix and suffix.
inputFileNameSuffix = "vmbootstrap/Arrays.impl.java.js";
CompilerOptions options = new CompilerOptions();
String code = LINE_JOINER.join(
"/** @constructor */",
"var Arrays = function() {};",
"Arrays.$create = function() { return {}; }",
"/** @constructor */",
"function Foo() { this.myprop = 1; }",
"/** @constructor */",
"function Bar() { this.myprop = 2; }",
"var x = /** @type {!Foo} */ (Arrays.$create()).myprop;");

options.setCheckTypes(true);
options.setJ2clPass(J2clPassMode.TRUE);
options.setDisambiguateProperties(true);

test(options, code,
LINE_JOINER.join(
"/** @constructor */",
"var Arrays = function() {};",
"Arrays.$create = function() { return {}; }",
"/** @constructor */",
"function Foo() { this.Foo$myprop = 1; }",
"/** @constructor */",
"function Bar() { this.Bar$myprop = 2; }",
"var x = {}.Foo$myprop;"));
}

public void testInlineFunctions() {
CompilerOptions options = createCompilerOptions();
String code = "function f() { return 3; } f(); ";
Expand Down
46 changes: 1 addition & 45 deletions test/com/google/javascript/jscomp/NodeUtilTest.java
Expand Up @@ -25,8 +25,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
import com.google.javascript.jscomp.type.ReverseAbstractInterpreter;
import com.google.javascript.jscomp.type.SemanticReverseAbstractInterpreter;
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.JSTypeExpression;
import com.google.javascript.rhino.Node;
Expand All @@ -42,29 +40,15 @@
*/
public final class NodeUtilTest extends TestCase {

private static Node parse(String js, boolean typeCheck) {
private static Node parse(String js) {
Compiler compiler = new Compiler();
compiler.initCompilerOptionsIfTesting();
compiler.getOptions().setLanguageIn(LanguageMode.ECMASCRIPT6);
Node n = compiler.parseTestCode(js);
if (typeCheck) {
ReverseAbstractInterpreter rai =
new SemanticReverseAbstractInterpreter(compiler.getTypeRegistry());
new TypeCheck(compiler, rai, compiler.getTypeRegistry())
.processForTesting(n.getFirstChild(), n.getLastChild());
}
assertThat(compiler.getErrors()).isEmpty();
return n;
}

private static Node parse(String js) {
return parse(js, false);
}

private static Node parseAndTypeCheck(String js) {
return parse(js, true);
}

static Node getNode(String js) {
Node root = parse("var a=(" + js + ");");
Node expr = root.getFirstChild();
Expand Down Expand Up @@ -2459,34 +2443,6 @@ public void testIsObjectDefinePropertyDefinition() {
getCallNode("Object.defineProperty();")));
}

public void testWasCasted() {
assertFalse(NodeUtil.wasCasted(getCallNode(parseAndTypeCheck("var x = foo();"))));
assertTrue(
NodeUtil.wasCasted(
getCallNode(parseAndTypeCheck("var x = /** @type {string} */ (foo());"))));
}

public void testMaybePropagateCastTo() {
Node ast = parseAndTypeCheck("var x = /** @type {string} */ (foo());");
Node x = getNameNode(ast, "x");
Node call = getCallNode(ast);

assertFalse(NodeUtil.wasCasted(x));
NodeUtil.maybePropagateCast(call, x);
assertTrue(NodeUtil.wasCasted(x));
}

public void testMaybePropagateCastTo_noExistingCast() {
Node ast = parseAndTypeCheck("var x = foo();");
Node x = getNameNode(ast, "x");
Node call = getCallNode(ast);

assertFalse(NodeUtil.wasCasted(x));
NodeUtil.maybePropagateCast(call, x);
assertFalse(NodeUtil.wasCasted(x));
}


private boolean executedOnceTestCase(String code) {
Node ast = parse(code);
Node nameNode = getNameNode(ast, "x");
Expand Down

0 comments on commit e02df6b

Please sign in to comment.