[MOREL-203] Cannot deduce type for 'from b in [SOME true, NONE]'

Type should be 'bool option list', but we get a type
inconsistency during translation. To fix it, we needed to
improve how type variables are substituted into type schemes
like "'a option" and "'a list".

Remove class AppliedDataType, class ApplyType, class DataTypeDef
(merging its functionality into Keys.DataTypeKey), remove
interface Transaction and class TemporaryType (and simplify
the process of creating datatype instances); remove field
DataTypeKey.parameterCount.

Fix typo: 'short-hand' should be 'shorthand'

Convert Type.Key into an abstract class, and replace
Key.moniker() with Key.toString().

Fixes hydromatic#203

README.md

@@ -310,7 +310,7 @@ implicit labels. For instance,
 from e in emps
   group e.deptno compute sum of e.salary, count
 ```
-is short-hand for
+is shorthand for
 ```
 from e in emps
   group deptno = e.deptno compute sum = sum of e.salary, count = count

src/main/java/net/hydromatic/morel/Main.java

@@ -58,6 +58,8 @@
 import java.util.Map;
 import java.util.function.Consumer;
 
+import static net.hydromatic.morel.util.Static.str;
+
 import static java.util.Objects.requireNonNull;
 
 /** Standard ML REPL. */
@@ -122,8 +124,7 @@ private static void readerToString(Reader r, StringBuilder b) {
   private static Reader stripOutLines(Reader in) {
     final StringBuilder b = new StringBuilder();
     readerToString(in, b);
-    final String s = b.toString();
-    b.setLength(0);
+    final String s = str(b);
     for (int i = 0, n = s.length();;) {
       int j0 = i == 0 && s.startsWith("> ") ? 0 : -1;
       int j1 = s.indexOf("\n> ", i);
@@ -373,9 +374,7 @@ protected BufferingReader(Reader in) {
     }
 
     public String flush() {
-      final String s = buf.toString();
-      buf.setLength(0);
-      return s;
+      return str(buf);
     }
   }
 }

src/main/java/net/hydromatic/morel/Shell.java

@@ -72,6 +72,8 @@
 import java.util.function.Consumer;
 import javax.annotation.Nonnull;
 
+import static net.hydromatic.morel.util.Static.str;
+
 import static java.util.Objects.requireNonNull;
 
 /** Command shell for ML, powered by JLine3. */
@@ -472,8 +474,7 @@ void extracted(@Nullable Map<String, Binding> outBindings) {
           try {
             buf.append(line.right);
             if (line.right.endsWith(";")) {
-              final String code = buf.toString();
-              buf.setLength(0);
+              final String code = str(buf);
               final MorelParserImpl smlParser =
                   new MorelParserImpl(new StringReader(code));
               final AstNode statement;

src/main/java/net/hydromatic/morel/ast/Ast.java

@@ -616,7 +616,7 @@ AstWriter unparse(AstWriter w, int left, int right) {
   public static class RecordType extends Type {
     public final Map<String, Type> fieldTypes;
 
-    /** Creates a TyVar. */
+    /** Creates a record type. */
     RecordType(Pos pos, ImmutableMap<String, Type> fieldTypes) {
       super(pos, Op.RECORD_TYPE);
       this.fieldTypes = requireNonNull(fieldTypes);

src/main/java/net/hydromatic/morel/ast/Core.java

@@ -98,6 +98,11 @@ public Type type() {
       return type;
     }
 
+    /** Returns the type's key. */
+    public Type.Key typeKey() {
+      return type().key();
+    }
+
     @Override public abstract Pat accept(Shuttle shuttle);
   }
 

src/main/java/net/hydromatic/morel/ast/CoreBuilder.java

@@ -56,9 +56,9 @@
 
 import static net.hydromatic.morel.type.RecordType.ORDERING;
 import static net.hydromatic.morel.util.Pair.forEach;
+import static net.hydromatic.morel.util.Static.transform;
 
 import static com.google.common.collect.Iterables.getOnlyElement;
-import static org.apache.calcite.util.Util.transform;
 
 /** Builds parse tree nodes. */
 public enum CoreBuilder {
@@ -533,7 +533,7 @@ public Core.Yield yield_(TypeSystem typeSystem, Core.Exp exp) {
     return yield_(bindings, exp);
   }
 
-  // Short-hands
+  // Shorthands
 
   /** Creates a reference to the {@code slot}th field of an expression,
    * "{@code #slot exp}". The expression's type must be record or tuple. */

src/main/java/net/hydromatic/morel/ast/Op.java

@@ -59,7 +59,7 @@ public enum Op {
 
   // miscellaneous
   BAR(" | "),
-  COMMA(", "),
+  COMMA(","),
   FUN_BIND(" and "),
   FUN_MATCH,
   TY_CON,
@@ -83,8 +83,9 @@ public enum Op {
   TY_VAR(true),
   RECORD_TYPE(true),
   DATA_TYPE(" ", 8),
-  /** Used internally, while resolving a self-referential DATA_TYPE. */
-  TEMPORARY_DATA_TYPE(true),
+  /** Used internally, as the 'type' of a type constructor that does not contain
+   * data. */
+  DUMMY_TYPE(true),
   APPLY_TYPE(" ", 8),
   TUPLE_TYPE(" * ", 7),
   COMPOSITE_TYPE,

src/main/java/net/hydromatic/morel/compile/BuiltIn.java

@@ -20,8 +20,8 @@
 
 import net.hydromatic.morel.type.Binding;
 import net.hydromatic.morel.type.DataType;
-import net.hydromatic.morel.type.DummyType;
 import net.hydromatic.morel.type.ForallType;
+import net.hydromatic.morel.type.Keys;
 import net.hydromatic.morel.type.PrimitiveType;
 import net.hydromatic.morel.type.RecordType;
 import net.hydromatic.morel.type.Type;
@@ -1705,24 +1705,24 @@ private static void defineDataType(TypeSystem ts, List<Binding> bindings,
     for (int i = 0; i < varCount; i++) {
       tyVars.add(ts.typeVariable(i));
     }
-    final SortedMap<String, Type> tyCons = new TreeMap<>();
+    final SortedMap<String, Type.Key> tyCons = new TreeMap<>();
     transform.apply(new DataTypeHelper() {
-      public DataTypeHelper tyCon(String name, Type type) {
-        tyCons.put(name, type);
+      public DataTypeHelper tyCon(String name, Type.Key typeKey) {
+        tyCons.put(name, typeKey);
         return this;
       }
 
       public DataTypeHelper tyCon(String name) {
-        return tyCon(name, DummyType.INSTANCE);
+        return tyCon(name, Keys.dummy());
       }
 
-      public TypeVar get(int i) {
-        return tyVars.get(i);
+      public Type.Key get(int i) {
+        return tyVars.get(i).key();
       }
     });
     final Type type = ts.dataTypeScheme(name, tyVars, tyCons);
-    final DataType dataType = (DataType) (type instanceof DataType ? type
-        : ((ForallType) type).type);
+    final DataType dataType =
+        (DataType) (type instanceof DataType ? type : ((ForallType) type).type);
     if (internal) {
       ts.setInternal(name);
     } else {
@@ -1754,8 +1754,8 @@ public BuiltIn reverse() {
   /** Callback used when defining a datatype. */
   private interface DataTypeHelper {
     DataTypeHelper tyCon(String name);
-    DataTypeHelper tyCon(String name, Type type);
-    TypeVar get(int i);
+    DataTypeHelper tyCon(String name, Type.Key typeKey);
+    Type.Key get(int i);
   }
 
   /** Built-in structure. */

src/main/java/net/hydromatic/morel/compile/Compiler.java

@@ -68,6 +68,7 @@
 import static net.hydromatic.morel.ast.Ast.Direction.DESC;
 import static net.hydromatic.morel.ast.CoreBuilder.core;
 import static net.hydromatic.morel.util.Pair.forEach;
+import static net.hydromatic.morel.util.Static.str;
 import static net.hydromatic.morel.util.Static.toImmutableList;
 import static net.hydromatic.morel.util.Static.transform;
 
@@ -593,11 +594,8 @@ private void compileDatatypeDecl(List<DataType> dataTypes,
         final List<Binding> immutableBindings =
             ImmutableList.copyOf(newBindings);
         actions.add((outLines, outBindings, evalEnv) -> {
-          final StringBuilder buf =
-              new StringBuilder().append("datatype ")
-                  .append(dataType.moniker).append(" = ");
-          dataType.def().describe(buf);
-          outLines.accept(buf.toString());
+          String line = dataType.describe(new StringBuilder()).toString();
+          outLines.accept(line);
           immutableBindings.forEach(outBindings);
         });
       }
@@ -727,18 +725,16 @@ private void compileValDecl(Context cx, Core.ValDecl valDecl,
                         stringDepth);
                 pretty.pretty(buf, pat2.type,
                     new Pretty.TypedVal(pat2.name, o2, pat2.type));
-                final String out = buf.toString();
-                buf.setLength(0);
-                outs.add(out);
-                outLines.accept(out);
+                final String line = str(buf);
+                outs.add(line);
+                outLines.accept(line);
               }
             });
           } catch (Codes.MorelRuntimeException e) {
             session.handle(e, buf);
-            final String out = buf.toString();
-            buf.setLength(0);
-            outs.add(out);
-            outLines.accept(out);
+            final String line = str(buf);
+            outs.add(line);
+            outLines.accept(line);
           }
           session.code = code;
           session.out = ImmutableList.copyOf(outs);

src/main/java/net/hydromatic/morel/compile/Compiles.java

@@ -98,6 +98,7 @@ private static CompiledStatement prepareDecl(TypeSystem typeSystem,
     // Should we skip printing the root pattern?
     // Yes, if they wrote 'val x = 1 and y = 2' and
     // core became 'val it as (x, y) = (1, 2)'.
+    // No, if they actually wrote 'val (x, y) = (1, 2)'.
     final Core.NamedPat skipPat = getSkipPat(resolved.node, coreDecl0);
 
     // Check for exhaustive and redundant patterns, and throw errors or

src/main/java/net/hydromatic/morel/compile/PatternCoverageChecker.java

@@ -210,7 +210,7 @@ private void addConsTerms(Path path, List<Sat.Term> terms,
   }
 
   private Sat.Variable typeConstructorTerm(Path path, String con) {
-    final Pair<DataType, Type> pair = typeSystem.lookupTyCon(con);
+    final Pair<DataType, Type.Key> pair = typeSystem.lookupTyCon(con);
     final DataType dataType = pair.left;
     DataTypeSlot slot =
         pathSlots.computeIfAbsent(path,

src/main/java/net/hydromatic/morel/compile/Pretty.java

@@ -21,7 +21,6 @@
 import net.hydromatic.morel.ast.Op;
 import net.hydromatic.morel.eval.Codes;
 import net.hydromatic.morel.foreign.RelList;
-import net.hydromatic.morel.type.ApplyType;
 import net.hydromatic.morel.type.DataType;
 import net.hydromatic.morel.type.ForallType;
 import net.hydromatic.morel.type.ListType;
@@ -211,30 +210,25 @@ private StringBuilder pretty2(@Nonnull StringBuilder buf,
       return pretty2(buf, indent, lineEnd, depth + 1, ((ForallType) type).type,
           ImmutableList.of(), value);
 
-    case APPLY_TYPE:
-      final ApplyType applyType = (ApplyType) type;
-      return pretty2(buf, indent, lineEnd, depth + 1, applyType.type,
-          applyType.types, value);
-
     case DATA_TYPE:
       final DataType dataType = (DataType) type;
       //noinspection unchecked,rawtypes
       list = (List) value;
       if (dataType.name.equals("vector")) {
-        final Type argType = Iterables.getOnlyElement(dataType.parameterTypes);
+        final Type argType = Iterables.getOnlyElement(dataType.arguments);
         return printList(buf.append('#'), indent, lineEnd, depth, argType,
             list);
       }
       final String tyConName = (String) list.get(0);
       buf.append(tyConName);
-      final Type typeConArgType = dataType.typeConstructors.get(tyConName);
+      final Type typeConArgType =
+          dataType.typeConstructors(typeSystem).get(tyConName);
+      requireNonNull(typeConArgType);
       if (list.size() == 2) {
         final Object arg = list.get(1);
         buf.append(' ');
         final boolean needParentheses =
-            (typeConArgType.op() == Op.APPLY_TYPE
-                || typeConArgType.op() == Op.DATA_TYPE)
-                && arg instanceof List;
+            typeConArgType.op() == Op.DATA_TYPE && arg instanceof List;
         if (needParentheses) {
           buf.append('(');
         }

src/main/java/net/hydromatic/morel/compile/TypeMap.java

@@ -28,7 +28,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedMap;
-import com.google.common.collect.Lists;
 
 import java.util.ArrayList;
 import java.util.Comparator;
@@ -39,7 +38,7 @@
 import javax.annotation.Nullable;
 
 import static net.hydromatic.morel.util.Pair.forEach;
-import static net.hydromatic.morel.util.Static.skip;
+import static net.hydromatic.morel.util.Static.transform;
 
 /** The result of type resolution, a map from AST nodes to types. */
 public class TypeMap {
@@ -133,15 +132,6 @@ public Type visit(Unifier.Sequence sequence) {
         final Type elementType = sequence.terms.get(0).accept(this);
         return typeMap.typeSystem.listType(elementType);
 
-      case TypeResolver.APPLY_TY_CON:
-        assert sequence.terms.size() == 2;
-        type = sequence.terms.get(0).accept(this);
-        argTypes = ImmutableList.builder();
-        for (Unifier.Term term : skip(sequence.terms)) {
-          argTypes.add(term.accept(this));
-        }
-        return typeMap.typeSystem.apply(type, argTypes.build());
-
       case "bool":
       case "char":
       case "int":
@@ -155,7 +145,7 @@ public Type visit(Unifier.Sequence sequence) {
             return type;
           }
           final List<Type> types =
-              Lists.transform(sequence.terms, t -> t.accept(this));
+              transform(sequence.terms, t -> t.accept(this));
           return typeMap.typeSystem.apply(type, types);
         }
         if (sequence.operator.startsWith(TypeResolver.RECORD_TY_CON)) {