Skip to content

Commit

Permalink
Multi value Complex test and robustness refactoring (#11525)
Browse files Browse the repository at this point in the history
While working on #11482 and enhancing the tests suite with more tests based on `type Complex` a [getRootNode() did not terminate in 100000 iterations](#11525 (comment)) problem was discovered. Detailed investigation revealed that the existing `ReadArgumentCheckNode` infrastructure was able to create a **cycle** of parent pointers in the Truffle AST.

The problem was in intricate manipulation of the AST while rewriting internals in `ReadArgumentCheckNode`. This PR avoids such manipulation by _refactoring the type checking code_. `ReadArgumentNode` knows nothing about types anymore. When a type check is needed, `IrToTruffle` adds additional `TypeCheckValueNode.wrap` around the `ReadArgumentNode` - that breaks the **vicious circle**.

All the _type checks_ nodes are moved to its own package. All but one of the classes are made package private. The external API for doing _type checking_ is concentrated into `TypeCheckValueNode`.
  • Loading branch information
JaroslavTulach authored and somebody1234 committed Nov 21, 2024
1 parent 10e1b76 commit 73b93f5
Show file tree
Hide file tree
Showing 21 changed files with 862 additions and 663 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,22 @@
public final class ReadArgumentNode extends ExpressionNode {
private final int index;
@Child ExpressionNode defaultValue;
@Child ReadArgumentCheckNode checkType;
private final CountingConditionProfile defaultingProfile = CountingConditionProfile.create();

private ReadArgumentNode(int position, ExpressionNode defaultValue, ReadArgumentCheckNode check) {
private ReadArgumentNode(int position, ExpressionNode defaultValue) {
this.index = position;
this.defaultValue = defaultValue;
this.checkType = check;
}

/**
* Creates an instance of this node.
*
* @param position the argument's position at the definition site
* @param defaultValue the default value provided for that argument
* @param check {@code null} or node to check type of input
* @return a node representing the argument at position {@code idx}
*/
public static ReadArgumentNode build(
int position, ExpressionNode defaultValue, ReadArgumentCheckNode check) {
return new ReadArgumentNode(position, defaultValue, check);
}

ReadArgumentNode plainRead() {
var node = (ReadArgumentNode) this.copy();
node.checkType = null;
return node;
public static ReadArgumentNode build(int position, ExpressionNode defaultValue) {
return new ReadArgumentNode(position, defaultValue);
}

/**
Expand Down Expand Up @@ -68,9 +58,6 @@ public Object executeGeneric(VirtualFrame frame) {
v = arguments[index];
}
}
if (checkType != null) {
v = checkType.handleCheckOrConversion(frame, v);
}
return v;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package org.enso.interpreter.node.typecheck;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.Node;
import java.util.List;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.node.expression.builtin.meta.AtomWithAHoleNode;
import org.enso.interpreter.runtime.data.text.Text;
import org.enso.interpreter.runtime.error.DataflowError;

/**
* Root of hierarchy of nodes checking types. This class (and its subclasses) are an implementation
* detail. The API to perform the is in {@link TypeCheckNode}.
*/
abstract sealed class AbstractTypeCheckNode extends Node
permits OneOfTypesCheckNode, AllOfTypesCheckNode, SingleTypeCheckNode, MetaTypeCheckNode {
private final String comment;
@CompilerDirectives.CompilationFinal private String expectedTypeMessage;

AbstractTypeCheckNode(String comment) {
this.comment = comment;
}

abstract Object findDirectMatch(VirtualFrame frame, Object value);

abstract Object executeCheckOrConversion(
VirtualFrame frame, Object value, ExpressionNode valueNode);

abstract String expectedTypeMessage();

/**
* The error message for this node's check. Ready for being used at "fast path".
*
* @return
*/
final String getExpectedTypeMessage() {
if (expectedTypeMessage == null) {
CompilerDirectives.transferToInterpreterAndInvalidate();
expectedTypeMessage = expectedTypeMessage();
}
return expectedTypeMessage;
}

/**
* Composes the comment message describing the kind of check this node performs. Ready for being
* used at "fast path".
*
* @return description of this node's "expectations"
*/
final Text getComment() {
var where = Text.create(comment == null ? "expression" : comment);
var exp = Text.create("expected ");
var got = Text.create(" to be {exp}, but got {got}");
return Text.create(exp, Text.create(where, got));
}

final String joinTypeParts(List<String> parts, String separator) {
assert !parts.isEmpty();
if (parts.size() == 1) {
return parts.get(0);
}

var separatorWithSpace = " " + separator + " ";
var builder = new StringBuilder();
boolean isFirst = true;
for (String part : parts) {
if (isFirst) {
isFirst = false;
} else {
builder.append(separatorWithSpace);
}

// If the part contains a space, it means it is not a single type but already a more complex
// expression with a separator.
// So to ensure we don't mess up the expression layers, we need to add parentheses around it.
boolean needsParentheses = part.contains(" ");
if (needsParentheses) {
builder.append("(");
}
builder.append(part);
if (needsParentheses) {
builder.append(")");
}
}

return builder.toString();
}

static boolean isAllFitValue(Object v) {
return v instanceof DataflowError || AtomWithAHoleNode.isHole(v);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package org.enso.interpreter.node.typecheck;

import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import java.util.Arrays;
import java.util.stream.Collectors;
import org.enso.interpreter.node.ExpressionNode;
import org.enso.interpreter.runtime.data.EnsoMultiValue;
import org.enso.interpreter.runtime.data.Type;
import org.enso.interpreter.runtime.library.dispatch.TypesLibrary;

final class AllOfTypesCheckNode extends AbstractTypeCheckNode {

@Children private AbstractTypeCheckNode[] checks;
@Child private TypesLibrary types;

AllOfTypesCheckNode(String name, AbstractTypeCheckNode[] checks) {
super(name);
this.checks = checks;
this.types = TypesLibrary.getFactory().createDispatched(checks.length);
}

AbstractTypeCheckNode[] getChecks() {
return checks;
}

@Override
Object findDirectMatch(VirtualFrame frame, Object value) {
return null;
}

@Override
@ExplodeLoop
Object executeCheckOrConversion(VirtualFrame frame, Object value, ExpressionNode expr) {
var values = new Object[checks.length];
var valueTypes = new Type[checks.length];
var at = 0;
for (var n : checks) {
var result = n.executeCheckOrConversion(frame, value, expr);
if (result == null) {
return null;
}
values[at] = result;
valueTypes[at] = types.getType(result);
at++;
}
return EnsoMultiValue.create(valueTypes, values);
}

@Override
String expectedTypeMessage() {
var parts =
Arrays.stream(checks)
.map(AbstractTypeCheckNode::expectedTypeMessage)
.collect(Collectors.toList());
return joinTypeParts(parts, "&");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.enso.interpreter.node.typecheck;

import com.oracle.truffle.api.TruffleLanguage;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.RootNode;
import org.enso.interpreter.node.BaseNode;
import org.enso.interpreter.node.callable.thunk.ThunkExecutorNode;
import org.enso.interpreter.runtime.callable.argument.ArgumentDefinition;
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.callable.function.FunctionSchema;

final class LazyCheckRootNode extends RootNode {

@Child private ThunkExecutorNode evalThunk;
@Child private TypeCheckValueNode check;
static final FunctionSchema SCHEMA =
FunctionSchema.newBuilder()
.argumentDefinitions(
new ArgumentDefinition(
0, "delegate", null, null, ArgumentDefinition.ExecutionMode.EXECUTE))
.hasPreapplied(true)
.build();

LazyCheckRootNode(TruffleLanguage<?> language, TypeCheckValueNode check) {
super(language);
this.check = check;
this.evalThunk = ThunkExecutorNode.build();
}

Function wrapThunk(Function thunk) {
return new Function(getCallTarget(), thunk.getScope(), SCHEMA, new Object[] {thunk}, null);
}

@Override
public Object execute(VirtualFrame frame) {
var state = Function.ArgumentsHelper.getState(frame.getArguments());
var args = Function.ArgumentsHelper.getPositionalArguments(frame.getArguments());
assert args.length == 1;
assert args[0] instanceof Function fn && fn.isThunk();
var raw = evalThunk.executeThunk(frame, args[0], state, BaseNode.TailStatus.NOT_TAIL);
var result = check.handleCheckOrConversion(frame, raw, null);
return result;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package org.enso.interpreter.node.typecheck;

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.interop.InteropLibrary;
import com.oracle.truffle.api.interop.UnsupportedMessageException;
import org.enso.interpreter.node.expression.builtin.meta.IsValueOfTypeNode;
import org.enso.interpreter.runtime.util.CachingSupplier;

/**
* Node for checking {@code polyglot java import} types. This class (and its subclasses)
* are an implementation detail. The API to perform the is in {@link TypeCheckNode}.
*/
non-sealed abstract class MetaTypeCheckNode extends AbstractTypeCheckNode {
private final CachingSupplier<? extends Object> expectedSupplier;
@CompilerDirectives.CompilationFinal private String expectedTypeMessage;

MetaTypeCheckNode(String name, CachingSupplier<? extends Object> expectedMetaSupplier) {
super(name);
this.expectedSupplier = expectedMetaSupplier;
}

abstract Object executeCheckOrConversion(VirtualFrame frame, Object value);

@Override
Object findDirectMatch(VirtualFrame frame, Object value) {
return executeCheckOrConversion(frame, value);
}

@Specialization
Object verifyMetaObject(VirtualFrame frame, Object v, @Cached IsValueOfTypeNode isA) {
if (isAllFitValue(v)) {
return v;
}
if (isA.execute(expectedSupplier.get(), v)) {
return v;
} else {
return null;
}
}

@Override
String expectedTypeMessage() {
if (expectedTypeMessage != null) {
return expectedTypeMessage;
}
CompilerDirectives.transferToInterpreterAndInvalidate();
com.oracle.truffle.api.interop.InteropLibrary iop = InteropLibrary.getUncached();
try {
expectedTypeMessage = iop.asString(iop.getMetaQualifiedName(expectedSupplier.get()));
} catch (UnsupportedMessageException ex) {
expectedTypeMessage = expectedSupplier.get().toString();
}
return expectedTypeMessage;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.enso.interpreter.node.typecheck;

import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.ExplodeLoop;
import java.util.Arrays;
import java.util.stream.Collectors;
import org.enso.interpreter.node.ExpressionNode;

final class OneOfTypesCheckNode extends AbstractTypeCheckNode {

@Children private AbstractTypeCheckNode[] checks;

OneOfTypesCheckNode(String name, AbstractTypeCheckNode[] checks) {
super(name);
this.checks = checks;
}

@Override
@ExplodeLoop
final Object findDirectMatch(VirtualFrame frame, Object value) {
for (var n : checks) {
java.lang.Object result = n.findDirectMatch(frame, value);
if (result != null) {
return result;
}
}
return null;
}

@Override
@ExplodeLoop
Object executeCheckOrConversion(VirtualFrame frame, Object value, ExpressionNode expr) {
java.lang.Object direct = findDirectMatch(frame, value);
if (direct != null) {
return direct;
}
for (var n : checks) {
java.lang.Object result = n.executeCheckOrConversion(frame, value, expr);
if (result != null) {
return result;
}
}
return null;
}

@Override
String expectedTypeMessage() {
java.util.List<java.lang.String> parts =
Arrays.stream(checks)
.map(AbstractTypeCheckNode::expectedTypeMessage)
.collect(Collectors.toList());
return joinTypeParts(parts, "|");
}
}
Loading

0 comments on commit 73b93f5

Please sign in to comment.