Optimize new inference in method args (#1845)

This commit is contained in:
Jen Basch
2026-09-03 13:16:17 -07:00
committed by GitHub
parent 7f5ae2a712
commit 249e563291
15 changed files with 150 additions and 59 deletions
@@ -878,7 +878,7 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
if (resolution instanceof LexicalMethod method) { if (resolution instanceof LexicalMethod method) {
var levelsUp = method.levelsUp(); var levelsUp = method.levelsUp();
var identifier = org.pkl.core.runtime.Identifier.method(name, method.isLocal()); var identifier = org.pkl.core.runtime.Identifier.method(name, method.isLocal());
var args = visitArgumentList(argList); var argInfo = visitArgumentList(argList);
var needsConst = var needsConst =
switch (constLevel) { switch (constLevel) {
case NONE -> false; case NONE -> false;
@@ -891,35 +891,57 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
var getModuleNode = new GetTypeAliasModuleNode(sourceSection); var getModuleNode = new GetTypeAliasModuleNode(sourceSection);
if (method.isObjectMethod()) { if (method.isObjectMethod()) {
return new InvokeQualifiedObjectMethodNode( return new InvokeQualifiedObjectMethodNode(
sourceSection, identifier, args, needsConst, getModuleNode); sourceSection,
identifier,
argInfo.getFirst(),
needsConst,
getModuleNode,
argInfo.getSecond());
} }
if (method.isOnClosedClass() || method.isLocal() || method.isExternal()) { if (method.isOnClosedClass() || method.isLocal() || method.isExternal()) {
return new InvokeQualifiedClassMethodNode( return new InvokeQualifiedClassMethodNode(
sourceSection, identifier, args, needsConst, getModuleNode); sourceSection,
identifier,
argInfo.getFirst(),
needsConst,
getModuleNode,
argInfo.getSecond());
} }
return InvokeMethodVirtualNodeGen.create( return InvokeMethodVirtualNodeGen.create(
sourceSection, sourceSection,
identifier, identifier,
args, argInfo.getFirst(),
MemberLookupMode.IMPLICIT_LEXICAL, MemberLookupMode.IMPLICIT_LEXICAL,
needsConst, needsConst,
argInfo.getSecond(),
getModuleNode, getModuleNode,
GetClassNodeGen.create(null)); GetClassNodeGen.create(null));
} }
if (method.isObjectMethod()) { if (method.isObjectMethod()) {
return new InvokeLexicalObjectMethodNode( return new InvokeLexicalObjectMethodNode(
sourceSection, identifier, levelsUp, args, needsConst); sourceSection,
identifier,
levelsUp,
argInfo.getFirst(),
needsConst,
argInfo.getSecond());
} }
if (method.isOnClosedClass() || method.isLocal() || method.isExternal()) { if (method.isOnClosedClass() || method.isLocal() || method.isExternal()) {
return new InvokeLexicalClassMethodNode( return new InvokeLexicalClassMethodNode(
sourceSection, identifier, levelsUp, args, needsConst); sourceSection,
identifier,
levelsUp,
argInfo.getFirst(),
needsConst,
argInfo.getSecond());
} }
return InvokeMethodVirtualNodeGen.create( return InvokeMethodVirtualNodeGen.create(
sourceSection, sourceSection,
identifier, identifier,
args, argInfo.getFirst(),
MemberLookupMode.IMPLICIT_LEXICAL, MemberLookupMode.IMPLICIT_LEXICAL,
needsConst, needsConst,
argInfo.getSecond(),
levelsUp == 0 ? new GetReceiverNode() : new GetEnclosingReceiverNode(levelsUp), levelsUp == 0 ? new GetReceiverNode() : new GetEnclosingReceiverNode(levelsUp),
GetClassNodeGen.create(null)); GetClassNodeGen.create(null));
} else if (resolution instanceof ImplicitBaseMethod) { } else if (resolution instanceof ImplicitBaseMethod) {
@@ -941,21 +963,25 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
var baseModule = BaseModule.getModule(); var baseModule = BaseModule.getModule();
var method = baseModule.getVmClass().getDeclaredMethod(identifier); var method = baseModule.getVmClass().getDeclaredMethod(identifier);
assert method != null; assert method != null;
var argInfo = visitArgumentList(argList);
return new InvokeMethodDirectNode( return new InvokeMethodDirectNode(
createSourceSection(expr), createSourceSection(expr),
method, method,
new ConstantValueNode(baseModule), new ConstantValueNode(baseModule),
visitArgumentList(argList)); argInfo.getFirst(),
argInfo.getSecond());
} }
} else if (resolution instanceof ImplicitThisMethod) { } else if (resolution instanceof ImplicitThisMethod) {
var isCustomThis = scope.isCustomThisScope(); var isCustomThis = scope.isCustomThisScope();
var needsConst = constLevel == ConstLevel.ALL && constDepth == -1 && !isCustomThis; var needsConst = constLevel == ConstLevel.ALL && constDepth == -1 && !isCustomThis;
var argInfo = visitArgumentList(argList);
return InvokeMethodVirtualNodeGen.create( return InvokeMethodVirtualNodeGen.create(
sourceSection, sourceSection,
org.pkl.core.runtime.Identifier.get(name), org.pkl.core.runtime.Identifier.get(name),
visitArgumentList(argList), argInfo.getFirst(),
MemberLookupMode.IMPLICIT_THIS, MemberLookupMode.IMPLICIT_THIS,
needsConst, needsConst,
argInfo.getSecond(),
VmUtils.createThisNode(VmUtils.unavailableSourceSection(), isCustomThis), VmUtils.createThisNode(VmUtils.unavailableSourceSection(), isCustomThis),
GetClassNodeGen.create(null)); GetClassNodeGen.create(null));
} else { } else {
@@ -1053,7 +1079,9 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
var parent = expr.parent(); var parent = expr.parent();
var scope = symbolTable.getCurrentScope(); var scope = symbolTable.getCurrentScope();
while (parent instanceof IfExpr // keep in sync with isImplicitNewExpr
while (parent instanceof IfExpr ifExpr
&& (ifExpr.getThen() == child || ifExpr.getEls() == child)
|| parent instanceof TraceExpr || parent instanceof TraceExpr
|| parent instanceof LetExpr letExpr && letExpr.getExpr() == child) { || parent instanceof LetExpr letExpr && letExpr.getExpr() == child) {
@@ -1135,8 +1163,9 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
.build(); .build();
} }
var argInfo = visitArgumentList(argCtx);
return InvokeSuperMethodNodeGen.create( return InvokeSuperMethodNodeGen.create(
sourceSection, memberName, visitArgumentList(argCtx), needsConst); sourceSection, memberName, argInfo.getFirst(), needsConst, argInfo.getSecond());
} }
// superproperty call // superproperty call
@@ -1150,11 +1179,10 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
@Override @Override
public ExpressionNode visitQualifiedAccessExpr(QualifiedAccessExpr expr) { public ExpressionNode visitQualifiedAccessExpr(QualifiedAccessExpr expr) {
if (expr.getArgumentList() != null) { var argList = expr.getArgumentList();
return doVisitMethodAccessExpr(expr); return argList != null
} ? doVisitMethodAccessExpr(expr, argList)
: doVisitPropertyInvocationExpr(expr);
return doVisitPropertyInvocationExpr(expr);
} }
@Override @Override
@@ -2306,13 +2334,32 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
} }
@Override @Override
public ExpressionNode[] visitArgumentList(ArgumentList argumentList) { public Pair<ExpressionNode[], Boolean> visitArgumentList(ArgumentList argumentList) {
var args = argumentList.getArguments(); var args = argumentList.getArguments();
var res = new ExpressionNode[args.size()]; var res = new ExpressionNode[args.size()];
var argsRequireInference = false;
for (int i = 0; i < res.length; i++) { for (int i = 0; i < res.length; i++) {
res[i] = visitExpr(args.get(i)); var expr = args.get(i);
res[i] = visitExpr(expr);
argsRequireInference = argsRequireInference || isImplicitNewExpr(expr);
} }
return res; return Pair.of(res, argsRequireInference);
}
private static boolean isImplicitNewExpr(Expr expr) {
// keep in sync with doVisitNewExprWithInferredParent
if (expr instanceof NewExpr newExpr && newExpr.getType() == null) {
return true;
} else if (expr instanceof IfExpr ifExpr) {
return isImplicitNewExpr(ifExpr.getThen()) || isImplicitNewExpr(ifExpr.getEls());
} else if (expr instanceof TraceExpr traceExpr) {
return isImplicitNewExpr(traceExpr.getExpr());
} else if (expr instanceof ParenthesizedExpr parenthesizedExpr) {
return isImplicitNewExpr(parenthesizedExpr.getExpr());
} else if (expr instanceof LetExpr letExpr) {
return isImplicitNewExpr(letExpr.getExpr());
}
return false;
} }
@Override @Override
@@ -2888,12 +2935,12 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
return ReadPropertyNodeGen.create(sourceSection, propertyName, needsConst, receiver); return ReadPropertyNodeGen.create(sourceSection, propertyName, needsConst, receiver);
} }
private ExpressionNode doVisitMethodAccessExpr(QualifiedAccessExpr expr) { private ExpressionNode doVisitMethodAccessExpr(QualifiedAccessExpr expr, ArgumentList argList) {
var sourceSection = createSourceSection(expr); var sourceSection = createSourceSection(expr);
var functionName = toIdentifier(expr.getIdentifier().getValue()); var functionName = toIdentifier(expr.getIdentifier().getValue());
var argCtx = expr.getArgumentList();
var receiver = visitExpr(expr.getExpr()); var receiver = visitExpr(expr.getExpr());
var needsConst = needsConst(receiver); var needsConst = needsConst(receiver);
var argInfo = visitArgumentList(argList);
if (expr.isNullable()) { if (expr.isNullable()) {
//noinspection ConstantConditions //noinspection ConstantConditions
@@ -2902,9 +2949,10 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
InvokeMethodVirtualNodeGen.create( InvokeMethodVirtualNodeGen.create(
sourceSection, sourceSection,
functionName, functionName,
visitArgumentList(argCtx), argInfo.getFirst(),
MemberLookupMode.EXPLICIT_RECEIVER, MemberLookupMode.EXPLICIT_RECEIVER,
needsConst, needsConst,
argInfo.getSecond(),
PropagateNullReceiverNodeGen.create(unavailableSourceSection(), receiver), PropagateNullReceiverNodeGen.create(unavailableSourceSection(), receiver),
GetClassNodeGen.create(null))); GetClassNodeGen.create(null)));
} }
@@ -2913,9 +2961,10 @@ public class AstBuilder extends AbstractAstBuilder<Object> {
return InvokeMethodVirtualNodeGen.create( return InvokeMethodVirtualNodeGen.create(
sourceSection, sourceSection,
functionName, functionName,
visitArgumentList(argCtx), argInfo.getFirst(),
MemberLookupMode.EXPLICIT_RECEIVER, MemberLookupMode.EXPLICIT_RECEIVER,
needsConst, needsConst,
argInfo.getSecond(),
receiver, receiver,
GetClassNodeGen.create(null)); GetClassNodeGen.create(null));
} }
@@ -31,8 +31,9 @@ public abstract sealed class AbstractInvokeLexicalMethodNode
Identifier methodName, Identifier methodName,
int levelsUp, int levelsUp,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
boolean needsConst) { boolean needsConst,
super(sourceSection, methodName, argumentNodes, needsConst); boolean argsRequireInference) {
super(sourceSection, methodName, argumentNodes, needsConst, argsRequireInference);
this.levelsUp = levelsUp; this.levelsUp = levelsUp;
} }
@@ -47,8 +47,9 @@ public abstract sealed class AbstractInvokeLexicalOrQualifiedMethodNode
SourceSection sourceSection, SourceSection sourceSection,
Identifier methodName, Identifier methodName,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
boolean needsConst) { boolean needsConst,
super(sourceSection, argumentNodes); boolean argsRequireInference) {
super(sourceSection, argumentNodes, argsRequireInference);
this.methodName = methodName; this.methodName = methodName;
this.needsConst = needsConst; this.needsConst = needsConst;
this.isConstChecked = false; this.isConstChecked = false;
@@ -28,10 +28,13 @@ import org.pkl.core.runtime.VmUtils;
public abstract class AbstractInvokeMethodNode extends ExpressionNode { public abstract class AbstractInvokeMethodNode extends ExpressionNode {
@Children protected final ExpressionNode[] argumentNodes; @Children protected final ExpressionNode[] argumentNodes;
protected final boolean argsRequireInference;
public AbstractInvokeMethodNode(SourceSection sourceSection, ExpressionNode[] argumentNodes) { public AbstractInvokeMethodNode(
SourceSection sourceSection, ExpressionNode[] argumentNodes, boolean argsRequireInference) {
super(sourceSection); super(sourceSection);
this.argumentNodes = argumentNodes; this.argumentNodes = argumentNodes;
this.argsRequireInference = argsRequireInference;
} }
@TruffleBoundary @TruffleBoundary
@@ -44,10 +47,13 @@ public abstract class AbstractInvokeMethodNode extends ExpressionNode {
@ExplodeLoop @ExplodeLoop
protected Object[] evalArgs( protected Object[] evalArgs(
VirtualFrame frame, @Nullable Method method, Object owner, @Nullable Object receiver) { VirtualFrame frame, @Nullable Method method, Object owner, @Nullable Object receiver) {
// TODO: optimize this away when the call does not contain any implicit new args int methodSlot = -1;
var methodSlot = getMethodSlot(frame.getFrameDescriptor()); Object prevMethod = null;
var prevMethod = frame.getAuxiliarySlot(methodSlot); if (argsRequireInference) {
frame.setAuxiliarySlot(methodSlot, method); methodSlot = getMethodSlot(frame.getFrameDescriptor());
prevMethod = frame.getAuxiliarySlot(methodSlot);
frame.setAuxiliarySlot(methodSlot, method);
}
var args = new Object[2 + argumentNodes.length]; var args = new Object[2 + argumentNodes.length];
args[0] = receiver; args[0] = receiver;
@@ -58,7 +64,9 @@ public abstract class AbstractInvokeMethodNode extends ExpressionNode {
args[2 + i] = argumentNodes[i].executeGeneric(frame); args[2 + i] = argumentNodes[i].executeGeneric(frame);
} }
} finally { } finally {
frame.setAuxiliarySlot(methodSlot, prevMethod); if (argsRequireInference) {
frame.setAuxiliarySlot(methodSlot, prevMethod);
}
} }
return args; return args;
@@ -31,8 +31,9 @@ public abstract sealed class AbstractInvokeQualifiedMethodNode
Identifier methodName, Identifier methodName,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
boolean needsConst, boolean needsConst,
ExpressionNode getReceiverNode) { ExpressionNode getReceiverNode,
super(sourceSection, methodName, argumentNodes, needsConst); boolean argsRequireInference) {
super(sourceSection, methodName, argumentNodes, needsConst, argsRequireInference);
this.getReceiverNode = getReceiverNode; this.getReceiverNode = getReceiverNode;
} }
@@ -31,8 +31,9 @@ public final class InvokeLexicalClassMethodNode extends AbstractInvokeLexicalMet
Identifier methodName, Identifier methodName,
int levelsUp, int levelsUp,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
boolean needsConst) { boolean needsConst,
super(sourceSection, methodName, levelsUp, argumentNodes, needsConst); boolean argsRequireInference) {
super(sourceSection, methodName, levelsUp, argumentNodes, needsConst, argsRequireInference);
} }
@Override @Override
@@ -30,8 +30,9 @@ public final class InvokeLexicalObjectMethodNode extends AbstractInvokeLexicalMe
Identifier methodName, Identifier methodName,
int levelsUp, int levelsUp,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
boolean needsConst) { boolean needsConst,
super(sourceSection, methodName, levelsUp, argumentNodes, needsConst); boolean argsRequireInference) {
super(sourceSection, methodName, levelsUp, argumentNodes, needsConst, argsRequireInference);
} }
@Override @Override
@@ -34,9 +34,9 @@ public final class InvokeMethodDirectNode extends AbstractInvokeMethodNode {
SourceSection sourceSection, SourceSection sourceSection,
ClassMethod method, ClassMethod method,
ExpressionNode receiverNode, ExpressionNode receiverNode,
ExpressionNode[] argumentNodes) { ExpressionNode[] argumentNodes,
boolean argsRequireInference) {
super(sourceSection, argumentNodes); super(sourceSection, argumentNodes, argsRequireInference);
this.method = method; this.method = method;
this.owner = method.getOwner(); this.owner = method.getOwner();
this.receiverNode = receiverNode; this.receiverNode = receiverNode;
@@ -52,9 +52,9 @@ public abstract class InvokeMethodVirtualNode extends AbstractInvokeMethodNode {
Identifier methodName, Identifier methodName,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
MemberLookupMode lookupMode, MemberLookupMode lookupMode,
boolean needsConst) { boolean needsConst,
boolean argsRequireInference) {
super(sourceSection, argumentNodes); super(sourceSection, argumentNodes, argsRequireInference);
this.methodName = methodName; this.methodName = methodName;
this.lookupMode = lookupMode; this.lookupMode = lookupMode;
this.needsConst = needsConst; this.needsConst = needsConst;
@@ -64,8 +64,9 @@ public abstract class InvokeMethodVirtualNode extends AbstractInvokeMethodNode {
SourceSection sourceSection, SourceSection sourceSection,
Identifier methodName, Identifier methodName,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
MemberLookupMode lookupMode) { MemberLookupMode lookupMode,
this(sourceSection, methodName, argumentNodes, lookupMode, false); boolean argsRequireInference) {
this(sourceSection, methodName, argumentNodes, lookupMode, false, argsRequireInference);
} }
/** /**
@@ -145,7 +146,14 @@ public abstract class InvokeMethodVirtualNode extends AbstractInvokeMethodNode {
@Override @Override
public WrapperNode createWrapper(ProbeNode probe) { public WrapperNode createWrapper(ProbeNode probe) {
return new InvokeMethodVirtualNodeWrapper( return new InvokeMethodVirtualNodeWrapper(
sourceSection, methodName, argumentNodes, lookupMode, needsConst, this, probe); sourceSection,
methodName,
argumentNodes,
lookupMode,
needsConst,
argsRequireInference,
this,
probe);
} }
private void checkConst(ClassMethod method) { private void checkConst(ClassMethod method) {
@@ -28,8 +28,15 @@ public final class InvokeQualifiedClassMethodNode extends AbstractInvokeQualifie
Identifier methodName, Identifier methodName,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
boolean needsConst, boolean needsConst,
ExpressionNode getReceiverNode) { ExpressionNode getReceiverNode,
super(sourceSection, methodName, argumentNodes, needsConst, getReceiverNode); boolean argsRequireInference) {
super(
sourceSection,
methodName,
argumentNodes,
needsConst,
getReceiverNode,
argsRequireInference);
} }
@Override @Override
@@ -30,8 +30,15 @@ public final class InvokeQualifiedObjectMethodNode extends AbstractInvokeQualifi
Identifier methodName, Identifier methodName,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
boolean needsConst, boolean needsConst,
ExpressionNode getReceiverNode) { ExpressionNode getReceiverNode,
super(sourceSection, methodName, argumentNodes, needsConst, getReceiverNode); boolean argsRequireInference) {
super(
sourceSection,
methodName,
argumentNodes,
needsConst,
getReceiverNode,
argsRequireInference);
} }
@Override @Override
@@ -35,9 +35,9 @@ public abstract class InvokeSuperMethodNode extends AbstractInvokeMethodNode {
SourceSection sourceSection, SourceSection sourceSection,
Identifier methodName, Identifier methodName,
ExpressionNode[] argumentNodes, ExpressionNode[] argumentNodes,
boolean needsConst) { boolean needsConst,
boolean argsRequireInference) {
super(sourceSection, argumentNodes); super(sourceSection, argumentNodes, argsRequireInference);
this.needsConst = needsConst; this.needsConst = needsConst;
assert !methodName.isLocalMethod(); assert !methodName.isLocalMethod();
@@ -88,6 +88,7 @@ public abstract class ToStringNode extends UnaryExpressionNode {
Identifier.TO_STRING, Identifier.TO_STRING,
new ExpressionNode[] {}, new ExpressionNode[] {},
MemberLookupMode.EXPLICIT_RECEIVER, MemberLookupMode.EXPLICIT_RECEIVER,
false,
null, null,
null); null);
} }
@@ -24,8 +24,11 @@ local quux = (a: Foo) -> a.x
qualified = this.bar(new { x = 1 }) qualified = this.bar(new { x = 1 })
unqualifiedLexical = bar(new { x = 1 }) unqualifiedLexical {
unqualifiedThis = new Qux { x = 7 }.bar(new { x = 1 }) call = bar(new { x = 1 })
}
unqualifiedThis1 = bar(new { x = 1 })
unqualifiedThis2 = new Qux { x = 7 }.bar(new { x = 1 })
`super` = new Foo { x = 2 }.superBar(new Foo { x = 3 }) `super` = new Foo { x = 2 }.superBar(new Foo { x = 3 })
nestedMethodCalls = outerMethod(bar(new { x = 8 }), new { x = 9 }) nestedMethodCalls = outerMethod(bar(new { x = 8 }), new { x = 9 })
objectMethod { objectMethod {
@@ -1,6 +1,9 @@
qualified = 1 qualified = 1
unqualifiedLexical = 1 unqualifiedLexical {
unqualifiedThis = 8 call = 1
}
unqualifiedThis1 = 1
unqualifiedThis2 = 8
`super` = 16 `super` = 16
nestedMethodCalls = 26 nestedMethodCalls = 26
objectMethod { objectMethod {