Skip to content

Commit

Permalink
[MOREL-203] Cannot deduce type for 'from b in [SOME true, NONE]'
Browse files Browse the repository at this point in the history
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'

Add shortcut method, 'String Static.str(StringBuilder)', to
get the contents of a StringBuilder and clear it.

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

Fixes hydromatic#203
  • Loading branch information
julianhyde committed Nov 4, 2023
1 parent dd1533e commit bc247af
Show file tree
Hide file tree
Showing 36 changed files with 635 additions and 832 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions src/main/java/net/hydromatic/morel/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -373,9 +374,7 @@ protected BufferingReader(Reader in) {
}

public String flush() {
final String s = buf.toString();
buf.setLength(0);
return s;
return str(buf);
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/net/hydromatic/morel/Shell.java
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/net/hydromatic/morel/ast/Ast.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/net/hydromatic/morel/ast/Core.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/net/hydromatic/morel/ast/CoreBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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. */
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/net/hydromatic/morel/ast/Op.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public enum Op {

// miscellaneous
BAR(" | "),
COMMA(", "),
COMMA(","),
FUN_BIND(" and "),
FUN_MATCH,
TY_CON,
Expand All @@ -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,
Expand Down
22 changes: 11 additions & 11 deletions src/main/java/net/hydromatic/morel/compile/BuiltIn.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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. */
Expand Down
22 changes: 9 additions & 13 deletions src/main/java/net/hydromatic/morel/compile/Compiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
});
}
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/main/java/net/hydromatic/morel/compile/Compiles.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 5 additions & 11 deletions src/main/java/net/hydromatic/morel/compile/Pretty.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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('(');
}
Expand Down
14 changes: 2 additions & 12 deletions src/main/java/net/hydromatic/morel/compile/TypeMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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":
Expand All @@ -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)) {
Expand Down
Loading

0 comments on commit bc247af

Please sign in to comment.