From c1cddc5a71bc2ee5e45e2a8fa5a4e69f0efbb20f Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Mon, 23 Jan 2017 23:10:35 +0300 Subject: [PATCH] Add JSByRef annotation to specify which parameters to pass to JS by reference --- .../src/main/java/org/teavm/jso/JSBody.java | 2 +- .../src/main/java/org/teavm/jso/JSByRef.java | 31 +++++ .../src/main/java/org/teavm/jso/impl/JS.java | 3 + .../org/teavm/jso/impl/JSClassProcessor.java | 128 ++++++++++-------- .../org/teavm/jso/impl/JSNativeGenerator.java | 8 +- .../java/org/teavm/jso/impl/JSTypeHelper.java | 43 +++--- .../org/teavm/jso/test/ConversionTest.java | 36 ++++- 7 files changed, 175 insertions(+), 76 deletions(-) create mode 100644 jso/core/src/main/java/org/teavm/jso/JSByRef.java diff --git a/jso/core/src/main/java/org/teavm/jso/JSBody.java b/jso/core/src/main/java/org/teavm/jso/JSBody.java index fae89add3..862e91485 100644 --- a/jso/core/src/main/java/org/teavm/jso/JSBody.java +++ b/jso/core/src/main/java/org/teavm/jso/JSBody.java @@ -127,7 +127,7 @@ public @interface JSBody { /** *

How method parameters are named inside JavaScript implementation.

*/ - String[] params(); + String[] params() default {}; /** *

JavaScript code.

diff --git a/jso/core/src/main/java/org/teavm/jso/JSByRef.java b/jso/core/src/main/java/org/teavm/jso/JSByRef.java new file mode 100644 index 000000000..9c43d264b --- /dev/null +++ b/jso/core/src/main/java/org/teavm/jso/JSByRef.java @@ -0,0 +1,31 @@ +/* + * Copyright 2017 Alexey Andreev. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.teavm.jso; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + *

Marks parameters of JavaScript methods that should be passed by reference. + * This annotation is only applicable to parameters of array type. More specifically: + * to: byte[], short[], char[], int[], float[], double[] or T[], where T is JSObject.

+ */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.PARAMETER) +public @interface JSByRef { +} diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JS.java b/jso/impl/src/main/java/org/teavm/jso/impl/JS.java index 0f15264da..718bd8476 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JS.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JS.java @@ -32,6 +32,9 @@ final class JS { private JS() { } + @InjectedBy(JSNativeGenerator.class) + public static native JSObject arrayData(Object array); + @InjectedBy(JSNativeGenerator.class) public static native JSObject wrap(byte value); diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java index 28a4cc62b..d36d3d3a6 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Function; import org.mozilla.javascript.CompilerEnvirons; @@ -36,6 +37,7 @@ import org.teavm.cache.NoCache; import org.teavm.diagnostics.Diagnostics; import org.teavm.interop.Sync; import org.teavm.jso.JSBody; +import org.teavm.jso.JSByRef; import org.teavm.jso.JSFunctor; import org.teavm.jso.JSIndexer; import org.teavm.jso.JSMethod; @@ -44,6 +46,7 @@ import org.teavm.jso.JSProperty; import org.teavm.jso.core.JSArray; import org.teavm.jso.core.JSArrayReader; import org.teavm.model.AccessLevel; +import org.teavm.model.AnnotationContainerReader; import org.teavm.model.AnnotationHolder; import org.teavm.model.AnnotationReader; import org.teavm.model.AnnotationValue; @@ -180,22 +183,22 @@ class JSClassProcessor { } } - private MethodReader getOverridenMethod(MethodReader finalMethod) { + private MethodReader getOverriddenMethod(MethodReader finalMethod) { MethodReference ref = finalMethod.getReference(); if (!overridenMethodCache.containsKey(ref)) { - overridenMethodCache.put(ref, findOverridenMethod(finalMethod.getOwnerName(), finalMethod)); + overridenMethodCache.put(ref, findOverriddenMethod(finalMethod.getOwnerName(), finalMethod)); } return overridenMethodCache.get(ref); } - private MethodReader findOverridenMethod(String className, MethodReader finalMethod) { + private MethodReader findOverriddenMethod(String className, MethodReader finalMethod) { if (finalMethod.getName().equals("")) { return null; } return classSource.getAncestors(className) .skip(1) .map(cls -> cls.getMethod(finalMethod.getDescriptor())) - .filter(method -> method != null) + .filter(Objects::nonNull) .findFirst() .orElse(null); } @@ -302,7 +305,7 @@ class JSClassProcessor { } if (method.getProgram() != null && method.getProgram().basicBlockCount() > 0) { - MethodReader overridden = getOverridenMethod(method); + MethodReader overridden = getOverriddenMethod(method); if (overridden != null) { diagnostics.error(callLocation, "JS final method {{m0}} overrides {{m1}}. " + "Overriding final method of overlay types is prohibited.", @@ -329,16 +332,8 @@ class JSClassProcessor { private boolean processJSBodyInvocation(MethodReader method, CallLocation callLocation, InvokeInstruction invoke, MethodHolder methodToProcess) { - boolean valid = true; - for (int i = 0; i < method.parameterCount(); ++i) { - ValueType arg = method.parameterType(i); - if (!typeHelper.isSupportedType(arg)) { - diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript method " - + " declaration. Its parameter #" + (i + 1) + " has invalid type {{t1}}", invoke.getMethod(), - arg); - valid = false; - } - } + boolean[] byRefParams = new boolean[method.parameterCount()]; + boolean valid = validateSignature(method, callLocation, byRefParams); if (invoke.getInstance() != null) { if (!typeHelper.isSupportedType(ValueType.object(method.getOwnerName()))) { diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript method " @@ -347,15 +342,6 @@ class JSClassProcessor { valid = false; } } - if (method.getResultType() != ValueType.VOID && !typeHelper.isSupportedType(method.getResultType())) { - diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript method " - + " declaration, since it returns invalid type {{t1}}", invoke.getMethod(), - method.getResultType()); - valid = false; - } - if (!valid) { - return false; - } requireJSBody(diagnostics, method); MethodReference delegate = repository.methodMap.get(method.getReference()); @@ -372,12 +358,13 @@ class JSClassProcessor { newInvoke.setReceiver(result); newInvoke.setLocation(invoke.getLocation()); if (invoke.getInstance() != null) { - Variable arg = wrapArgument(callLocation, invoke.getInstance(), ValueType.object(method.getOwnerName())); + Variable arg = wrapArgument(callLocation, invoke.getInstance(), + ValueType.object(method.getOwnerName()), false); newInvoke.getArguments().add(arg); } - for (int k = 0; k < invoke.getArguments().size(); ++k) { - Variable arg = wrapArgument(callLocation, invoke.getArguments().get(k), - method.parameterType(k)); + for (int i = 0; i < invoke.getArguments().size(); ++i) { + Variable arg = wrapArgument(callLocation, invoke.getArguments().get(i), + method.parameterType(i), byRefParams[i]); newInvoke.getArguments().add(arg); } replacement.add(newInvoke); @@ -420,7 +407,7 @@ class JSClassProcessor { propertyName = cutPrefix(method.getName(), 3); } Variable wrapped = wrapArgument(callLocation, invoke.getArguments().get(0), - method.parameterType(0)); + method.parameterType(0), false); addPropertySet(propertyName, invoke.getInstance(), wrapped, invoke.getLocation()); return true; } @@ -433,7 +420,7 @@ class JSClassProcessor { if (isProperGetIndexer(method.getDescriptor())) { Variable result = invoke.getReceiver() != null ? program.createVariable() : null; addIndexerGet(invoke.getInstance(), wrap(invoke.getArguments().get(0), - method.parameterType(0), invoke.getLocation()), result, invoke.getLocation()); + method.parameterType(0), invoke.getLocation(), false), result, invoke.getLocation()); if (result != null) { result = unwrap(callLocation, result, method.getResultType()); copyVar(result, invoke.getReceiver(), invoke.getLocation()); @@ -442,9 +429,9 @@ class JSClassProcessor { } if (isProperSetIndexer(method.getDescriptor())) { Variable index = wrap(invoke.getArguments().get(0), method.parameterType(0), - invoke.getLocation()); + invoke.getLocation(), false); Variable value = wrap(invoke.getArguments().get(1), method.parameterType(1), - invoke.getLocation()); + invoke.getLocation(), false); addIndexerSet(invoke.getInstance(), index, value, invoke.getLocation()); return true; } @@ -453,6 +440,36 @@ class JSClassProcessor { return false; } + private boolean validateSignature(MethodReader method, CallLocation callLocation, boolean[] byRefParams) { + if (method.getResultType() != ValueType.VOID && !typeHelper.isSupportedType(method.getResultType())) { + diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript method " + + "declaration", method.getReference()); + return false; + } + + ValueType[] parameterTypes = method.getParameterTypes(); + AnnotationContainerReader[] parameterAnnotations = method.getParameterAnnotations(); + for (int i = 0; i < parameterTypes.length; i++) { + ValueType paramType = parameterTypes[i]; + if (!typeHelper.isSupportedType(paramType)) { + diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript method " + + "declaration: its " + (i + 1) + "th argument has wrong type", method.getReference()); + return false; + } + if (parameterAnnotations[i].get(JSByRef.class.getName()) != null) { + if (!typeHelper.isSupportedByRefType(paramType)) { + diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript method " + + "declaration: its " + (i + 1) + "th argument is declared as JSByRef, " + + "which has incompatible type", method.getReference()); + return false; + } + byRefParams[i] = true; + } + } + + return true; + } + private boolean processMethod(MethodReader method, CallLocation callLocation, InvokeInstruction invoke) { String name = method.getName(); @@ -463,18 +480,10 @@ class JSClassProcessor { name = redefinedMethodName.getString(); } } - if (method.getResultType() != ValueType.VOID && !typeHelper.isSupportedType(method.getResultType())) { - diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript method " - + "declaration", invoke.getMethod()); - return false; - } - for (ValueType arg : method.getParameterTypes()) { - if (!typeHelper.isSupportedType(arg)) { - diagnostics.error(callLocation, "Method {{m0}} is not a proper native JavaScript method " - + " declaration", invoke.getMethod()); - return false; - } + boolean[] byRefParams = new boolean[method.parameterCount() + 1]; + if (!validateSignature(method, callLocation, byRefParams)) { + return false; } Variable result = invoke.getReceiver() != null ? program.createVariable() : null; @@ -488,9 +497,9 @@ class JSClassProcessor { newInvoke.getArguments().add(addStringWrap(addString(name, invoke.getLocation()), invoke.getLocation())); newInvoke.setLocation(invoke.getLocation()); - for (int k = 0; k < invoke.getArguments().size(); ++k) { - Variable arg = wrapArgument(callLocation, invoke.getArguments().get(k), - method.parameterType(k)); + for (int i = 0; i < invoke.getArguments().size(); ++i) { + Variable arg = wrapArgument(callLocation, invoke.getArguments().get(i), + method.parameterType(i), byRefParams[i]); newInvoke.getArguments().add(arg); } replacement.add(newInvoke); @@ -515,7 +524,8 @@ class JSClassProcessor { // validate parameter names AnnotationReader bodyAnnot = methodToProcess.getAnnotations().get(JSBody.class.getName()); - int jsParamCount = bodyAnnot.getValue("params").getList().size(); + AnnotationValue paramsValue = bodyAnnot.getValue("params"); + int jsParamCount = paramsValue != null ? paramsValue.getList().size() : 0; if (methodToProcess.parameterCount() != jsParamCount) { diagnostics.error(location, "JSBody method {{m0}} declares " + methodToProcess.parameterCount() + " parameters, but annotation specifies " + jsParamCount, methodToProcess.getReference()); @@ -553,9 +563,9 @@ class JSClassProcessor { MethodReference proxyMethod = new MethodReference(methodToProcess.getOwnerName(), methodToProcess.getName() + "$js_body$_" + methodIndexGenerator++, proxyParamTypes); String script = bodyAnnot.getValue("script").getString(); - String[] parameterNames = bodyAnnot.getValue("params").getList().stream() + String[] parameterNames = paramsValue != null ? paramsValue.getList().stream() .map(AnnotationValue::getString) - .toArray(String[]::new); + .toArray(String[]::new) : new String[0]; // Parse JS script TeaVMErrorReporter errorReporter = new TeaVMErrorReporter(diagnostics, @@ -657,7 +667,7 @@ class JSClassProcessor { ExitInstruction exit = new ExitInstruction(); if (insn.getReceiver() != null) { replacement.clear(); - exit.setValueToReturn(wrap(insn.getReceiver(), callee.getResultType(), null)); + exit.setValueToReturn(wrap(insn.getReceiver(), callee.getResultType(), null, false)); block.addAll(replacement); } block.add(exit); @@ -725,7 +735,7 @@ class JSClassProcessor { } private Variable addStringWrap(Variable var, TextLocation location) { - return wrap(var, ValueType.object("java.lang.String"), location); + return wrap(var, ValueType.object("java.lang.String"), location, false); } private Variable addString(String str, TextLocation location) { @@ -972,7 +982,7 @@ class JSClassProcessor { return result; } - private Variable wrapArgument(CallLocation location, Variable var, ValueType type) { + private Variable wrapArgument(CallLocation location, Variable var, ValueType type, boolean byRef) { if (type instanceof ValueType.Object) { String className = ((ValueType.Object) type).getClassName(); ClassReader cls = classSource.get(className); @@ -980,7 +990,7 @@ class JSClassProcessor { return wrapFunctor(location, var, cls); } } - return wrap(var, type, location.getSourceLocation()); + return wrap(var, type, location.getSourceLocation(), byRef); } private boolean isProperFunctor(ClassReader type) { @@ -1006,7 +1016,17 @@ class JSClassProcessor { return functor; } - private Variable wrap(Variable var, ValueType type, TextLocation location) { + private Variable wrap(Variable var, ValueType type, TextLocation location, boolean byRef) { + if (byRef) { + InvokeInstruction insn = new InvokeInstruction(); + insn.setMethod(new MethodReference(JS.class, "arrayData", Object.class, JSObject.class)); + insn.setReceiver(program.createVariable()); + insn.setType(InvocationType.SPECIAL); + insn.getArguments().add(var); + replacement.add(insn); + return insn.getReceiver(); + } + if (type instanceof ValueType.Object) { String className = ((ValueType.Object) type).getClassName(); if (!className.equals("java.lang.String")) { diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSNativeGenerator.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSNativeGenerator.java index 02b9dbdea..cf5bab807 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSNativeGenerator.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSNativeGenerator.java @@ -35,10 +35,6 @@ import org.teavm.model.MethodReader; import org.teavm.model.MethodReference; import org.teavm.model.ValueType; -/** - * - * @author Alexey Andreev - */ public class JSNativeGenerator implements Injector, DependencyPlugin, Generator { @Override public void generate(GeneratorContext context, SourceWriter writer, MethodReference methodRef) @@ -76,6 +72,10 @@ public class JSNativeGenerator implements Injector, DependencyPlugin, Generator public void generate(InjectorContext context, MethodReference methodRef) throws IOException { SourceWriter writer = context.getWriter(); switch (methodRef.getName()) { + case "arrayData": + context.writeExpr(context.getArgument(0)); + writer.append(".data"); + break; case "get": context.writeExpr(context.getArgument(0), Precedence.MEMBER_ACCESS); renderProperty(context.getArgument(1), context); diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSTypeHelper.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSTypeHelper.java index ebe8c91fd..c6690d93c 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSTypeHelper.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSTypeHelper.java @@ -23,10 +23,6 @@ import org.teavm.model.ClassReaderSource; import org.teavm.model.ElementModifier; import org.teavm.model.ValueType; -/** - * - * @author Alexey Andreev - */ class JSTypeHelper { private ClassReaderSource classSource; private Map knownJavaScriptClasses = new HashMap<>(); @@ -38,21 +34,12 @@ class JSTypeHelper { } public boolean isJavaScriptClass(String className) { - Boolean known = knownJavaScriptClasses.get(className); - if (known == null) { - known = examineIfJavaScriptClass(className); - knownJavaScriptClasses.put(className, known); - } - return known; + return knownJavaScriptClasses.computeIfAbsent(className, k -> examineIfJavaScriptClass(className)); } public boolean isJavaScriptImplementation(String className) { - Boolean known = knownJavaScriptImplementations.get(className); - if (known == null) { - known = examineIfJavaScriptImplementation(className); - knownJavaScriptImplementations.put(className, known); - } - return known; + return knownJavaScriptImplementations + .computeIfAbsent(className, k -> examineIfJavaScriptImplementation(className)); } private boolean examineIfJavaScriptClass(String className) { @@ -104,4 +91,28 @@ class JSTypeHelper { return false; } } + + public boolean isSupportedByRefType(ValueType type) { + if (!(type instanceof ValueType.Array)) { + return false; + } + ValueType itemType = ((ValueType.Array) type).getItemType(); + if (itemType instanceof ValueType.Primitive) { + switch (((ValueType.Primitive) itemType).getKind()) { + case BYTE: + case SHORT: + case CHARACTER: + case INTEGER: + case FLOAT: + case DOUBLE: + return true; + default: + return false; + } + } else if (itemType instanceof ValueType.Object) { + return isJavaScriptClass(((ValueType.Object) itemType).getClassName()); + } else { + return false; + } + } } diff --git a/tests/src/test/java/org/teavm/jso/test/ConversionTest.java b/tests/src/test/java/org/teavm/jso/test/ConversionTest.java index 62b0cb17d..0d1a822f8 100644 --- a/tests/src/test/java/org/teavm/jso/test/ConversionTest.java +++ b/tests/src/test/java/org/teavm/jso/test/ConversionTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.*; import org.junit.Test; import org.junit.runner.RunWith; import org.teavm.jso.JSBody; +import org.teavm.jso.JSByRef; import org.teavm.jso.JSObject; import org.teavm.jso.JSProperty; import org.teavm.jso.core.JSString; @@ -142,13 +143,26 @@ public class ConversionTest { assertEquals(23, array[0]); } + @Test + public void passesArrayByRef() { + int[] array = { 23, 42 }; + + mutateByRef(array); + assertEquals(24, array[0]); + assertEquals(43, array[1]); + + createByRefMutator().mutate(array); + assertEquals(25, array[0]); + assertEquals(44, array[1]); + } + @JSBody(params = { "a", "b", "c", "d", "e", "f", "g", "h" }, script = "" + "return '' + a + ':' + b + ':' + c + ':' + d + ':' + e + ':' + f.toFixed(1) + ':'" + "+ g.toFixed(1) + ':' + h;") private static native String combinePrimitives(boolean a, byte b, short c, char d, int e, float f, double g, String h); - @JSBody(params = {}, script = "return { a : true, b : 2, c : 3, d : 64, e : 4, f : 5.5, g : 6.5, h : 'foo' };") + @JSBody(script = "return { a : true, b : 2, c : 3, d : 64, e : 4, f : 5.5, g : 6.5, h : 'foo' };") private static native Primitives getPrimitives(); @JSBody(params = { "a", "b", "c", "d", "e", "f", "g", "h" }, script = "" @@ -300,4 +314,24 @@ public class ConversionTest { @JSBody(params = "array", script = "array[0]++; return array[0];") private static native int mutate(int[] array); + + @JSBody(params = "array", script = "" + + "for (var i = 0; i < array.length; ++i) {" + + "array[i]++;" + + "}") + private static native void mutateByRef(@JSByRef int[] array); + + private interface ByRefMutator extends JSObject { + void mutate(@JSByRef int[] array); + } + + @JSBody(script = "" + + "return {" + + "mutate : function(array) {" + + "for (var i = 0; i < array.length; ++i) {" + + "array[i]++;" + + "}" + + "}" + + "};") + private static native ByRefMutator createByRefMutator(); }