Typecheck all WIP

7 files changed, 121 insertions, 52 deletions

diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java
index 2e2858da238..262aba89f27 100755
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java
@@ -3,6 +3,7 @@ package com.yahoo.searchlib.rankingexpression;
 
 import com.google.common.collect.ImmutableList;
 import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode;
+import com.yahoo.searchlib.rankingexpression.rule.FunctionReferenceContext;
 import com.yahoo.searchlib.rankingexpression.rule.SerializationContext;
 import com.yahoo.text.Utf8;
 
@@ -11,9 +12,9 @@ import java.security.NoSuchAlgorithmException;
 import java.util.*;
 
 /**
- * <p>A function defined by a ranking expression</p>
+ * A function defined by a ranking expression
  *
- * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a>
+ * @author Simon Thoresen
  * @author bratseth
  */
 public class ExpressionFunction {
@@ -23,7 +24,7 @@ public class ExpressionFunction {
     private final RankingExpression body;
 
     /**
-     * <p>Constructs a new function</p>
+     * Constructs a new function
      *
      * @param name the name of this function
      * @param arguments its argument names
@@ -43,28 +44,27 @@ public class ExpressionFunction {
     public RankingExpression getBody() { return body; }
 
     /**
-     * <p>Create and return an instance of this function based on the given
-     * arguments. If function calls are nested, this call might produce
-     * additional scripts.</p>
+     * Creates and returns an instance of this function based on the given
+     * arguments. If function calls are nested, this call may produce
+     * additional functions.
      *
      * @param context the context used to expand this
-     * @param arguments the arguments to instantiate on.
+     * @param argumentValues the arguments to instantiate on.
      * @param path the expansion path leading to this.
      * @return the script function instance created.
      */
-    public Instance expand(SerializationContext context, List<ExpressionNode> arguments, Deque<String> path) {
+    public Instance expand(SerializationContext context, List<ExpressionNode> argumentValues, Deque<String> path) {
         Map<String, String> argumentBindings = new HashMap<>();
-        for (int i = 0; i < this.arguments.size() && i < arguments.size(); ++i) {
-            argumentBindings.put(this.arguments.get(i), arguments.get(i).toString(context, path, null));
+        for (int i = 0; i < arguments.size() && i < arguments.size(); ++i) {
+            argumentBindings.put(arguments.get(i), argumentValues.get(i).toString(context, path, null));
         }
-        return new Instance(toSymbol(argumentBindings), body.getRoot().toString(context.createBinding(argumentBindings), path, null));
+        return new Instance(toSymbol(argumentBindings), body.getRoot().toString(context.withBindings(argumentBindings), path, null));
     }
 
     /**
      * Returns a symbolic string that represents this function with a given
      * list of arguments. The arguments are mangled by hashing the string
-     * representation of the argument expressions, so we might need to revisit
-     * this if we start seeing collisions.
+     * representation of the argument expressions.
      *
      * @param  argumentBindings the bound arguments to include in the symbolic name.
      * @return the symbolic name for an instance of this function
@@ -85,8 +85,8 @@ public class ExpressionFunction {
 
 
     /**
-     * <p>Returns a more unique hash code than what Java's own {@link
-     * String#hashCode()} method would produce.</p>
+     * Returns a more unique hash code than what Java's own {@link
+     * String#hashCode()} method would produce.
      *
      * @param str The string to hash.
      * @return A 64 bit long hash code.
@@ -136,4 +136,5 @@ public class ExpressionFunction {
         }
 
     }
+
 }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeMapContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapTypeContext.java
index ff2088263d8..d461ae52cbe 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeMapContext.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapTypeContext.java
@@ -13,7 +13,7 @@ import java.util.Map;
  *
  * @author bratseth
  */
-public class TypeMapContext implements TypeContext {
+public class MapTypeContext implements TypeContext {
 
     private final Map<String, TensorType> featureTypes = new HashMap<>();
 
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/EvaluationTypeContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/EvaluationTypeContext.java
new file mode 100644
index 00000000000..3807fbe2207
--- /dev/null
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/EvaluationTypeContext.java
@@ -0,0 +1,11 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.searchlib.rankingexpression.rule;
+
+import com.yahoo.tensor.TensorType;
+import com.yahoo.tensor.evaluation.TypeContext;
+
+public interface EvaluationTypeContext extends TypeContext {
+
+    TensorType getType(String name, Arguments arguments, String output);
+
+}
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/FunctionReferenceContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/FunctionReferenceContext.java
new file mode 100644
index 00000000000..289495f6ce9
--- /dev/null
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/FunctionReferenceContext.java
@@ -0,0 +1,73 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.searchlib.rankingexpression.rule;
+
+import com.google.common.collect.ImmutableMap;
+import com.yahoo.searchlib.rankingexpression.ExpressionFunction;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * The context of a function invocation.
+ *
+ * @author bratseth
+ */
+public class FunctionReferenceContext {
+
+    /** Expression functions indexed by name */
+    private final ImmutableMap<String, ExpressionFunction> functions;
+
+    /** Mapping from argument names to the expressions they resolve to */
+    public final Map<String, String> bindings = new HashMap<>();
+
+    /** Create a context for a single serialization task */
+    public FunctionReferenceContext() {
+        this(Collections.emptyList());
+    }
+
+    /** Create a context for a single serialization task */
+    public FunctionReferenceContext(Collection<ExpressionFunction> functions) {
+        this(toMap(functions), Collections.emptyMap());
+    }
+
+    public FunctionReferenceContext(Collection<ExpressionFunction> functions, Map<String, String> bindings) {
+        this(toMap(functions), bindings);
+    }
+
+    /** Create a context for a single serialization task */
+    public FunctionReferenceContext(Map<String, ExpressionFunction> functions) {
+        this(functions.values());
+    }
+
+    /** Create a context for a single serialization task */
+    public FunctionReferenceContext(Map<String, ExpressionFunction> functions, Map<String, String> bindings) {
+        this.functions = ImmutableMap.copyOf(functions);
+        if (bindings != null)
+            this.bindings.putAll(bindings);
+    }
+
+    private static ImmutableMap<String, ExpressionFunction> toMap(Collection<ExpressionFunction> list) {
+        ImmutableMap.Builder<String,ExpressionFunction> mapBuilder = new ImmutableMap.Builder<>();
+        for (ExpressionFunction function : list)
+            mapBuilder.put(function.getName(), function);
+        return mapBuilder.build();
+    }
+
+    /**
+     * Returns a function or null if it isn't defined in this context
+     */
+    public ExpressionFunction getFunction(String name) { return functions.get(name); }
+
+    protected Map<String, ExpressionFunction> functions() { return functions; }
+
+    /** Returns the resolution of an argument, or null if it isn't defined in this context */
+    public String getBinding(String name) { return bindings.get(name); }
+
+    /** Returns a new context with the bindings replaced by the given bindings */
+    public FunctionReferenceContext withBindings(Map<String, String> bindings) {
+        return new FunctionReferenceContext(this.functions, bindings);
+    }
+
+}
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
index b9b377dc0ec..ebfef21a815 100755
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
@@ -13,11 +13,13 @@ import java.util.Deque;
 import java.util.List;
 
 /**
- * A node referring either to a value in the context or to another named ranking expression.
+ * A node referring either to a value in the context or to a named ranking expression (function aka macro).
  *
  * @author simon
  * @author bratseth
  */
+// TODO: Using the same node to represent formal function argument, the function itself, and to features is confusing.
+//       At least the first two should be split into separate classes.
 public final class ReferenceNode extends CompositeNode {
 
     private final String name, output;
@@ -71,7 +73,7 @@ public final class ReferenceNode extends CompositeNode {
         List<ExpressionNode> myArguments = this.arguments.expressions();
 
         String resolvedArgument = context.getBinding(myName);
-        if (resolvedArgument != null && this.arguments.expressions().size() == 0 && myOutput == null) {
+        if (resolvedArgument != null && isBindableName()) {
             // Replace this whole node with the value of the argument value that it maps to
             myName = resolvedArgument;
             myArguments = null;
@@ -110,15 +112,18 @@ public final class ReferenceNode extends CompositeNode {
         return ret.toString();
     }
 
+    /** Returns whether this is a name that can be bound to a value (during argument passing) */
+    public boolean isBindableName() {
+        return this.arguments.expressions().size() == 0 && output == null;
+    }
+
     @Override
     public TensorType type(TypeContext context) {
-        // Ensure base name (excluding output exists,
-        // but don't support outputs of different tensor types (not used, so no need)
-        String name = toString(new SerializationContext(), null, false);
-        TensorType type = context.getType(name);
-
+        String feature = toString(new SerializationContext(), null, false);
+        TensorType type = context.getType(feature);
+        // TensorType type = context.getType(name, arguments, output); TODO
         if (type == null)
-            throw new IllegalArgumentException("Unknown feature '" + toString() + "'");
+            throw new IllegalArgumentException("Unknown feature '" + feature + "'");
         return type;
     }
 
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SerializationContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SerializationContext.java
index ba765d07094..796c13a8669 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SerializationContext.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SerializationContext.java
@@ -16,17 +16,11 @@ import java.util.Map;
  *
  * @author bratseth
  */
-public class SerializationContext {
+public class SerializationContext extends FunctionReferenceContext {
     
-    /** Expression functions indexed by name */
-    private final ImmutableMap<String, ExpressionFunction> functions;
-
-    /** A cache of already serialized expressions indexed by name */
+    /** Serialized form of functions indexed by name */
     private final Map<String, String> serializedFunctions;
 
-    /** Mapping from argument names to the expressions they resolve to */
-    public final Map<String, String> bindings = new HashMap<>();
-
     /** Create a context for a single serialization task */
     public SerializationContext() {
         this(Collections.emptyList());
@@ -77,17 +71,10 @@ public class SerializationContext {
      */
     public SerializationContext(ImmutableMap<String,ExpressionFunction> functions, Map<String, String> bindings,
                                 Map<String, String> serializedFunctions) {
-        this.functions = functions;
+        super(functions, bindings);
         this.serializedFunctions = serializedFunctions;
-        if (bindings != null)
-            this.bindings.putAll(bindings);
     }
 
-    /**
-     * Returns a function or null if it isn't defined in this context
-     */
-    public ExpressionFunction getFunction(String name) { return functions.get(name); }
-
     /** Adds the serialization of a function */
     public void addFunctionSerialization(String name, String expressionString) {
         serializedFunctions.put(name, expressionString);
@@ -98,17 +85,9 @@ public class SerializationContext {
         return serializedFunctions.get(name);
     }
 
-    /**
-     * Returns the resolution of an argument, or null if it isn't defined in this context
-     */
-    public String getBinding(String name) { return bindings.get(name); }
-
-    /**
-     * Returns a new context which shares the functions and serialized function map with this but has different
-     * arguments.
-     */
-    public SerializationContext createBinding(Map<String, String> arguments) {
-        return new SerializationContext(this.functions, arguments, this.serializedFunctions);
+    @Override
+    public SerializationContext withBindings(Map<String, String> bindings) {
+        return new SerializationContext(functions().values(), bindings, this.serializedFunctions);
     }
 
     public Map<String, String> serializedFunctions() { return serializedFunctions; }
diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeResolutionTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeResolutionTestCase.java
index c882c887c8d..18444c7f8b6 100644
--- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeResolutionTestCase.java
+++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeResolutionTestCase.java
@@ -18,7 +18,7 @@ public class TypeResolutionTestCase {
 
     @Test
     public void testTypeResolution() {
-        TypeMapContext context = new TypeMapContext();
+        MapTypeContext context = new MapTypeContext();
         context.setType("query(x1)", TensorType.fromSpec("tensor(x[])"));
         context.setType("query(x2)", TensorType.fromSpec("tensor(x[10])"));
         context.setType("query(y1)", TensorType.fromSpec("tensor(y[])"));