Remove JsInliner limitation to keep field accesses.
JsInliner avoids removal of unused expression that are purely
de-references. Our guess is, it was working this way to preserve
any potential NPE on dereferences. However java optimizations
already not trying to preserve such NPEs so we don't find any
reason to have it in js optimizations.
Change-Id: I055a83019fff17b4358a107e01eaa1d1bb0a193e
Review-Link: https://gwt-review.googlesource.com/#/c/13521/
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JFieldRef.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JFieldRef.java
index 4743a78..0c294d9 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JFieldRef.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JFieldRef.java
@@ -91,11 +91,7 @@
if (hasClinit()) {
return true;
}
- JExpression expr = instance;
- if (expr == null) {
- return false;
- }
- return expr.hasSideEffects();
+ return instance != null && instance.hasSideEffects();
}
/**
diff --git a/dev/core/src/com/google/gwt/dev/js/JsInliner.java b/dev/core/src/com/google/gwt/dev/js/JsInliner.java
index 4492b70..11483a5 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsInliner.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsInliner.java
@@ -641,12 +641,9 @@
*/
private JsFunction programFunction;
- private JsProgram program;
-
private final Set<JsName> safeToInlineAtTopLevel;
public InliningVisitor(JsProgram program, Set<JsNode> whitelist) {
- this.program = program;
this.whitelist = whitelist;
invocationCountingVisitor.accept(program);
JsName defineClass = getFunctionName(program, "JavaClassHierarchySetupUtil.defineClass");
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsArrayAccess.java b/dev/core/src/com/google/gwt/dev/js/ast/JsArrayAccess.java
index b4c039e..ebef970 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsArrayAccess.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsArrayAccess.java
@@ -53,11 +53,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return false;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsArrayLiteral.java b/dev/core/src/com/google/gwt/dev/js/ast/JsArrayLiteral.java
index eb3b219..fc3313b 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsArrayLiteral.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsArrayLiteral.java
@@ -77,11 +77,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperation.java b/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperation.java
index d4e6ff5..5933b84 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperation.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsBinaryOperation.java
@@ -61,37 +61,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- // Precarious coding, but none of these can have null results.
- if (op.getPrecedence() > 5) {
- return true;
- }
- if (op == JsBinaryOperator.OR) {
- if (arg1 instanceof CanBooleanEval) {
- if (((CanBooleanEval) arg1).isBooleanTrue()) {
- assert arg1.isDefinitelyNotNull();
- return true;
- }
- }
- }
- // AND and OR can return nulls
- if (op.isAssignment()) {
- if (op == JsBinaryOperator.ASG) {
- return arg2.isDefinitelyNotNull();
- } else {
- // All other ASG's are math ops.
- return true;
- }
- }
-
- if (op == JsBinaryOperator.COMMA) {
- return arg2.isDefinitelyNotNull();
- }
-
- return false;
- }
-
- @Override
public boolean isDefinitelyNull() {
if (op == JsBinaryOperator.AND) {
return arg1.isDefinitelyNull();
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsBooleanLiteral.java b/dev/core/src/com/google/gwt/dev/js/ast/JsBooleanLiteral.java
index f864486..4d9d6dc 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsBooleanLiteral.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsBooleanLiteral.java
@@ -56,11 +56,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsConditional.java b/dev/core/src/com/google/gwt/dev/js/ast/JsConditional.java
index fc69d29..059c84f 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsConditional.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsConditional.java
@@ -61,11 +61,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return thenExpr.isDefinitelyNotNull() && elseExpr.isDefinitelyNotNull();
- }
-
- @Override
public boolean isDefinitelyNull() {
return thenExpr.isDefinitelyNull() && elseExpr.isDefinitelyNull();
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsExpression.java b/dev/core/src/com/google/gwt/dev/js/ast/JsExpression.java
index 1af822a..d759b24 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsExpression.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsExpression.java
@@ -30,11 +30,6 @@
public abstract boolean hasSideEffects();
/**
- * True if the target expression is definitely not null.
- */
- public abstract boolean isDefinitelyNotNull();
-
- /**
* True if the target expression is definitely null.
*/
public abstract boolean isDefinitelyNull();
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsFunction.java b/dev/core/src/com/google/gwt/dev/js/ast/JsFunction.java
index f2fc5ca..1b513e9 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsFunction.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsFunction.java
@@ -138,11 +138,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsInvocation.java b/dev/core/src/com/google/gwt/dev/js/ast/JsInvocation.java
index 93e1d21..b400016 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsInvocation.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsInvocation.java
@@ -73,11 +73,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return false;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsNameOf.java b/dev/core/src/com/google/gwt/dev/js/ast/JsNameOf.java
index 0e3dd2c..6bddb75 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsNameOf.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsNameOf.java
@@ -41,12 +41,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- // GenerateJsAST would have already replaced unnamed references with null
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java b/dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java
index 797ce6b..32d9619 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java
@@ -59,14 +59,7 @@
@Override
public boolean hasSideEffects() {
- if (qualifier == null) {
- return false;
- }
- if (!qualifier.isDefinitelyNotNull()) {
- // Could trigger NPE.
- return true;
- }
- return qualifier.hasSideEffects();
+ return qualifier != null && qualifier.hasSideEffects();
}
@Override
@@ -80,12 +73,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- // TODO: look for single-assignment of stuff from Java?
- return false;
- }
-
- @Override
public boolean isDefinitelyNull() {
return name == JsRootScope.INSTANCE.getUndefined();
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsNew.java b/dev/core/src/com/google/gwt/dev/js/ast/JsNew.java
index 495884e..0fc6edf 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsNew.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsNew.java
@@ -52,13 +52,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- // Sadly, in JS it can be!
- // TODO: analysis could probably determine most instances cannot be null.
- return false;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsNullLiteral.java b/dev/core/src/com/google/gwt/dev/js/ast/JsNullLiteral.java
index bb3db23..c5a7a3b 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsNullLiteral.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsNullLiteral.java
@@ -43,11 +43,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return false;
- }
-
- @Override
public boolean isDefinitelyNull() {
return true;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsNumberLiteral.java b/dev/core/src/com/google/gwt/dev/js/ast/JsNumberLiteral.java
index e61650d..edc2b00 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsNumberLiteral.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsNumberLiteral.java
@@ -60,11 +60,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsNumericEntry.java b/dev/core/src/com/google/gwt/dev/js/ast/JsNumericEntry.java
index d7d509d..a831fe0 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsNumericEntry.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsNumericEntry.java
@@ -49,11 +49,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsObjectLiteral.java b/dev/core/src/com/google/gwt/dev/js/ast/JsObjectLiteral.java
index ab1360b..f60c448 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsObjectLiteral.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsObjectLiteral.java
@@ -84,11 +84,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsPostfixOperation.java b/dev/core/src/com/google/gwt/dev/js/ast/JsPostfixOperation.java
index 72fda97..95066e8 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsPostfixOperation.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsPostfixOperation.java
@@ -34,11 +34,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsPrefixOperation.java b/dev/core/src/com/google/gwt/dev/js/ast/JsPrefixOperation.java
index 5e31e13..14e3e8a 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsPrefixOperation.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsPrefixOperation.java
@@ -58,14 +58,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- if (getOperator() == JsUnaryOperator.TYPEOF) {
- return true;
- }
- return getOperator() != JsUnaryOperator.VOID;
- }
-
- @Override
public boolean isDefinitelyNull() {
return getOperator() == JsUnaryOperator.VOID;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsRegExp.java b/dev/core/src/com/google/gwt/dev/js/ast/JsRegExp.java
index 5a5be32..3c27100 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsRegExp.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsRegExp.java
@@ -70,11 +70,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsStringLiteral.java b/dev/core/src/com/google/gwt/dev/js/ast/JsStringLiteral.java
index 9cfee40..df9a9d6 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsStringLiteral.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsStringLiteral.java
@@ -62,11 +62,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsThisRef.java b/dev/core/src/com/google/gwt/dev/js/ast/JsThisRef.java
index 6ae98b7..0dd92f9 100644
--- a/dev/core/src/com/google/gwt/dev/js/ast/JsThisRef.java
+++ b/dev/core/src/com/google/gwt/dev/js/ast/JsThisRef.java
@@ -40,15 +40,6 @@
}
@Override
- public boolean isDefinitelyNotNull() {
- /*
- * You'd think that you could get a null this via function.call/apply, but in fact you can't:
- * they just make this be the window object instead. So it really can't ever be null.
- */
- return true;
- }
-
- @Override
public boolean isDefinitelyNull() {
return false;
}
diff --git a/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java b/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
index a88b7e6..52fdba3 100644
--- a/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/JsInlinerTest.java
@@ -113,9 +113,9 @@
// Inline a devirtualized getter
input = Joiner.on('\n').join(
"function getP(t) { return t.a; }",
- "function b1(o) { getP(o) == getP(o); } b1({});");
+ "function b1(o) { return getP(o) == getP(o); } b1({});");
expected = Joiner.on('\n').join(
- "function b1(o){o.a==o.a;}",
+ "function b1(o){return o.a==o.a;}",
"b1({});");
verifyOptimized(expected, input);