Browse Source

Support negative array ofsets in painless

Adds support for indexing into lists and arrays with negative
indexes meaning "counting from the back". So for if
`x = ["cat", "dog", "chicken"]` then `x[-1] == "chicken"`.

This adds an extra branch to every array and list access but
some performance testing makes it look like the branch predictor
successfully predicts the branch every time so there isn't a
in execution time for this feature when the index is positive.
When the index is negative performance testing showed the runtime
is the same as writing `x[x.length - 1]`, again, presumably thanks
to the branch predictor.

Those performance metrics were calculated for lists and arrays but
`def`s get roughly the same treatment though instead of inlining
the test they need to make a invoke dynamic so we don't screw up
maps.

Closes #20870
Nik Everett 9 years ago
parent
commit
3a7a218e8f

+ 20 - 1
docs/reference/modules/scripting/painless-syntax.asciidoc

@@ -28,6 +28,23 @@ String constants can be declared with single quotes, to avoid escaping horrors w
 def mystring = 'foo';
 ---------------------------------------------------------
 
+[float]
+[[painless-arrays]]
+==== Arrays
+
+Arrays can be subscripted starting from `0` for traditional array access or with
+negative numbers to starting from the back of the array. So the following
+returns `2`.
+
+[source,painless]
+---------------------------------------------------------
+int[] x = new int[5];
+x[0]++;
+x[-5]++;
+return x[0];
+---------------------------------------------------------
+
+
 [float]
 [[painless-lists]]
 ==== List
@@ -39,11 +56,13 @@ Lists can be created explicitly (e.g. `new ArrayList()`) or initialized similar
 def list = [1,2,3];
 ---------------------------------------------------------
 
-Lists can also be accessed similar to arrays: they support subscript and `.length`:
+Lists can also be accessed similar to arrays. They support `.length` and
+subscripts, including negative subscripts to read from the back of the list:
 
 [source,painless]
 ---------------------------------------------------------
 def list = [1,2,3];
+list[-1] = 5
 return list[0]
 ---------------------------------------------------------
 

+ 86 - 0
modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java

@@ -109,6 +109,10 @@ public final class Def {
     private static final MethodHandle LIST_SET;
     /** pointer to Iterable.iterator() */
     private static final MethodHandle ITERATOR;
+    /** pointer to {@link Def#mapIndexNormalize}. */
+    private static final MethodHandle MAP_INDEX_NORMALIZE;
+    /** pointer to {@link Def#listIndexNormalize}. */
+    private static final MethodHandle LIST_INDEX_NORMALIZE;
     /** factory for arraylength MethodHandle (intrinsic) from Java 9 */
     private static final MethodHandle JAVA9_ARRAY_LENGTH_MH_FACTORY;
 
@@ -121,6 +125,10 @@ public final class Def {
             LIST_GET = lookup.findVirtual(List.class, "get", MethodType.methodType(Object.class, int.class));
             LIST_SET = lookup.findVirtual(List.class, "set", MethodType.methodType(Object.class, int.class, Object.class));
             ITERATOR = lookup.findVirtual(Iterable.class, "iterator", MethodType.methodType(Iterator.class));
+            MAP_INDEX_NORMALIZE = lookup.findStatic(Def.class, "mapIndexNormalize",
+                    MethodType.methodType(Object.class, Map.class, Object.class));
+            LIST_INDEX_NORMALIZE = lookup.findStatic(Def.class, "listIndexNormalize",
+                    MethodType.methodType(int.class, List.class, int.class));
         } catch (final ReflectiveOperationException roe) {
             throw new AssertionError(roe);
         }
@@ -522,6 +530,26 @@ public final class Def {
                                            "for class [" + receiverClass.getCanonicalName() + "].");
     }
 
+    /**
+     * Returns a method handle to normalize the index into an array. This is what makes lists and arrays stored in {@code def} support
+     * negative offsets.
+     * @param receiverClass Class of the array to store the value in
+     * @return a MethodHandle that accepts the receiver as first argument, the index as second argument, and returns the normalized index
+     *   to use with array loads and array stores
+     */
+    static MethodHandle lookupIndexNormalize(Class<?> receiverClass) {
+        if (receiverClass.isArray()) {
+            return ArrayIndexNormalizeHelper.arrayIndexNormalizer(receiverClass);
+        } else if (Map.class.isAssignableFrom(receiverClass)) {
+            // noop so that mymap[key] doesn't do funny things with negative keys
+            return MAP_INDEX_NORMALIZE;
+        } else if (List.class.isAssignableFrom(receiverClass)) {
+            return LIST_INDEX_NORMALIZE;
+        }
+        throw new IllegalArgumentException("Attempting to address a non-array-like type " +
+                                           "[" + receiverClass.getCanonicalName() + "] as an array.");
+    }
+
     /**
      * Returns a method handle to do an array store.
      * @param receiverClass Class of the array to store the value in
@@ -814,4 +842,62 @@ public final class Def {
             return ((Number)value).doubleValue();
         }
     }
+
+    /**
+     * "Normalizes" the index into a {@code Map} by making no change to the index.
+     */
+    public static Object mapIndexNormalize(final Map<?, ?> value, Object index) {
+        return index;
+    }
+
+    /**
+     * "Normalizes" the idnex into a {@code List} by flipping negative indexes around so they are "from the end" of the list.
+     */
+    public static int listIndexNormalize(final List<?> value, int index) {
+        return index >= 0 ? index : value.size() + index;
+    }
+
+    /**
+     * Methods to normalize array indices to support negative indices into arrays stored in {@code def}s.
+     */
+    @SuppressWarnings("unused") // normalizeIndex() methods are are actually used, javac just does not know :)
+    private static final class ArrayIndexNormalizeHelper {
+        private static final Lookup PRIV_LOOKUP = MethodHandles.lookup();
+
+        private static final Map<Class<?>,MethodHandle> ARRAY_TYPE_MH_MAPPING = Collections.unmodifiableMap(
+            Stream.of(boolean[].class, byte[].class, short[].class, int[].class, long[].class,
+                char[].class, float[].class, double[].class, Object[].class)
+                .collect(Collectors.toMap(Function.identity(), type -> {
+                    try {
+                        return PRIV_LOOKUP.findStatic(PRIV_LOOKUP.lookupClass(), "normalizeIndex",
+                                MethodType.methodType(int.class, type, int.class));
+                    } catch (ReflectiveOperationException e) {
+                        throw new AssertionError(e);
+                    }
+                }))
+        );
+
+        private static final MethodHandle OBJECT_ARRAY_MH = ARRAY_TYPE_MH_MAPPING.get(Object[].class);
+
+        static int normalizeIndex(final boolean[] array, final int index) { return index >= 0 ? index : index + array.length; }
+        static int normalizeIndex(final byte[] array, final int index) { return index >= 0 ? index : index + array.length; }
+        static int normalizeIndex(final short[] array, final int index) { return index >= 0 ? index : index + array.length; }
+        static int normalizeIndex(final int[] array, final int index) { return index >= 0 ? index : index + array.length; }
+        static int normalizeIndex(final long[] array, final int index) { return index >= 0 ? index : index + array.length; }
+        static int normalizeIndex(final char[] array, final int index) { return index >= 0 ? index : index + array.length; }
+        static int normalizeIndex(final float[] array, final int index) { return index >= 0 ? index : index + array.length; }
+        static int normalizeIndex(final double[] array, final int index) { return index >= 0 ? index : index + array.length; }
+        static int normalizeIndex(final Object[] array, final int index) { return index >= 0 ? index : index + array.length; }
+
+        static MethodHandle arrayIndexNormalizer(Class<?> arrayType) {
+            if (!arrayType.isArray()) {
+                throw new IllegalArgumentException("type must be an array");
+            }
+            return (ARRAY_TYPE_MH_MAPPING.containsKey(arrayType)) ?
+                ARRAY_TYPE_MH_MAPPING.get(arrayType) :
+                OBJECT_ARRAY_MH.asType(OBJECT_ARRAY_MH.type().changeParameterType(0, arrayType));
+        }
+
+        private ArrayIndexNormalizeHelper() {}
+    }
 }

+ 8 - 2
modules/lang-painless/src/main/java/org/elasticsearch/painless/DefBootstrap.java

@@ -32,9 +32,10 @@ import java.lang.invoke.WrongMethodTypeException;
 /**
  * Painless invokedynamic bootstrap for the call site.
  * <p>
- * Has 7 flavors (passed as static bootstrap parameters): dynamic method call,
+ * Has 11 flavors (passed as static bootstrap parameters): dynamic method call,
  * dynamic field load (getter), and dynamic field store (setter), dynamic array load,
- * dynamic array store, iterator, and method reference.
+ * dynamic array store, iterator, method reference, unary operator, binary operator,
+ * shift operator, and dynamic array index normalize.
  * <p>
  * When a new type is encountered at the call site, we lookup from the appropriate
  * whitelist, and cache with a guard. If we encounter too many types, we stop caching.
@@ -69,6 +70,8 @@ public final class DefBootstrap {
     public static final int BINARY_OPERATOR = 8;
     /** static bootstrap parameter indicating a shift operator, e.g. foo &gt;&gt; bar */
     public static final int SHIFT_OPERATOR = 9;
+    /** static bootstrap parameter indicating a request to normalize an index for array-like-access */
+    public static final int INDEX_NORMALIZE = 10;
     
     // constants for the flags parameter of operators
     /** 
@@ -152,6 +155,8 @@ public final class DefBootstrap {
                     return Def.lookupIterator(receiver);
                 case REFERENCE:
                     return Def.lookupReference(lookup, (String) args[0], receiver, name);
+                case INDEX_NORMALIZE:
+                    return Def.lookupIndexNormalize(receiver);
                 default: throw new AssertionError();
             }
         }
@@ -448,6 +453,7 @@ public final class DefBootstrap {
             case ARRAY_LOAD:
             case ARRAY_STORE:
             case ITERATOR:
+            case INDEX_NORMALIZE:
                 if (args.length > 0) {
                     throw new BootstrapMethodError("Illegal static bootstrap parameters for flavor: " + flavor);
                 }

+ 0 - 1
modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java

@@ -30,7 +30,6 @@ import org.objectweb.asm.commons.Method;
 
 import java.util.ArrayDeque;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Deque;
 import java.util.List;

+ 5 - 0
modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java

@@ -32,6 +32,7 @@ import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.util.BitSet;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Objects;
@@ -112,6 +113,7 @@ public final class WriterConstants {
     public static final Method DEF_TO_LONG_EXPLICIT   = getAsmMethod(long.class   , "DefTolongExplicit"  , Object.class);
     public static final Method DEF_TO_FLOAT_EXPLICIT  = getAsmMethod(float.class  , "DefTofloatExplicit" , Object.class);
     public static final Method DEF_TO_DOUBLE_EXPLICIT = getAsmMethod(double.class , "DefTodoubleExplicit", Object.class);
+    public static final Type DEF_ARRAY_LENGTH_METHOD_TYPE = Type.getMethodType(Type.INT_TYPE, Definition.DEF_TYPE.type);
 
     /** invokedynamic bootstrap for lambda expression/method references */
     public static final MethodType LAMBDA_BOOTSTRAP_TYPE =
@@ -158,6 +160,9 @@ public final class WriterConstants {
     public static final Type OBJECTS_TYPE = Type.getType(Objects.class);
     public static final Method EQUALS = getAsmMethod(boolean.class, "equals", Object.class, Object.class);
 
+    public static final Type COLLECTION_TYPE = Type.getType(Collection.class);
+    public static final Method COLLECTION_SIZE = getAsmMethod(int.class, "size");
+
     private static Method getAsmMethod(final Class<?> rtype, final String name, final Class<?>... ptypes) {
         return new Method(name, MethodType.methodType(rtype, ptypes).toMethodDescriptorString());
     }

+ 20 - 0
modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AStoreable.java

@@ -23,8 +23,11 @@ import org.elasticsearch.painless.Definition.Type;
 import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Location;
 import org.elasticsearch.painless.MethodWriter;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.Opcodes;
 
 import java.util.Objects;
+import java.util.function.Consumer;
 
 /**
  * The super class for an expression that can store a value in local memory.
@@ -100,4 +103,21 @@ abstract class AStoreable extends AExpression {
      * Called to store a storabable to local memory.
      */
     abstract void store(MethodWriter writer, Globals globals);
+
+    /**
+     * Writes the opcodes to flip a negative array index (meaning slots from the end of the array) into a 0-based one (meaning slots from
+     * the start of the array).
+     */
+    static void writeIndexFlip(MethodWriter writer, Consumer<MethodWriter> writeGetLength) {
+        Label noFlip = new Label();
+        // Everywhere when it says 'array' below that could also be a list
+        // The stack after each instruction:       array, unnormalized_index
+        writer.dup();                           // array, unnormalized_index, unnormalized_index
+        writer.ifZCmp(Opcodes.IFGE, noFlip);    // array, unnormalized_index
+        writer.swap();                          // negative_index, array
+        writer.dupX1();                         // array, negative_index, array
+        writeGetLength.accept(writer);          // array, negative_index, length
+        writer.visitInsn(Opcodes.IADD);         // array, noralized_index
+        writer.mark(noFlip);                    // array, noralized_index
+    }
 }

+ 3 - 4
modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubBrace.java

@@ -60,10 +60,8 @@ final class PSubBrace extends AStoreable {
 
     @Override
     void write(MethodWriter writer, Globals globals) {
-        if (!write) {
-            setup(writer, globals);
-            load(writer, globals);
-        }
+        setup(writer, globals);
+        load(writer, globals);
     }
 
     @Override
@@ -84,6 +82,7 @@ final class PSubBrace extends AStoreable {
     @Override
     void setup(MethodWriter writer, Globals globals) {
         index.write(writer, globals);
+        writeIndexFlip(writer, MethodWriter::arrayLength);
     }
 
     @Override

+ 8 - 9
modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefArray.java

@@ -34,7 +34,6 @@ import java.util.Set;
  * Represents an array load/store or shortcut on a def type.  (Internal only.)
  */
 final class PSubDefArray extends AStoreable {
-
     private AExpression index;
 
     PSubDefArray(Location location, AExpression index) {
@@ -59,13 +58,8 @@ final class PSubDefArray extends AStoreable {
 
     @Override
     void write(MethodWriter writer, Globals globals) {
-        index.write(writer, globals);
-
-        writer.writeDebugInfo(location);
-
-        org.objectweb.asm.Type methodType =
-            org.objectweb.asm.Type.getMethodType(actual.type, Definition.DEF_TYPE.type, index.actual.type);
-        writer.invokeDefCall("arrayLoad", methodType, DefBootstrap.ARRAY_LOAD);
+        setup(writer, globals);
+        load(writer, globals);
     }
 
     @Override
@@ -85,7 +79,12 @@ final class PSubDefArray extends AStoreable {
 
     @Override
     void setup(MethodWriter writer, Globals globals) {
-        index.write(writer, globals);
+        // Current stack:                                                                    def
+        writer.dup();                                                                     // def, def
+        index.write(writer, globals);                                                     // def, def, unnormalized_index
+        org.objectweb.asm.Type methodType = org.objectweb.asm.Type.getMethodType(
+                index.actual.type, Definition.DEF_TYPE.type, index.actual.type);
+        writer.invokeDefCall("normalizeIndex", methodType, DefBootstrap.INDEX_NORMALIZE); // def, normalized_index
     }
 
     @Override

+ 6 - 9
modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubListShortcut.java

@@ -28,6 +28,7 @@ import org.elasticsearch.painless.Globals;
 import org.elasticsearch.painless.Locals;
 import org.elasticsearch.painless.Location;
 import org.elasticsearch.painless.MethodWriter;
+import org.elasticsearch.painless.WriterConstants;
 
 import java.util.Objects;
 import java.util.Set;
@@ -87,15 +88,8 @@ final class PSubListShortcut extends AStoreable {
 
     @Override
     void write(MethodWriter writer, Globals globals) {
-        index.write(writer, globals);
-
-        writer.writeDebugInfo(location);
-
-        getter.write(writer);
-
-        if (!getter.rtn.clazz.equals(getter.handle.type().returnType())) {
-            writer.checkCast(getter.rtn.type);
-        }
+        setup(writer, globals);
+        load(writer, globals);
     }
 
     @Override
@@ -116,6 +110,9 @@ final class PSubListShortcut extends AStoreable {
     @Override
     void setup(MethodWriter writer, Globals globals) {
         index.write(writer, globals);
+        writeIndexFlip(writer, w -> {
+            w.invokeInterface(WriterConstants.COLLECTION_TYPE, WriterConstants.COLLECTION_SIZE);
+        });
     }
 
     @Override

+ 105 - 0
modules/lang-painless/src/test/java/org/elasticsearch/painless/ArrayLikeObjectTestCase.java

@@ -0,0 +1,105 @@
+/*
+ * 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;
+
+import org.elasticsearch.common.Nullable;
+import org.hamcrest.Matcher;
+
+import static java.util.Collections.singletonMap;
+
+/**
+ * Superclass for testing array-like objects (arrays and lists).
+ */
+public abstract class ArrayLikeObjectTestCase extends ScriptTestCase {
+    /**
+     * Build the string for declaring the variable holding the array-like-object to test. So {@code int[]} for arrays and {@code List} for
+     * lists.
+     */
+    protected abstract String declType(String valueType);
+    /**
+     * Build the string for calling the constructor for the array-like-object to test. So {@code new int[5]} for arrays and
+     * {@code [0, 0, 0, 0, 0]} or {@code [null, null, null, null, null]} for lists.
+     */
+    protected abstract String valueCtorCall(String valueType, int size);
+    /**
+     * The type of the exception thrown by out of bounds accesses;
+     */
+    protected abstract Matcher<? super IndexOutOfBoundsException> outOfBoundsExceptionMatcher(int index, int size);
+
+    private void arrayLoadStoreTestCase(boolean declareAsDef, String valueType, Object val, @Nullable Number valPlusOne) {
+        String declType = declareAsDef ? "def" : declType(valueType);
+        String valueCtorCall = valueCtorCall(valueType, 5);
+        String decl = declType + " x = " + valueCtorCall;
+        assertEquals(5, exec(decl + "; return x.length", true));
+        assertEquals(val, exec(decl + "; x[ 0] = params.val; return x[ 0];", singletonMap("val", val), true));
+        assertEquals(val, exec(decl + "; x[ 0] = params.val; return x[-5];", singletonMap("val", val), true));
+        assertEquals(val, exec(decl + "; x[-5] = params.val; return x[-5];", singletonMap("val", val), true));
+
+        expectOutOfBounds( 6, decl + "; return x[ 6]", val);
+        expectOutOfBounds(-1, decl + "; return x[-6]", val);
+        expectOutOfBounds( 6, decl + "; x[ 6] = params.val; return 0", val);
+        expectOutOfBounds(-1, decl + "; x[-6] = params.val; return 0", val);
+
+        if (valPlusOne != null) {
+            assertEquals(val,        exec(decl + "; x[0] = params.val; x[ 0] = x[ 0]++; return x[0];", singletonMap("val", val), true));
+            assertEquals(val,        exec(decl + "; x[0] = params.val; x[ 0] = x[-5]++; return x[0];", singletonMap("val", val), true));
+            assertEquals(valPlusOne, exec(decl + "; x[0] = params.val; x[ 0] = ++x[ 0]; return x[0];", singletonMap("val", val), true));
+            assertEquals(valPlusOne, exec(decl + "; x[0] = params.val; x[ 0] = ++x[-5]; return x[0];", singletonMap("val", val), true));
+            assertEquals(valPlusOne, exec(decl + "; x[0] = params.val; x[ 0]++        ; return x[0];", singletonMap("val", val), true));
+            assertEquals(valPlusOne, exec(decl + "; x[0] = params.val; x[-5]++        ; return x[0];", singletonMap("val", val), true));
+            assertEquals(valPlusOne, exec(decl + "; x[0] = params.val; x[ 0] += 1     ; return x[0];", singletonMap("val", val), true));
+            assertEquals(valPlusOne, exec(decl + "; x[0] = params.val; x[-5] += 1     ; return x[0];", singletonMap("val", val), true));
+
+            expectOutOfBounds( 6, decl + "; return x[ 6]++", val);
+            expectOutOfBounds(-1, decl + "; return x[-6]++", val);
+            expectOutOfBounds( 6, decl + "; return ++x[ 6]", val);
+            expectOutOfBounds(-1, decl + "; return ++x[-6]", val);
+            expectOutOfBounds( 6, decl + "; x[ 6] += 1; return 0", val);
+            expectOutOfBounds(-1, decl + "; x[-6] += 1; return 0", val);
+        }
+    }
+
+    private void expectOutOfBounds(int index, String script, Object val) {
+        IndexOutOfBoundsException e = expectScriptThrows(IndexOutOfBoundsException.class,
+                () -> exec(script, singletonMap("val", val), true));
+        try {
+            assertThat(e, outOfBoundsExceptionMatcher(index, 5));
+        } catch (AssertionError ae) {
+            ae.addSuppressed(e);   // Mark the exception we are testing as suppressed so we get its stack trace. If it has one :(
+            throw ae;
+        }
+    }
+
+    public void testInts() {         arrayLoadStoreTestCase(false, "int",    5,         6); }
+    public void testIntsInDef() {    arrayLoadStoreTestCase(true,  "int",    5,         6); }
+    public void testLongs() {        arrayLoadStoreTestCase(false, "long",   5L,        6L); }
+    public void testLongsInDef() {   arrayLoadStoreTestCase(true,  "long",   5L,        6L); }
+    public void testShorts() {       arrayLoadStoreTestCase(false, "short",  (short) 5, (short) 6); }
+    public void testShortsInDef() {  arrayLoadStoreTestCase(true,  "short",  (short) 5, (short) 6); }
+    public void testBytes() {        arrayLoadStoreTestCase(false, "byte",   (byte) 5,  (byte) 6); }
+    public void testBytesInDef() {   arrayLoadStoreTestCase(true,  "byte",   (byte) 5,  (byte) 6); }
+    public void testFloats() {       arrayLoadStoreTestCase(false, "float",  5.0f,      6.0f); }
+    public void testFloatsInDef() {  arrayLoadStoreTestCase(true,  "float",  5.0f,      6.0f); }
+    public void testDoubles() {      arrayLoadStoreTestCase(false, "double", 5.0d,      6.0d); }
+    public void testDoublesInDef() { arrayLoadStoreTestCase(true,  "double", 5.0d,      6.0d); }
+    public void testStrings() {      arrayLoadStoreTestCase(false, "String", "cat",     null); }
+    public void testStringsInDef() { arrayLoadStoreTestCase(true,  "String", "cat",     null); }
+    public void testDef() {          arrayLoadStoreTestCase(true,  "def",    5,         null); }
+}

+ 23 - 25
modules/lang-painless/src/test/java/org/elasticsearch/painless/ArrayTests.java

@@ -19,11 +19,32 @@
 
 package org.elasticsearch.painless;
 
+import org.hamcrest.Matcher;
+
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodType;
 
-/** Tests for or operator across all types */
-public class ArrayTests extends ScriptTestCase {
+import static org.hamcrest.Matchers.both;
+import static org.hamcrest.Matchers.hasToString;
+import static org.hamcrest.Matchers.instanceOf;
+
+/** Tests for working with arrays. */
+public class ArrayTests extends ArrayLikeObjectTestCase {
+    @Override
+    protected String declType(String valueType) {
+        return valueType + "[]";
+    }
+
+    @Override
+    protected String valueCtorCall(String valueType, int size) {
+        return "new " + valueType + "[" + size + "]";
+    }
+
+    @Override
+    protected Matcher<? super IndexOutOfBoundsException> outOfBoundsExceptionMatcher(int index, int size) {
+        return both(instanceOf(ArrayIndexOutOfBoundsException.class))
+                .and(hasToString("java.lang.ArrayIndexOutOfBoundsException: " + index));
+    }
 
     public void testArrayLengthHelper() throws Throwable {
         assertArrayLength(2, new int[2]);
@@ -45,29 +66,6 @@ public class ArrayTests extends ScriptTestCase {
                 .invokeExact(array));
     }
 
-    public void testArrayLoadStoreInt() {
-        assertEquals(5, exec("def x = new int[5]; return x.length"));
-        assertEquals(5, exec("def x = new int[4]; x[0] = 5; return x[0];"));
-    }
-
-    public void testArrayLoadStoreString() {
-        assertEquals(5, exec("def x = new String[5]; return x.length"));
-        assertEquals("foobar", exec("def x = new String[4]; x[0] = 'foobar'; return x[0];"));
-    }
-
-    public void testArrayLoadStoreDef() {
-        assertEquals(5, exec("def x = new def[5]; return x.length"));
-        assertEquals(5, exec("def x = new def[4]; x[0] = 5; return x[0];"));
-    }
-
-    public void testArrayCompoundInt() {
-        assertEquals(6, exec("int[] x = new int[5]; x[0] = 5; x[0]++; return x[0];"));
-    }
-
-    public void testArrayCompoundDef() {
-        assertEquals(6, exec("def x = new int[5]; x[0] = 5; x[0]++; return x[0];"));
-    }
-
     public void testJacksCrazyExpression1() {
         assertEquals(1, exec("int x; def[] y = new def[1]; x = y[0] = 1; return x;"));
     }

+ 70 - 0
modules/lang-painless/src/test/java/org/elasticsearch/painless/ListTests.java

@@ -0,0 +1,70 @@
+/*
+ * 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;
+
+import org.hamcrest.Matcher;
+
+import java.util.Arrays;
+
+import static org.hamcrest.Matchers.both;
+import static org.hamcrest.Matchers.either;
+import static org.hamcrest.Matchers.hasToString;
+import static org.hamcrest.Matchers.instanceOf;
+
+/** Tests for working with lists. */
+public class ListTests extends ArrayLikeObjectTestCase {
+    @Override
+    protected String declType(String valueType) {
+        return "List";
+    }
+
+    @Override
+    protected String valueCtorCall(String valueType, int size) {
+        String[] fill = new String[size];
+        Arrays.fill(fill, fillValue(valueType));
+        return "[" + String.join(",", fill) + "]";
+    }
+
+    private String fillValue(String valueType) {
+        switch (valueType) {
+        case "int":    return "0";
+        case "long":   return "0L";
+        case "short":  return "(short) 0";
+        case "byte":   return "(byte) 0";
+        case "float":  return "0.0f";
+        case "double": return "0.0"; // Double is implicit for decimal constants
+        default:       return null;
+        }
+    }
+
+    @Override
+    protected Matcher<? super IndexOutOfBoundsException> outOfBoundsExceptionMatcher(int index, int size) {
+        if (index > size) {
+            return hasToString("java.lang.IndexOutOfBoundsException: Index: " + index + ", Size: " + size);
+        } else {
+            Matcher<? super IndexOutOfBoundsException> m = both(instanceOf(ArrayIndexOutOfBoundsException.class))
+                    .and(hasToString("java.lang.ArrayIndexOutOfBoundsException: " + index));
+            // If we set -XX:-OmitStackTraceInFastThrow we wouldn't need this
+            m = either(m).or(instanceOf(ArrayIndexOutOfBoundsException.class));
+            return m;
+        }
+    }
+
+}

+ 45 - 0
modules/lang-painless/src/test/java/org/elasticsearch/painless/MapTests.java

@@ -0,0 +1,45 @@
+/*
+ * 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;
+
+import static java.util.Collections.singletonMap;
+
+/** Tests for working with maps. */
+public class MapTests extends ScriptTestCase {
+    private void mapAccessesTestCase(String listType) {
+        Object val = randomFrom("test", 1, 1.3, new Object());
+        String decl = listType + " x = ['a': 1, 'b': 2, 0: 2, -5: 'slot', 123.1: 12]";
+        assertEquals(5, exec(decl + "; return x.size()"));
+        assertEquals(2, exec(decl + "; return x[0];", true));
+        assertEquals(1, exec(decl + "; return x['a'];", true));
+        assertEquals(12, exec(decl + "; return x[123.1];", true));
+        assertEquals(val,    exec(decl + "; x[ 0] = params.val; return x[ 0];", singletonMap("val", val), true));
+        assertEquals("slot", exec(decl + "; x[ 0] = params.val; return x[-5];", singletonMap("val", val), true));
+        assertEquals(val,    exec(decl + "; x[-5] = params.val; return x[-5];", singletonMap("val", val), true));
+    }
+
+    public void testMapInDefAccesses() {
+        mapAccessesTestCase("def");
+    }
+
+    public void testMapAccesses() {
+        mapAccessesTestCase("Map");
+    }
+}