Fix incorrect isNoopTypeCheck() calls (#1717)

`isNoopTypeCheck()` is not reliable when inside a TypeNode's constructor. This defers these calls to node execution time.
This commit is contained in:
Jen Basch
2026-07-02 14:24:55 -07:00
committed by GitHub
parent 08b0975af9
commit 6a4d3acf21
5 changed files with 59 additions and 40 deletions
@@ -849,19 +849,14 @@ public abstract class TypeNode extends PklNode {
public static class UnionTypeNode extends WriteFrameSlotTypeNode {
@Children final TypeNode[] elementTypeNodes;
private final boolean skipElementTypeChecks;
private final int defaultIndex;
public UnionTypeNode(
SourceSection sourceSection,
int defaultIndex,
TypeNode[] elementTypeNodes,
boolean skipElementTypeChecks) {
SourceSection sourceSection, int defaultIndex, TypeNode[] elementTypeNodes) {
super(sourceSection);
assert elementTypeNodes.length > 0;
this.elementTypeNodes = elementTypeNodes;
this.defaultIndex = defaultIndex;
this.skipElementTypeChecks = skipElementTypeChecks;
}
@Override
@@ -879,7 +874,10 @@ public abstract class TypeNode extends PklNode {
@Override
public boolean isNoopTypeCheck() {
return skipElementTypeChecks;
for (var element : elementTypeNodes) {
if (!element.isNoopTypeCheck()) return false;
}
return true;
}
@Override
@@ -985,8 +983,6 @@ public abstract class TypeNode extends PklNode {
@Fallback
@ExplodeLoop
protected Object executeLazily(VirtualFrame frame, Object value) {
if (skipElementTypeChecks) return value;
// escape analysis should remove this allocation in compiled code
var typeMismatches = new VmTypeMismatchException[elementTypeNodes.length];
@@ -1037,8 +1033,6 @@ public abstract class TypeNode extends PklNode {
@Override
public Object executeEagerly(VirtualFrame frame, Object value) {
if (skipElementTypeChecks) return value;
// escape analysis should remove this allocation in compiled code
var typeMismatches = new VmTypeMismatchException[elementTypeNodes.length];
@@ -1162,7 +1156,6 @@ public abstract class TypeNode extends PklNode {
@Child private TypeNode elementTypeNode;
public CollectionTypeNode(SourceSection sourceSection, TypeNode elementTypeNode) {
super(sourceSection);
this.elementTypeNode = elementTypeNode;
}
@@ -1272,12 +1265,10 @@ public abstract class TypeNode extends PklNode {
public static final class ListTypeNode extends ObjectSlotTypeNode {
@Child private TypeNode elementTypeNode;
private final boolean skipElementTypeChecks;
public ListTypeNode(SourceSection sourceSection, TypeNode elementTypeNode) {
super(sourceSection);
this.elementTypeNode = elementTypeNode;
skipElementTypeChecks = elementTypeNode.isNoopTypeCheck();
}
@Override
@@ -1322,7 +1313,7 @@ public abstract class TypeNode extends PklNode {
if (!(value instanceof VmList vmList)) {
throw typeMismatch(value, BaseModule.getListClass());
}
if (skipElementTypeChecks) return vmList;
if (elementTypeNode.isNoopTypeCheck()) return vmList;
for (var elem : vmList) {
elementTypeNode.executeEagerly(frame, elem);
@@ -1339,7 +1330,7 @@ public abstract class TypeNode extends PklNode {
if (!(value instanceof VmList vmList)) {
throw typeMismatch(value, BaseModule.getListClass());
}
if (skipElementTypeChecks) return vmList;
if (elementTypeNode.isNoopTypeCheck()) return vmList;
var ret = vmList;
var idx = 0;
@@ -1371,12 +1362,10 @@ public abstract class TypeNode extends PklNode {
public abstract static class SetTypeNode extends ObjectSlotTypeNode {
@Child private TypeNode elementTypeNode;
private final boolean skipElementTypeChecks;
protected SetTypeNode(SourceSection sourceSection, TypeNode elementTypeNode) {
super(sourceSection);
this.elementTypeNode = elementTypeNode;
skipElementTypeChecks = elementTypeNode.isNoopTypeCheck();
}
@Override
@@ -1426,7 +1415,7 @@ public abstract class TypeNode extends PklNode {
@Specialization
protected Object eval(VirtualFrame frame, VmSet value) {
if (skipElementTypeChecks) return value;
if (elementTypeNode.isNoopTypeCheck()) return value;
for (var elem : value) {
// no point doing a lazy check because set members have their hash code computed, which
// necessarily deep-forces them.
@@ -1451,14 +1440,11 @@ public abstract class TypeNode extends PklNode {
public static final class MapTypeNode extends ObjectSlotTypeNode {
@Child private TypeNode keyTypeNode;
@Child private TypeNode valueTypeNode;
private final boolean skipEntryTypeChecks;
public MapTypeNode(SourceSection sourceSection, TypeNode keyTypeNode, TypeNode valueTypeNode) {
super(sourceSection);
this.keyTypeNode = keyTypeNode;
this.valueTypeNode = valueTypeNode;
skipEntryTypeChecks = keyTypeNode.isNoopTypeCheck() && valueTypeNode.isNoopTypeCheck();
}
@Override
@@ -1531,7 +1517,7 @@ public abstract class TypeNode extends PklNode {
}
private Object eval(VirtualFrame frame, VmMap value) {
if (skipEntryTypeChecks) return value;
if (keyTypeNode.isNoopTypeCheck() && valueTypeNode.isNoopTypeCheck()) return value;
var ret = value;
for (var entry : value) {
@@ -1548,7 +1534,7 @@ public abstract class TypeNode extends PklNode {
}
private Object evalEager(VirtualFrame frame, VmMap value) {
if (skipEntryTypeChecks) return value;
if (keyTypeNode.isNoopTypeCheck() && valueTypeNode.isNoopTypeCheck()) return value;
for (var entry : value) {
keyTypeNode.executeEagerly(frame, VmUtils.getKey(entry));
valueTypeNode.executeEagerly(frame, VmUtils.getValue(entry));
@@ -1712,9 +1698,6 @@ public abstract class TypeNode extends PklNode {
@Child protected TypeNode valueTypeNode;
@Child @Nullable protected ListingOrMappingTypeCastNode valueTypeCastNode;
private final boolean skipKeyTypeChecks;
private final boolean skipValueTypeChecks;
protected ListingOrMappingTypeNode(
SourceSection sourceSection,
VmLanguage language,
@@ -1725,9 +1708,6 @@ public abstract class TypeNode extends PklNode {
this.language = language;
this.keyTypeNode = keyTypeNode;
this.valueTypeNode = valueTypeNode;
skipKeyTypeChecks = keyTypeNode == null || keyTypeNode.isNoopTypeCheck();
skipValueTypeChecks = valueTypeNode.isNoopTypeCheck();
}
private boolean isListing() {
@@ -1849,7 +1829,11 @@ public abstract class TypeNode extends PklNode {
}
protected void doEagerCheck(VirtualFrame frame, VmObject object) {
doEagerCheck(frame, object, skipKeyTypeChecks, skipValueTypeChecks);
doEagerCheck(
frame,
object,
keyTypeNode == null || keyTypeNode.isNoopTypeCheck(),
valueTypeNode.isNoopTypeCheck());
}
protected void doEagerCheck(
@@ -2262,7 +2246,6 @@ public abstract class TypeNode extends PklNode {
public PairTypeNode(
SourceSection sourceSection, TypeNode firstTypeNode, TypeNode secondTypeNode) {
super(sourceSection);
this.firstTypeNode = firstTypeNode;
this.secondTypeNode = secondTypeNode;
@@ -2773,6 +2756,21 @@ public abstract class TypeNode extends PklNode {
}
}
@Override
public Object executeEagerly(VirtualFrame frame, Object value) {
var prevOwner = VmUtils.getOwner(frame);
var prevReceiver = VmUtils.getReceiver(frame);
setOwner(frame, VmUtils.getOwner(typeAlias.getEnclosingFrame()));
setReceiver(frame, VmUtils.getReceiver(typeAlias.getEnclosingFrame()));
try {
return aliasedTypeNode.executeEagerly(frame, value);
} finally {
setOwner(frame, prevOwner);
setReceiver(frame, prevReceiver);
}
}
/** See docstring on {@link TypeAliasTypeNode#executeLazily}. */
@Override
public Object executeAndSet(VirtualFrame frame, Object value) {
@@ -361,16 +361,13 @@ public abstract class UnresolvedTypeNode extends PklNode {
CompilerDirectives.transferToInterpreter();
var elementTypeNodes = new TypeNode[unresolvedElementTypeNodes.length];
var skipElementTypeChecks = true;
for (var i = 0; i < elementTypeNodes.length; i++) {
var elementTypeNode = unresolvedElementTypeNodes[i].execute(frame);
elementTypeNodes[i] = elementTypeNode;
skipElementTypeChecks &= elementTypeNode.isNoopTypeCheck();
}
return new UnionTypeNode(
sourceSection, defaultIndex, elementTypeNodes, skipElementTypeChecks);
return new UnionTypeNode(sourceSection, defaultIndex, elementTypeNodes);
}
}
@@ -645,10 +645,7 @@ public final class RendererNodes {
hasString && elements.size() == 1
? elements.get(0)
: new UnionTypeNode(
VmUtils.unavailableSourceSection(),
-1,
elements.toArray(new TypeNode[0]),
false);
VmUtils.unavailableSourceSection(), -1, elements.toArray(new TypeNode[0]));
return type;
}
return type;
@@ -0,0 +1,20 @@
typealias MyList<T> = List<T>
res1 = List(new Dynamic {}) is MyList<module>
typealias MySet<T> = Set<T>
res2 = Set(new Dynamic {}) is MySet<module>
typealias MyMap<T> = Map<Any, T>
res3 = Map("foo", new Dynamic {}) is MyMap<module>
typealias MyListing<T> = Listing<T>
res4 = new Listing { new Dynamic {} } is MyListing<module>
typealias MyMapping<K, V> = Mapping<K, V>
res5 = new Mapping { ["foo"] = new Dynamic {} } is MyMapping<String, module>
typealias MyUnion<A, B> = A | B
res6 = new Dynamic {} is MyUnion<module, module>
typealias NestedUnion<A, B> = (A | A) | (B | B)
res7 = new Dynamic {} is NestedUnion<module, module>
@@ -0,0 +1,7 @@
res1 = false
res2 = false
res3 = false
res4 = false
res5 = false
res6 = false
res7 = false