Don't let the non-Enso types float around the Enso interpreter! (#9584)

This commit is contained in:
Jaroslav Tulach 2024-04-02 06:22:19 +02:00 committed by GitHub
parent 084f4ee033
commit 11e1e9efa0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 149 additions and 65 deletions

View File

@ -40,9 +40,9 @@ public class WarningsTest extends TestBase {
public void doubleWithWarningsWrap() {
var warn1 = Warning.create(ensoContext, "w1", this);
var warn2 = Warning.create(ensoContext, "w2", this);
var value = 42;
var value = 42L;
var with1 = WithWarnings.wrap(ensoContext, 42, warn1);
var with1 = WithWarnings.wrap(ensoContext, value, warn1);
var with2 = WithWarnings.wrap(ensoContext, with1, warn2);
assertEquals(value, with1.getValue());

View File

@ -131,7 +131,6 @@ public final class EnsoFile implements EnsoObject {
@Builtin.Method(name = "creation_time_builtin")
@Builtin.WrapException(from = IOException.class)
@Builtin.ReturningGuestObject
@CompilerDirectives.TruffleBoundary
public EnsoDateTime getCreationTime() throws IOException {
return new EnsoDateTime(
@ -140,7 +139,6 @@ public final class EnsoFile implements EnsoObject {
@Builtin.Method(name = "last_modified_time_builtin")
@Builtin.WrapException(from = IOException.class)
@Builtin.ReturningGuestObject
@CompilerDirectives.TruffleBoundary
public EnsoDateTime getLastModifiedTime() throws IOException {
return new EnsoDateTime(
@ -157,7 +155,7 @@ public final class EnsoFile implements EnsoObject {
@Builtin.Method(name = "parent")
@CompilerDirectives.TruffleBoundary
public Object getParent() {
public EnsoObject getParent() {
var parentOrNull = this.truffleFile.getParent();
if (parentOrNull != null) {
return new EnsoFile(parentOrNull);

View File

@ -60,6 +60,7 @@ public final class ManagedResource implements EnsoObject {
"Takes the value held by the managed resource and removes the finalization callbacks,"
+ " effectively making the underlying resource unmanaged again.")
@Builtin.Specialize
@SuppressWarnings("generic-enso-builtin-type")
public Object take(EnsoContext context) {
context.getResourceManager().take(this);
return this.getResource();

View File

@ -30,6 +30,7 @@ public final class Ref implements EnsoObject {
* @return the current value of the reference.
*/
@Builtin.Method(name = "get", description = "Gets the value stored in the reference")
@SuppressWarnings("generic-enso-builtin-type")
public Object getValue() {
return value;
}
@ -41,6 +42,7 @@ public final class Ref implements EnsoObject {
* @returns the original value
*/
@Builtin.Method(name = "put", description = "Stores a new value in the reference")
@SuppressWarnings("generic-enso-builtin-type")
public Object setValue(Object value) {
Object old = this.value;
this.value = value;

View File

@ -80,6 +80,7 @@ public final class ArrayLikeHelpers {
description = "Creates new Vector with given length and provided elements.",
autoRegister = false)
@Builtin.Specialize()
@SuppressWarnings("generic-enso-builtin-type")
public static Object vectorFromFunction(
VirtualFrame frame,
long length,
@ -123,6 +124,7 @@ public final class ArrayLikeHelpers {
@Builtin.Method(
name = "vector_to_array",
description = "Returns an Array representation of this Vector.")
@SuppressWarnings("generic-enso-builtin-type")
public static Object vectorToArray(Object obj) {
if (obj instanceof Vector.Generic vector) {
return vector.toArray();
@ -132,6 +134,7 @@ public final class ArrayLikeHelpers {
}
@Builtin.Method(name = "new_vector_builder", description = "Returns new vector builder.")
@SuppressWarnings("generic-enso-builtin-type")
public static Object newVectorBuilder(long capacity) {
return ArrayBuilder.newBuilder((int) Math.min(Math.abs(capacity), Integer.MAX_VALUE));
}

View File

@ -176,12 +176,12 @@ final class ArraySlice implements EnsoObject {
}
@ExportMessage
boolean hasType() {
boolean hasType(@Shared("ignore") @Cached("1") int ignore) {
return true;
}
@ExportMessage
Type getType(@Bind("$node") Node node) {
Type getType(@Bind("$node") Node node, @Shared("ignore") @Cached("1") int ignore) {
var ctx = EnsoContext.get(node);
return ctx.getBuiltins().array();
}

View File

@ -43,11 +43,13 @@ public final class Warning implements EnsoObject {
}
@Builtin.Method(name = "value", description = "Gets the payload of the warning.")
@SuppressWarnings("generic-enso-builtin-type")
public Object getValue() {
return value;
}
@Builtin.Method(name = "origin", description = "Gets the payload of the warning.")
@SuppressWarnings("generic-enso-builtin-type")
public Object getOrigin() {
return origin;
}
@ -152,6 +154,7 @@ public final class Warning implements EnsoObject {
description = "Sets all the warnings associated with the value.",
autoRegister = false)
@Builtin.Specialize
@SuppressWarnings("generic-enso-builtin-type")
public static Object set(
EnsoContext ctx, WithWarnings value, Object warnings, InteropLibrary interop) {
return setGeneric(ctx, value.getValue(), interop, warnings);
@ -161,6 +164,7 @@ public final class Warning implements EnsoObject {
name = "set_array",
description = "Sets all the warnings associated with the value.",
autoRegister = false)
@SuppressWarnings("generic-enso-builtin-type")
@Builtin.Specialize(fallback = true)
public static Object set(EnsoContext ctx, Object value, Object warnings, InteropLibrary interop) {
return setGeneric(ctx, value, interop, warnings);

View File

@ -2,6 +2,7 @@ package org.enso.interpreter.runtime.error;
import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.dsl.Cached.Shared;
import com.oracle.truffle.api.interop.TruffleObject;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
import com.oracle.truffle.api.library.CachedLibrary;
import com.oracle.truffle.api.library.ExportLibrary;
@ -39,7 +40,7 @@ public final class WithWarnings implements EnsoObject {
* @param warnings non-empty warnings to be attached to a value
*/
private WithWarnings(Object value, int maxWarnings, boolean limitReached, Warning... warnings) {
assert !(value instanceof WithWarnings);
assert isAcceptableValue(value);
this.warnings = createSetFromArray(maxWarnings, warnings);
assert this.warnings.size() > 0;
this.value = value;
@ -69,7 +70,7 @@ public final class WithWarnings implements EnsoObject {
EconomicSet<Warning> warnings,
boolean limitReached,
Warning... additionalWarnings) {
assert !(value instanceof WithWarnings);
assert isAcceptableValue(value);
this.warnings = cloneSetAndAppend(maxWarnings, warnings, additionalWarnings);
assert this.warnings.size() > 0;
this.value = value;
@ -82,6 +83,18 @@ public final class WithWarnings implements EnsoObject {
this(value, maxWarnings, warnings, false, additionalWarnings);
}
private static boolean isAcceptableValue(Object value) {
assert value != null;
assert !(value instanceof WithWarnings) : "Trying to double wrap WithWarnings " + value;
boolean goodValue =
value instanceof TruffleObject
|| value instanceof Long
|| value instanceof Double
|| value instanceof Boolean;
assert goodValue : "Unexpected value floating around " + value + " type: " + value.getClass();
return goodValue;
}
public static WithWarnings wrap(EnsoContext ctx, Object value, Warning... warnings) {
if (value instanceof WithWarnings with) {
return with.append(ctx, warnings);

View File

@ -36,7 +36,7 @@ public class System {
@Builtin.Method(description = "Check if the operating system is UNIX.", autoRegister = false)
@CompilerDirectives.TruffleBoundary
public static Boolean is_unix() {
public static boolean is_unix() {
return SystemUtils.IS_OS_UNIX;
}

View File

@ -11,12 +11,24 @@ import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.IntStream;
import javax.annotation.processing.*;
import javax.annotation.processing.AbstractProcessor;
import javax.annotation.processing.Processor;
import javax.annotation.processing.RoundEnvironment;
import javax.annotation.processing.SupportedAnnotationTypes;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.*;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.PackageElement;
import javax.lang.model.element.TypeElement;
import javax.tools.Diagnostic;
import javax.tools.Diagnostic.Kind;
import javax.tools.JavaFileObject;
import org.enso.interpreter.dsl.builtins.*;
import org.enso.interpreter.dsl.builtins.ClassName;
import org.enso.interpreter.dsl.builtins.MethodNodeClassGenerator;
import org.enso.interpreter.dsl.builtins.NoSpecializationClassGenerator;
import org.enso.interpreter.dsl.builtins.SpecializationClassGenerator;
import org.enso.interpreter.dsl.builtins.TypeWithKind;
import org.openide.util.lookup.ServiceProvider;
/**
@ -166,6 +178,27 @@ public class BuiltinsProcessor extends AbstractProcessor {
Boolean needsFrame = checkNeedsFrame(element);
boolean isConstructor = method.getKind() == ElementKind.CONSTRUCTOR;
OK:
if (!TypeWithKind.isValidGuestType(processingEnv, method.getReturnType())) {
if (method.getAnnotation(Builtin.ReturningGuestObject.class) != null) {
// guest objects can be of any type
break OK;
}
if (method.getAnnotation(SuppressWarnings.class) instanceof SuppressWarnings sw
&& Arrays.asList(sw.value()).contains("generic-enso-builtin-type")) {
// assume the case was review
break OK;
}
processingEnv
.getMessager()
.printMessage(
Kind.WARNING,
"Suspicious return type: "
+ method.getReturnType()
+ " use @SuppressWarnings(\"generic-enso-builtin-type\") if OK",
method);
}
if (annotation.expandVarargs() != 0) {
if (annotation.expandVarargs() < 0)
throw new RuntimeException(

View File

@ -62,7 +62,7 @@ public final class ExecuteMethodImplGenerator extends MethodGenerator {
}
private String[] auxToHostConversions(MethodParameter param) {
if (param.needsToHostTranslation()) {
if (param.needsToHostTranslation(processingEnvironment)) {
TypeWithKind tpeWithKind = TypeWithKind.createFromTpe(param.tpe());
String hostParam = param.hostVarName();
String tmpObject = "itemsOf" + param.capitalizedName();
@ -100,7 +100,10 @@ public final class ExecuteMethodImplGenerator extends MethodGenerator {
paramsApplied =
StringUtils.join(
params.stream()
.flatMap(x -> x.paramUseNames(expandVararg(paramsLen, x.index())))
.flatMap(
x ->
x.paramUseNames(
processingEnvironment, expandVararg(paramsLen, x.index())))
.toArray(),
", ");
}
@ -125,7 +128,7 @@ public final class ExecuteMethodImplGenerator extends MethodGenerator {
+ "));"
};
default:
if (returnTpe.isValidGuestType()) {
if (returnTpe.isValidGuestType(processingEnvironment) || returnTpe.isObject()) {
return new String[] {" return " + qual + "." + name + "(" + paramsApplied + ");"};
} else {
if (!convertToGuestValue) {
@ -150,12 +153,12 @@ public final class ExecuteMethodImplGenerator extends MethodGenerator {
boolean result = false;
// Does the return value need to be translated to a guest value?
if (!isConstructor && (returnTpe.kind() == TypeKind.OBJECT)) {
if (!returnTpe.isValidGuestType()) {
if (!returnTpe.isValidGuestType(processingEnvironment)) {
result = true;
}
}
// Do any of params need to be translated to a host object?
return result || params.stream().anyMatch(p -> p.needsToHostTranslation());
return result || params.stream().anyMatch(p -> p.needsToHostTranslation(processingEnvironment));
}
public List<String> generate(String name, String owner) {

View File

@ -7,7 +7,12 @@ import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.*;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.AnnotationValue;
import javax.lang.model.element.Element;
import javax.lang.model.element.Name;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.SimpleAnnotationValueVisitor9;
import javax.lang.model.util.SimpleElementVisitor9;
@ -90,7 +95,7 @@ public abstract class MethodGenerator {
else {
switch (tpe.kind()) {
case OBJECT:
if (tpe.isValidGuestType()) {
if (tpe.isValidGuestType(processingEnvironment) || tpe.isObject()) {
return tpe.baseType();
} else {
if (!convertToGuestValue) {
@ -123,7 +128,7 @@ public abstract class MethodGenerator {
String ensoName =
CaseFormat.LOWER_CAMEL.to(CaseFormat.LOWER_UNDERSCORE, v.getSimpleName().toString());
TypeWithKind tpe = TypeWithKind.createFromTpe(v.asType().toString());
if (tpe.kind() == TypeKind.ARRAY && !tpe.isValidGuestType()) {
if (tpe.kind() == TypeKind.ARRAY && !tpe.isValidGuestType(processingEnvironment)) {
processingEnv
.getMessager()
.printMessage(

View File

@ -4,6 +4,7 @@ import java.util.List;
import java.util.Optional;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import javax.annotation.processing.ProcessingEnvironment;
import org.apache.commons.lang3.StringUtils;
/**
@ -58,11 +59,11 @@ public record MethodParameter(int index, String name, String tpe, List<String> a
*
* @return true, if it needs, false otherwise.
*/
public boolean needsToHostTranslation() {
public boolean needsToHostTranslation(ProcessingEnvironment env) {
TypeWithKind tpeWithKind = TypeWithKind.createFromTpe(tpe);
switch (tpeWithKind.kind()) {
case ARRAY:
return !tpeWithKind.isValidGuestType();
return !tpeWithKind.isValidGuestType(env);
default:
return false;
}
@ -99,8 +100,8 @@ public record MethodParameter(int index, String name, String tpe, List<String> a
* @param expand For a non-empty value n, the parameter must be repeated n-times.
* @return A string representation of the parameter variable, potetnially repeated for varargs
*/
public Stream<String> paramUseNames(Optional<Integer> expand) {
if (needsToHostTranslation()) {
public Stream<String> paramUseNames(ProcessingEnvironment env, Optional<Integer> expand) {
if (needsToHostTranslation(env)) {
return Stream.of(hostVarName());
} else {
return expand

View File

@ -2,6 +2,7 @@ package org.enso.interpreter.dsl.builtins;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.ExecutableElement;
import javax.tools.Diagnostic.Kind;
/** Generator for builtin method class with no specialization. */
public final class NoSpecializationClassGenerator extends MethodNodeClassGenerator {
@ -34,8 +35,18 @@ public final class NoSpecializationClassGenerator extends MethodNodeClassGenerat
@Override
protected MethodGenerator methodsGen() {
var asGuestValue = needsGuestValueConversion(origin);
if (asGuestValue
&& TypeWithKind.isTruffleObject(processingEnvironment, origin.getReturnType())) {
processingEnvironment
.getMessager()
.printMessage(
Kind.ERROR,
"Value is already TruffleObject, don't use @Builtin.ReturningGuestObject",
origin);
}
return new ExecuteMethodImplGenerator(
processingEnvironment, origin, needsGuestValueConversion(origin), varArgExpansion);
processingEnvironment, origin, asGuestValue, varArgExpansion);
}
@Override

View File

@ -288,11 +288,18 @@ public final class SpecializedMethodsGenerator extends MethodGenerator {
+ "));");
break;
default:
if (returnTpe.isValidGuestType()) {
if (returnTpe.isValidGuestType(processingEnvironment)) {
methodBody.add(" return " + qual + "." + name + "(" + paramsApplied + ");");
} else if (convertToGuestValue) {
methodBody.add(" var result = " + qual + "." + name + "(" + paramsApplied + ");");
methodBody.add(" return EnsoContext.get(this).asGuestValue(result);");
} else if (returnTpe.isObject()) {
methodBody.add(" var result = " + qual + "." + name + "(" + paramsApplied + ");");
methodBody.add(
" assert result instanceof Long || result instanceof Double || result instanceof"
+ " Boolean || result instanceof"
+ " com.oracle.truffle.api.interop.TruffleObject;");
methodBody.add(" return result;");
} else {
processingEnvironment
.getMessager()

View File

@ -1,8 +1,7 @@
package org.enso.interpreter.dsl.builtins;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.type.TypeMirror;
/**
* TypeWithKind provides a convenience wrapper for the types that can be encountered in builtins
@ -23,42 +22,46 @@ public record TypeWithKind(String baseType, TypeKind kind) {
}
}
// Heuristic
boolean isValidGuestType() {
return baseType.startsWith("java.lang")
|| primitiveTypes.contains(baseType)
|| validGuestTypes.contains(baseType);
boolean isValidGuestType(ProcessingEnvironment env) {
var typeMirror =
switch (baseType) {
case "long" -> env.getTypeUtils().getPrimitiveType(javax.lang.model.type.TypeKind.LONG);
case "double" -> env.getTypeUtils()
.getPrimitiveType(javax.lang.model.type.TypeKind.DOUBLE);
case "boolean" -> env.getTypeUtils()
.getPrimitiveType(javax.lang.model.type.TypeKind.BOOLEAN);
default -> {
var less = baseType.indexOf('<');
var erased = less == -1 ? baseType : baseType.substring(0, less);
var elem = env.getElementUtils().getTypeElement(erased);
if (elem == null) {
throw new NullPointerException("Cannot find " + baseType + " type");
}
yield elem.asType();
}
};
return isValidGuestType(env, typeMirror);
}
private static final List<String> primitiveTypes =
Stream.of(Boolean.TYPE, Long.TYPE, Double.TYPE, Float.TYPE)
.map(Class::getSimpleName)
.collect(Collectors.toList());
public static boolean isValidGuestType(ProcessingEnvironment processingEnv, TypeMirror type) {
return switch (type.getKind()) {
case BOOLEAN, LONG, DOUBLE -> true;
case VOID -> true;
case DECLARED -> isTruffleObject(processingEnv, type);
default -> false;
};
}
/**
* A list of hard-coded types that can be used in the parameter or return type position that are
* valid host types i.e extend TruffleObject. Cannot go via reflection and check that they
* implement the interface, unfortunately.
*/
private static final List<String> validGuestTypes =
List.of(
"org.enso.interpreter.runtime.data.EnsoObject",
"org.enso.interpreter.runtime.data.atom.Atom",
"org.enso.interpreter.runtime.callable.function.Function",
"org.enso.interpreter.runtime.data.hash.EnsoHashMap",
"org.enso.interpreter.runtime.data.vector.Array",
"org.enso.interpreter.runtime.data.vector.Vector",
"org.enso.interpreter.runtime.data.vector.ArrayOverBuffer",
"org.enso.interpreter.runtime.data.vector.ArrayProxy",
"org.enso.interpreter.runtime.data.EnsoFile",
"org.enso.interpreter.runtime.data.EnsoDate",
"org.enso.interpreter.runtime.data.EnsoDateTime",
"org.enso.interpreter.runtime.data.EnsoTimeOfDay",
"org.enso.interpreter.runtime.data.EnsoTimeZone",
"org.enso.interpreter.runtime.data.EnsoDuration",
"org.enso.interpreter.runtime.data.ManagedResource",
"org.enso.interpreter.runtime.data.Ref",
"org.enso.interpreter.runtime.data.text.Text",
"org.enso.interpreter.runtime.error.Warning",
"org.enso.interpreter.runtime.error.WithWarnings");
static boolean isTruffleObject(ProcessingEnvironment processingEnv, TypeMirror type) {
var truffleObject =
processingEnv
.getElementUtils()
.getTypeElement("com.oracle.truffle.api.interop.TruffleObject");
var isSubclass = processingEnv.getTypeUtils().isSubtype(type, truffleObject.asType());
return isSubclass;
}
boolean isObject() {
return "java.lang.Object".equals(baseType());
}
}