Fix JS varargs when null array is passed.
There is no way to express this situation in JS, the arguments object
is always non null. Hence, we insert a precondition check when calling
a JS varargs method with null as the varargs parameter when necessary.
Change-Id: If63cb06a36597f4fa5c6291de36ce82bd5ae70c1
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/RuntimeConstants.java b/dev/core/src/com/google/gwt/dev/jjs/ast/RuntimeConstants.java
index 47042ba..46bd8f4 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/RuntimeConstants.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/RuntimeConstants.java
@@ -24,6 +24,7 @@
public static final String ASYNC_FRAGMENT_LOADER_ON_LOAD = "AsyncFragmentLoader.onLoad";
public static final String ASYNC_FRAGMENT_LOADER_RUN_ASYNC = "AsyncFragmentLoader.runAsync";
+ public static final String ARRAY_ENSURE_NOT_NULL = "Array.ensureNotNull";
public static final String ARRAY_GET_CLASS_LITERAL_FOR_ARRAY = "Array.getClassLiteralForArray";
public static final String ARRAY_INITIALIZE_UNIDIMENSIONAL_ARRAY = "Array.initUnidimensionalArray";
public static final String ARRAY_INITIALIZE_MULTIDIMENSIONAL_ARRAY = "Array.initMultidimensionalArray";
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementJsVarargs.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementJsVarargs.java
index 610af99..0938544 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementJsVarargs.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ImplementJsVarargs.java
@@ -43,6 +43,7 @@
import com.google.gwt.dev.jjs.ast.JUnaryOperator;
import com.google.gwt.dev.jjs.ast.JVariableRef;
import com.google.gwt.dev.jjs.ast.JVisitor;
+import com.google.gwt.dev.jjs.ast.RuntimeConstants;
import com.google.gwt.thirdparty.guava.common.collect.Iterables;
import java.util.Collections;
@@ -289,6 +290,9 @@
varargsParameter = Iterables.getLast(x.getParams());
varargsIndex = x.getParams().size() - 1;
+ // JsVarargs parameter can be assumend not null in the implementing method
+ varargsParameter.setType(varargsParameter.getType().strengthenToNonNull());
+
argumentsCopyVariable = null;
switch (needsVarargsProcessing(x)) {
case GENERAL_ACCESS:
@@ -360,6 +364,7 @@
SourceInfo sourceInfo = varargsParameter.getSourceInfo();
JBlock preamble = new JBlock(sourceInfo);
+ JArrayType varargsArrayType = (JArrayType) varargsParameter.getType().getUnderlyingType();
// (1) varargs_ = new VarArgsType[varargs.length - varArgsParameterIndex]
JExpression lengthMinusVarargsIndex = varargsIndex == 0
@@ -368,10 +373,9 @@
new JArrayLength(sourceInfo, varargsParameter.createRef(sourceInfo)),
new JIntLiteral(sourceInfo, varargsIndex));
JNewArray arrayVariable = JNewArray.createArrayWithDimensionExpressions(sourceInfo,
- (JArrayType) varargsParameter.getType(),
- Collections.singletonList(lengthMinusVarargsIndex));
+ varargsArrayType, Collections.singletonList(lengthMinusVarargsIndex));
arrayVariable.getLeafTypeClassLiteral().setField(
- program.getClassLiteralField(((JArrayType) varargsParameter.getType()).getLeafType()));
+ program.getClassLiteralField(varargsArrayType.getLeafType()));
preamble.addStmt(new JDeclarationStatement(
sourceInfo, argumentsCopyVariable.createRef(sourceInfo), arrayVariable));
@@ -385,7 +389,7 @@
JBlock block = new JBlock(sourceInfo);
block.addStmt(new JBinaryOperation(
sourceInfo,
- ((JArrayType) varargsParameter.getType()).getElementType(),
+ varargsArrayType.getElementType(),
JBinaryOperator.ASG,
new JArrayRef(sourceInfo, replacer.replace(varargsParameter.createRef(sourceInfo)),
index.createRef(sourceInfo)),
@@ -437,10 +441,20 @@
JNewArray varargArray = (JNewArray) varargArgument;
if (varargArray.getInitializers() != null) {
x.setArg(varargIndex, ArrayNormalizer.getInitializerArray(varargArray));
+ madeChanges();
return;
}
}
- // Passed as an array to varargs method.
+
+ SourceInfo varargsArgumentsourceInfo = varargArgument.getSourceInfo();
+ if (varargArgument.getType().canBeNull()) {
+ // varargsArgument => Array.ensureNotNull(varargsArgument)
+ x.setArg(varargIndex, new JMethodCall(varargsArgumentsourceInfo, null,
+ program.getIndexedMethod(RuntimeConstants.ARRAY_ENSURE_NOT_NULL), varargArgument));
+ }
+
+ // Passed as an array to varargs method will result in an apply call, in which case hoist the
+ // qualifier to make sure it is only evaluated once.
JExpression instance = x.getInstance();
if (x.getTarget().needsDynamicDispatch() && !x.isStaticDispatchOnly()
&& instance != null && !(instance instanceof JVariableRef)) {
diff --git a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Array.java b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Array.java
index 1af28b4..c16a522 100644
--- a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Array.java
+++ b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Array.java
@@ -16,6 +16,7 @@
package com.google.gwt.lang;
import static javaemul.internal.InternalPreconditions.checkArrayType;
+import static javaemul.internal.InternalPreconditions.checkNotNull;
import com.google.gwt.core.client.JavaScriptObject;
@@ -322,6 +323,10 @@
&& elementTypeCategory <= TYPE_PRIMITIVE_BOOLEAN;
};
+ public static Object ensureNotNull(Object array) {
+ return checkNotNull(array);
+ }
+
private Array() {
}
}
diff --git a/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java b/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
index c11946f..5bb4166 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
@@ -73,6 +73,7 @@
" int elementTypeId, int elementTypeCategory, Object array) { }",
" static <T> Class<T> getClassLiteralForArray() { return null; }",
" public static boolean isJavaArray(Object o) { return false; }",
+ " static Object ensureNotNull(Object o) { return null; }",
"}"
);
}
diff --git a/user/test/com/google/gwt/core/interop/JsTypeVarargsTest.java b/user/test/com/google/gwt/core/interop/JsTypeVarargsTest.java
index 82ce8b0..9ab1c19 100644
--- a/user/test/com/google/gwt/core/interop/JsTypeVarargsTest.java
+++ b/user/test/com/google/gwt/core/interop/JsTypeVarargsTest.java
@@ -20,6 +20,7 @@
import com.google.gwt.core.client.ScriptInjector;
import com.google.gwt.junit.client.GWTTestCase;
+import javaemul.internal.annotations.DoNotInline;
import jsinterop.annotations.JsFunction;
import jsinterop.annotations.JsMethod;
import jsinterop.annotations.JsPackage;
@@ -54,28 +55,50 @@
}-*/;
@JsMethod
- public static native int varargsMethod1(Object... varargs) /*-{
+ @DoNotInline
+ public static native int varargsLengthThruArguments(Object... varargs) /*-{
return arguments.length;
}-*/;
@JsMethod
- public static int varargsMethod2(Object... varargs) {
+ @DoNotInline
+ public static int varargsLength(Object... varargs) {
+ return varargs.length;
+ }
+
+ @JsMethod
+ @DoNotInline
+ public static int stringVarargsLength(String... varargs) {
+ return varargs.length;
+ }
+
+ @JsMethod
+ @DoNotInline
+ public static int stringVarargsLengthV2(int i, String... varargs) {
return varargs.length;
}
@JsMethod(namespace = JsPackage.GLOBAL)
- public static Object varargsMethod3(int slot, Object... varargs) {
+ @DoNotInline
+ public static Object getVarargsSlot(int slot, Object... varargs) {
return varargs[slot];
}
@JsMethod(namespace = JsPackage.GLOBAL)
- public static Object[] varargsMethod4(int slot, Object... varargs) {
+ @DoNotInline
+ public static Object[] clrearVarargsSlot(int slot, Object... varargs) {
varargs[slot] = null;
return varargs;
}
- private static native Object callVarargsMethod3FromJSNI() /*-{
- return $global.varargsMethod3(2, "1", "2", "3", "4");
+ @JsMethod(namespace = JsPackage.GLOBAL)
+ @DoNotInline
+ public static Class<?> getVarargsArrayClass(String... varargs) {
+ return varargs.getClass();
+ }
+
+ private static native Object callGetVarargsSlotUsingJsName() /*-{
+ return $global.getVarargsSlot(2, "1", "2", "3", "4");
}-*/;
@JsType(isNative = true, namespace = GLOBAL, name = "Object")
@@ -108,7 +131,7 @@
static class SubSubclassNativeWithVarargsConstructor
extends SubclassNativeWithVarargsConstructor {
SubSubclassNativeWithVarargsConstructor() {
- super(0, null);
+ super(0, new Object[0]);
}
Object varargsMethod(int i, Object... args) {
@@ -121,15 +144,60 @@
}
public void testVarargsCall_regularMethods() {
- assertEquals(3, varargsMethod1("A", "B", "C"));
- assertEquals(4, varargsMethod2("A", "B", "C", "D"));
- assertEquals(2, varargsMethod1(new NativeJsType[]{null, null}));
- assertEquals(5, varargsMethod2(new NativeJsType[]{null, null, null, null, null}));
- assertEquals("C", varargsMethod3(2, "A", "B", "C", "D"));
- assertEquals("3", callVarargsMethod3FromJSNI());
- assertNull(varargsMethod4(1, "A", "B", "C")[1]);
- assertEquals("A", varargsMethod4(1, "A", "B", "C")[0]);
- assertEquals(3, varargsMethod4(1, "A", "B", "C").length);
+ assertEquals(3, varargsLengthThruArguments("A", "B", "C"));
+ assertEquals(4, varargsLength("A", "B", "C", "D"));
+ assertEquals(2, varargsLengthThruArguments(new NativeJsType[]{null, null}));
+ assertEquals(5, varargsLength(new NativeJsType[]{null, null, null, null, null}));
+ assertEquals("C", getVarargsSlot(2, "A", "B", "C", "D"));
+ assertEquals("3", callGetVarargsSlotUsingJsName());
+ assertNull(clrearVarargsSlot(1, "A", "B", "C")[1]);
+ assertEquals("A", clrearVarargsSlot(1, "A", "B", "C")[0]);
+ assertEquals(3, clrearVarargsSlot(1, "A", "B", "C").length);
+ assertSame(String[].class, getVarargsArrayClass("A", "B", "C"));
+ }
+ public void testVarargsCall_edgeCases() {
+ assertSame(String[].class, getVarargsArrayClass());
+ assertSame(String[].class, getVarargsArrayClass(new String[0]));
+ assertSame(String[].class, getVarargsArrayClass((String) null));
+ try {
+ assertSame(String[].class, getVarargsArrayClass(null));
+ fail("Should have thrown exception");
+ } catch (NullPointerException expected) {
+ }
+ try {
+ assertSame(String[].class, getVarargsArrayClass((String[]) null));
+ fail("Should have thrown exception");
+ } catch (NullPointerException expected) {
+ }
+
+ assertEquals(0, stringVarargsLength());
+ assertEquals(0, stringVarargsLength(new String[0]));
+ assertEquals(1, stringVarargsLength((String) null));
+ try {
+ assertEquals(0, stringVarargsLength(null));
+ fail("Should have thrown exception");
+ } catch (NullPointerException expected) {
+ }
+ try {
+ assertEquals(0, stringVarargsLength((String[]) null));
+ fail("Should have thrown exception");
+ } catch (NullPointerException expected) {
+ }
+
+ // Test with an additional parameter as it results in a slightly different call site.
+ assertEquals(0, stringVarargsLengthV2(0));
+ assertEquals(0, stringVarargsLengthV2(0, new String[0]));
+ assertEquals(1, stringVarargsLengthV2(0, (String) null));
+ try {
+ assertEquals(0, stringVarargsLengthV2(0, null));
+ fail("Should have thrown exception");
+ } catch (NullPointerException expected) {
+ }
+ try {
+ assertEquals(0, stringVarargsLengthV2(0, (String[]) null));
+ fail("Should have thrown exception");
+ } catch (NullPointerException expected) {
+ }
}
public void testVarargsCall_constructors() {
@@ -220,7 +288,8 @@
return obj;
}
public void testVarargsCall_sideEffectingInstance() {
- SubclassNativeWithVarargsConstructor object = new SubclassNativeWithVarargsConstructor(0, null);
+ SubclassNativeWithVarargsConstructor object =
+ new SubclassNativeWithVarargsConstructor(0, new Object[0]);
sideEffectCount = 0;
Object[] params = new Object[] { object, null };
assertSame(object, doSideEffect(object).varargsMethod(0, params));
