From a7a5be79b440ddbde54cd70e69cae5a00c01d317 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Tue, 25 Jan 2022 16:44:09 +0100 Subject: [PATCH 1/4] removed accidental normalization of ADT type parameters to void as an erroneous side-effect of Type.instantiate --- .../vallang/type/AbstractDataType.java | 21 +++++++++---------- .../vallang/type/FunctionType.java | 13 ------------ .../io/usethesource/vallang/type/Type.java | 14 +++++++++++++ .../vallang/basic/TypeFactorySmokeTest.java | 12 +++++++++-- 4 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/type/AbstractDataType.java b/src/main/java/io/usethesource/vallang/type/AbstractDataType.java index dc0d57484..8c49f7d0c 100644 --- a/src/main/java/io/usethesource/vallang/type/AbstractDataType.java +++ b/src/main/java/io/usethesource/vallang/type/AbstractDataType.java @@ -374,21 +374,20 @@ public boolean equals(@Nullable Object o) { @Override public Type instantiate(Map bindings) { - if (bindings.isEmpty()) { + if (bindings.isEmpty() || !isParameterized()) { return this; } - Type[] params = new Type[0]; - if (isParameterized()) { - params = new Type[fParameters.getArity()]; - int i = 0; - for (Type p : fParameters) { - params[i++] = p.instantiate(bindings); - } - } - TypeStore store = new TypeStore(); - store.declareAbstractDataType(this); + + // TODO: find out why we had this call + // store.declareAbstractDataType(this); + + // Here it is important _not_ to reuse TupleType.instantiate, since + // that has a normalizing feature to `void` if one of the parameters + // reduced to `void`. The type parameters of an ADT + // can be void and still the ADT type can have values, + Type params = instantiateTuple((TupleType) fParameters, bindings); return TypeFactory.getInstance().abstractDataType(store, fName, params); } diff --git a/src/main/java/io/usethesource/vallang/type/FunctionType.java b/src/main/java/io/usethesource/vallang/type/FunctionType.java index 2d7e23dd0..e3e8098b0 100644 --- a/src/main/java/io/usethesource/vallang/type/FunctionType.java +++ b/src/main/java/io/usethesource/vallang/type/FunctionType.java @@ -418,19 +418,6 @@ public Type instantiate(Map bindings) { return TF.functionType(returnType.instantiate(bindings), instantiateTuple(argumentTypes, bindings), keywordParameters != null ? instantiateTuple(keywordParameters, bindings) : TypeFactory.getInstance().tupleEmpty()); } - /** - * Instantiate a tuple but do not reduce to void like TupleType.instantiate would - * if one of the parameters was substituted by void - */ - private Type instantiateTuple(TupleType t, Map bindings) { - Type[] fChildren = new Type[t.getArity()]; - for (int i = t.fFieldTypes.length - 1; i >= 0; i--) { - fChildren[i] = t.getFieldType(i).instantiate(bindings); - } - - return TypeFactory.getInstance().getFromCache(new TupleType(fChildren)); - } - @Override public boolean isOpen() { return returnType.isOpen() || argumentTypes.isOpen(); diff --git a/src/main/java/io/usethesource/vallang/type/Type.java b/src/main/java/io/usethesource/vallang/type/Type.java index 9cbd1feec..870f15b7c 100644 --- a/src/main/java/io/usethesource/vallang/type/Type.java +++ b/src/main/java/io/usethesource/vallang/type/Type.java @@ -664,6 +664,20 @@ protected final boolean isSubtypeOfAlias(Type type) { return isSubtypeOf(type.getAliased()); } + /** + * Instantiate a tuple but do not reduce to void like TupleType.instantiate would + * if one of the parameters was substituted by void. This is needed for other types + * which use TupleType as an array of Types (i.e. for type parameters, fields and keyword fields) + */ + protected Type instantiateTuple(TupleType t, Map bindings) { + Type[] fChildren = new Type[t.getArity()]; + for (int i = t.fFieldTypes.length - 1; i >= 0; i--) { + fChildren[i] = t.getFieldType(i).instantiate(bindings); + } + + return TypeFactory.getInstance().getFromCache(new TupleType(fChildren)); + } + abstract protected boolean isSubtypeOfReal(Type type); abstract protected boolean isSubtypeOfInteger(Type type); abstract protected boolean isSubtypeOfRational(Type type); diff --git a/src/test/java/io/usethesource/vallang/basic/TypeFactorySmokeTest.java b/src/test/java/io/usethesource/vallang/basic/TypeFactorySmokeTest.java index a9a2bbb8d..7620e0e71 100644 --- a/src/test/java/io/usethesource/vallang/basic/TypeFactorySmokeTest.java +++ b/src/test/java/io/usethesource/vallang/basic/TypeFactorySmokeTest.java @@ -12,6 +12,7 @@ package io.usethesource.vallang.basic; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -51,8 +52,15 @@ public void testGetInstance() { @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void testGetTypeByDescriptor() { - // TODO: needs to be tested, after we've implemented it + public void testParametrizedAbstractDatatypeArityInvariant(TypeStore store) { + Type param = ft.parameterType("T"); + Type adt = ft.abstractDataType(store, "Test", ft.tupleType(param)); + Map bindings = new HashMap<>(); + bindings.put(param, ft.voidType()); + Type instant = adt.instantiate(bindings); + + assertTrue(instant.isParameterized()); + assertFalse(instant.getTypeParameters().isBottom()); } @ParameterizedTest From 28a311984ec8a37936f9e72fffbb324b60f3a7c7 Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 26 Jan 2022 10:30:25 +0100 Subject: [PATCH 2/4] added specific test for the current issue --- .../vallang/specification/TypeTest.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/test/java/io/usethesource/vallang/specification/TypeTest.java b/src/test/java/io/usethesource/vallang/specification/TypeTest.java index dcea01a00..d7468834d 100644 --- a/src/test/java/io/usethesource/vallang/specification/TypeTest.java +++ b/src/test/java/io/usethesource/vallang/specification/TypeTest.java @@ -10,6 +10,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Random; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -25,6 +26,7 @@ import io.usethesource.vallang.type.Type; import io.usethesource.vallang.type.TypeFactory; import io.usethesource.vallang.type.TypeStore; +import io.usethesource.vallang.type.TypeFactory.RandomTypesConfig; public class TypeTest { @@ -349,7 +351,7 @@ public void testGetTypeDescriptor(Type t1, Type t2) { } @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void testMatchAndInstantiate(TypeFactory ft) { + public void testMatchAndInstantiate(TypeFactory ft, TypeStore store) { Type X = ft.parameterType("X"); Map bindings = new HashMap<>(); @@ -390,8 +392,15 @@ public void testMatchAndInstantiate(TypeFactory ft) { fail("instantiate failed"); } + bindings.clear(); + Type adtX = ft.abstractDataType(store, "A", X); + bindings.put(X, ft.voidType()); + assertTrue(adtX.instantiate(bindings).isSubtypeOf(adtX)); + + bindings.put(X, ft.integerType()); + assertTrue(adtX.instantiate(bindings).isSubtypeOf(adtX)); } - + @ParameterizedTest @ArgumentsSource(ValueProvider.class) public void testAlias(TypeFactory ft) { Type alias = ft.aliasType(new TypeStore(), "myValue", ft.valueType()); From 9bc4e3ac9489a52e3882a26eef0837c8b73b448a Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 26 Jan 2022 10:32:23 +0100 Subject: [PATCH 3/4] called wrong method --- .../java/io/usethesource/vallang/type/AbstractDataType.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/usethesource/vallang/type/AbstractDataType.java b/src/main/java/io/usethesource/vallang/type/AbstractDataType.java index 8c49f7d0c..0240b19ee 100644 --- a/src/main/java/io/usethesource/vallang/type/AbstractDataType.java +++ b/src/main/java/io/usethesource/vallang/type/AbstractDataType.java @@ -389,7 +389,7 @@ public Type instantiate(Map bindings) { // can be void and still the ADT type can have values, Type params = instantiateTuple((TupleType) fParameters, bindings); - return TypeFactory.getInstance().abstractDataType(store, fName, params); + return TypeFactory.getInstance().abstractDataTypeFromTuple(store, fName, params); } @Override From ab36f9e4a9e65ab3b4a490d6d8f435b0ab22667e Mon Sep 17 00:00:00 2001 From: "Jurgen J. Vinju" Date: Wed, 26 Jan 2022 10:56:14 +0100 Subject: [PATCH 4/4] removed unused imports --- .../java/io/usethesource/vallang/specification/TypeTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/io/usethesource/vallang/specification/TypeTest.java b/src/test/java/io/usethesource/vallang/specification/TypeTest.java index d7468834d..f56a06459 100644 --- a/src/test/java/io/usethesource/vallang/specification/TypeTest.java +++ b/src/test/java/io/usethesource/vallang/specification/TypeTest.java @@ -10,7 +10,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.Random; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -26,7 +25,6 @@ import io.usethesource.vallang.type.Type; import io.usethesource.vallang.type.TypeFactory; import io.usethesource.vallang.type.TypeStore; -import io.usethesource.vallang.type.TypeFactory.RandomTypesConfig; public class TypeTest {