From 604bcd93202e866cad16b74ea0640f490be9548f Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sun, 15 May 2016 10:30:11 +0200 Subject: [PATCH] painless: Make field stores not box; use GeneratorAdapter.invokeDynmaic for consistency with other method calls --- .../elasticsearch/painless/node/ADefLink.java | 39 +++++++++++++++++++ .../elasticsearch/painless/node/EChain.java | 13 ++++++- .../elasticsearch/painless/node/LCall.java | 5 +-- .../painless/node/LDefArray.java | 11 ++++-- .../elasticsearch/painless/node/LDefCall.java | 4 +- .../painless/node/LDefField.java | 11 ++++-- 6 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ADefLink.java diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ADefLink.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ADefLink.java new file mode 100644 index 00000000000..ad0beb23cbc --- /dev/null +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ADefLink.java @@ -0,0 +1,39 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you 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.elasticsearch.painless.node; + +import org.elasticsearch.painless.Definition.Type; + +/** + * The superclass for all LDef* (link) nodes that store or return a DEF. (Internal only.) + */ +abstract class ADefLink extends ALink { + + /** + * The type of the original type that was pushed on stack, set by {@link EChain} during analyze. + * This value is only used for writing the 'store' bytecode, otherwise ignored. + */ + Type storeValueType = null; + + ADefLink(final int line, final String location, final int size) { + super(line, location, size); + } + +} diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java index bc3c326f651..4cfb35e48a8 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EChain.java @@ -222,8 +222,17 @@ public final class EChain extends AExpression { private void analyzeWrite(final CompilerSettings settings, final Definition definition, final Variables variables) { final ALink last = links.get(links.size() - 1); - expression.expected = last.after; - expression.analyze(settings, definition, variables); + // If the store node is a DEF node, we remove the cast to DEF from the expression + // and promote the real type to it: + if (last instanceof ADefLink) { + final ADefLink lastDef = (ADefLink) last; + expression.analyze(settings, definition, variables); + lastDef.storeValueType = expression.expected = expression.actual; + } else { + // otherwise we adapt the type of the expression to the store type + expression.expected = last.after; + expression.analyze(settings, definition, variables); + } expression = expression.cast(settings, definition, variables); statement = true; diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LCall.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LCall.java index acb6646614a..ee9a996ac22 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LCall.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LCall.java @@ -59,9 +59,6 @@ public final class LCall extends ALink { method = statik ? struct.functions.get(name) : struct.methods.get(name); if (method != null) { - final Definition.Type[] types = new Definition.Type[method.arguments.size()]; - method.arguments.toArray(types); - if (method.arguments.size() != arguments.size()) { throw new IllegalArgumentException(error("When calling [" + name + "] on type [" + struct.name + "]" + " expected [" + method.arguments.size() + "] arguments, but found [" + arguments.size() + "].")); @@ -70,7 +67,7 @@ public final class LCall extends ALink { for (int argument = 0; argument < arguments.size(); ++argument) { final AExpression expression = arguments.get(argument); - expression.expected = types[argument]; + expression.expected = method.arguments.get(argument); expression.analyze(settings, definition, variables); arguments.set(argument, expression.cast(settings, definition, variables)); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefArray.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefArray.java index 38e8bf89cc9..d3d129d3ba9 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefArray.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefArray.java @@ -31,7 +31,7 @@ import static org.elasticsearch.painless.WriterConstants.DEF_BOOTSTRAP_HANDLE; /** * Represents an array load/store or shortcut on a def type. (Internal only.) */ -final class LDefArray extends ALink { +final class LDefArray extends ADefLink { AExpression index; @@ -61,13 +61,16 @@ final class LDefArray extends ALink { @Override void load(final CompilerSettings settings, final Definition definition, final GeneratorAdapter adapter) { final String desc = Type.getMethodDescriptor(after.type, definition.defType.type, index.actual.type); - adapter.visitInvokeDynamicInsn("arrayLoad", desc, DEF_BOOTSTRAP_HANDLE, DefBootstrap.ARRAY_LOAD); + adapter.invokeDynamic("arrayLoad", desc, DEF_BOOTSTRAP_HANDLE, DefBootstrap.ARRAY_LOAD); } @Override void store(final CompilerSettings settings, final Definition definition, final GeneratorAdapter adapter) { + if (storeValueType == null) { + throw new IllegalStateException(error("Illegal tree structure.")); + } final String desc = Type.getMethodDescriptor(definition.voidType.type, definition.defType.type, - index.actual.type, definition.defType.type); - adapter.visitInvokeDynamicInsn("arrayStore", desc, DEF_BOOTSTRAP_HANDLE, DefBootstrap.ARRAY_STORE); + index.actual.type, storeValueType.type); + adapter.invokeDynamic("arrayStore", desc, DEF_BOOTSTRAP_HANDLE, DefBootstrap.ARRAY_STORE); } } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefCall.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefCall.java index 2bf53bcf89b..459f7221b68 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefCall.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefCall.java @@ -32,7 +32,7 @@ import static org.elasticsearch.painless.WriterConstants.DEF_BOOTSTRAP_HANDLE; /** * Represents a method call made on a def type. (Internal only.) */ -final class LDefCall extends ALink { +final class LDefCall extends ADefLink { final String name; final List arguments; @@ -84,7 +84,7 @@ final class LDefCall extends ALink { // return value signature.append(after.type.getDescriptor()); - adapter.visitInvokeDynamicInsn(name, signature.toString(), DEF_BOOTSTRAP_HANDLE, DefBootstrap.METHOD_CALL); + adapter.invokeDynamic(name, signature.toString(), DEF_BOOTSTRAP_HANDLE, DefBootstrap.METHOD_CALL); } @Override diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefField.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefField.java index 68fb7ffb95a..caf78dccdd1 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefField.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/LDefField.java @@ -31,7 +31,7 @@ import static org.elasticsearch.painless.WriterConstants.DEF_BOOTSTRAP_HANDLE; /** * Represents a field load/store or shortcut on a def type. (Internal only.) */ -final class LDefField extends ALink { +final class LDefField extends ADefLink { final String value; @@ -57,12 +57,15 @@ final class LDefField extends ALink { @Override void load(final CompilerSettings settings, final Definition definition, final GeneratorAdapter adapter) { final String desc = Type.getMethodDescriptor(after.type, definition.defType.type); - adapter.visitInvokeDynamicInsn(value, desc, DEF_BOOTSTRAP_HANDLE, DefBootstrap.LOAD); + adapter.invokeDynamic(value, desc, DEF_BOOTSTRAP_HANDLE, DefBootstrap.LOAD); } @Override void store(final CompilerSettings settings, final Definition definition, final GeneratorAdapter adapter) { - final String desc = Type.getMethodDescriptor(definition.voidType.type, definition.defType.type, definition.defType.type); - adapter.visitInvokeDynamicInsn(value, desc, DEF_BOOTSTRAP_HANDLE, DefBootstrap.STORE); + if (storeValueType == null) { + throw new IllegalStateException(error("Illegal tree structure.")); + } + final String desc = Type.getMethodDescriptor(definition.voidType.type, definition.defType.type, storeValueType.type); + adapter.invokeDynamic(value, desc, DEF_BOOTSTRAP_HANDLE, DefBootstrap.STORE); } }