Improve method.isConst() checks in InvokeMethodVirtualNode

- Check `method.isConst()` every time a method is resolved
  instead of once per node instance (`isConstChecked`).
  Given that `needsConst` only happens in very specific circumstances,
  I'm not entirely sure if every resolved method needs to be checked.
  However, it's a cleaner solution in any case, and `method.isConst()`
  is a fast check that also never happens on the `evalCached` fast path.
- Do not check for const-ness of `FunctionN.apply` methods.
  This check seems unnecessary and would always fail if triggered.
  (According to `base.pkl`, none of the `FunctionN.apply` methods is const.)
- Remove unnecessary Truffle boundaries for modifier checks,
  which are just bitwise operations.
- Improve const/import docs.
This commit is contained in:
translatenix
2024-04-17 14:04:01 -07:00
committed by Daniel Chao
parent 76f1b92039
commit 5de90d5868
4 changed files with 11 additions and 35 deletions
@@ -989,8 +989,8 @@ Because a `local` property is added to the lexical scope, but not (observably) t
==== ====
An _import clause_ defines a local property in the containing module. An _import clause_ defines a local property in the containing module.
This means `import "someModule.pkl"` is equivalent to `local someModule = import("someModule.pkl")`. This means `import "someModule.pkl"` is effectively `const local someModule = import("someModule.pkl")`.
Also, `import "someModule.pkl" as otherName` is equivalent to `local otherName = import("someModule.pkl")`. Also, `import "someModule.pkl" as otherName` is effectively `const local otherName = import("someModule.pkl")`.
==== ====
[[fixed-properties]] [[fixed-properties]]
@@ -16,7 +16,6 @@
package org.pkl.core.ast.expression.member; package org.pkl.core.ast.expression.member;
import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
import com.oracle.truffle.api.RootCallTarget; import com.oracle.truffle.api.RootCallTarget;
import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Cached;
import com.oracle.truffle.api.dsl.ImportStatic; import com.oracle.truffle.api.dsl.ImportStatic;
@@ -44,7 +43,6 @@ public abstract class InvokeMethodVirtualNode extends ExpressionNode {
@Children private final ExpressionNode[] argumentNodes; @Children private final ExpressionNode[] argumentNodes;
private final MemberLookupMode lookupMode; private final MemberLookupMode lookupMode;
private final boolean needsConst; private final boolean needsConst;
@CompilationFinal private boolean isConstChecked;
protected InvokeMethodVirtualNode( protected InvokeMethodVirtualNode(
SourceSection sourceSection, SourceSection sourceSection,
@@ -101,10 +99,9 @@ public abstract class InvokeMethodVirtualNode extends ExpressionNode {
protected Object evalFunction( protected Object evalFunction(
VirtualFrame frame, VirtualFrame frame,
VmFunction receiver, VmFunction receiver,
VmClass receiverClass, @SuppressWarnings("unused") VmClass receiverClass,
@Cached("create()") IndirectCallNode callNode) { @Cached("create()") IndirectCallNode callNode) {
checkConst(receiverClass);
var args = new Object[2 + argumentNodes.length]; var args = new Object[2 + argumentNodes.length];
args[0] = receiver.getThisValue(); args[0] = receiver.getThisValue();
args[1] = receiver; args[1] = receiver;
@@ -125,7 +122,6 @@ public abstract class InvokeMethodVirtualNode extends ExpressionNode {
@Cached("resolveMethod(receiverClass)") ClassMethod method, @Cached("resolveMethod(receiverClass)") ClassMethod method,
@Cached("create(method.getCallTarget(sourceSection))") DirectCallNode callNode) { @Cached("create(method.getCallTarget(sourceSection))") DirectCallNode callNode) {
checkConst(method);
var args = new Object[2 + argumentNodes.length]; var args = new Object[2 + argumentNodes.length];
args[0] = receiver; args[0] = receiver;
args[1] = method.getOwner(); args[1] = method.getOwner();
@@ -145,8 +141,6 @@ public abstract class InvokeMethodVirtualNode extends ExpressionNode {
@Cached("create()") IndirectCallNode callNode) { @Cached("create()") IndirectCallNode callNode) {
var method = resolveMethod(receiverClass); var method = resolveMethod(receiverClass);
checkConst(method);
var args = new Object[2 + argumentNodes.length]; var args = new Object[2 + argumentNodes.length];
args[0] = receiver; args[0] = receiver;
args[1] = method.getOwner(); args[1] = method.getOwner();
@@ -161,7 +155,10 @@ public abstract class InvokeMethodVirtualNode extends ExpressionNode {
protected ClassMethod resolveMethod(VmClass receiverClass) { protected ClassMethod resolveMethod(VmClass receiverClass) {
var method = receiverClass.getMethod(methodName); var method = receiverClass.getMethod(methodName);
if (method != null) return method; if (method != null) {
checkConst(method);
return method;
}
CompilerDirectives.transferToInterpreter(); CompilerDirectives.transferToInterpreter();
@@ -174,17 +171,10 @@ public abstract class InvokeMethodVirtualNode extends ExpressionNode {
.build(); .build();
} }
private void checkConst(VmClass receiverClass) {
checkConst(resolveMethod(receiverClass));
}
private void checkConst(ClassMethod method) { private void checkConst(ClassMethod method) {
if (needsConst && !isConstChecked) { if (needsConst && !method.isConst()) {
CompilerDirectives.transferToInterpreterAndInvalidate(); CompilerDirectives.transferToInterpreter();
if (!method.isConst()) { throw exceptionBuilder().evalError("methodMustBeConst", methodName.toString()).build();
throw exceptionBuilder().evalError("methodMustBeConst", methodName.toString()).build();
}
isConstChecked = true;
} }
} }
} }
@@ -194,7 +194,7 @@ public final class ResolveVariableNode extends ExpressionNode {
} }
} }
// Assuming this method exists at all, it must be a method accessible through `this`. // Assuming this property exists at all, it must be a property accessible through `this`.
/// ///
// Reading a property off of implicit `this` needs a const check if this node is not in a const // Reading a property off of implicit `this` needs a const check if this node is not in a const
// scope. // scope.
@@ -15,7 +15,6 @@
*/ */
package org.pkl.core.ast.member; package org.pkl.core.ast.member;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import com.oracle.truffle.api.source.SourceSection; import com.oracle.truffle.api.source.SourceSection;
import org.pkl.core.ast.VmModifier; import org.pkl.core.ast.VmModifier;
import org.pkl.core.runtime.Identifier; import org.pkl.core.runtime.Identifier;
@@ -76,42 +75,34 @@ public abstract class Member {
/** For use in user-facing messages. Non-null iff getName() is non-null. */ /** For use in user-facing messages. Non-null iff getName() is non-null. */
public abstract @Nullable String getCallSignature(); public abstract @Nullable String getCallSignature();
@TruffleBoundary
public final boolean isLocal() { public final boolean isLocal() {
return VmModifier.isLocal(modifiers); return VmModifier.isLocal(modifiers);
} }
@TruffleBoundary
public final boolean isConst() { public final boolean isConst() {
return VmModifier.isConst(modifiers); return VmModifier.isConst(modifiers);
} }
@TruffleBoundary
public final boolean isFixed() { public final boolean isFixed() {
return VmModifier.isFixed(modifiers); return VmModifier.isFixed(modifiers);
} }
@TruffleBoundary
public final boolean isHidden() { public final boolean isHidden() {
return VmModifier.isHidden(modifiers); return VmModifier.isHidden(modifiers);
} }
@TruffleBoundary
public final boolean isExternal() { public final boolean isExternal() {
return VmModifier.isExternal(modifiers); return VmModifier.isExternal(modifiers);
} }
@TruffleBoundary
public final boolean isClass() { public final boolean isClass() {
return VmModifier.isClass(modifiers); return VmModifier.isClass(modifiers);
} }
@TruffleBoundary
public final boolean isTypeAlias() { public final boolean isTypeAlias() {
return VmModifier.isTypeAlias(modifiers); return VmModifier.isTypeAlias(modifiers);
} }
@TruffleBoundary
public final boolean isImport() { public final boolean isImport() {
return VmModifier.isImport(modifiers); return VmModifier.isImport(modifiers);
} }
@@ -120,27 +111,22 @@ public abstract class Member {
return VmModifier.isGlob(modifiers); return VmModifier.isGlob(modifiers);
} }
@TruffleBoundary
public final boolean isAbstract() { public final boolean isAbstract() {
return VmModifier.isAbstract(modifiers); return VmModifier.isAbstract(modifiers);
} }
@TruffleBoundary
public final boolean isType() { public final boolean isType() {
return VmModifier.isType(modifiers); return VmModifier.isType(modifiers);
} }
@TruffleBoundary
public final boolean isLocalOrExternalOrHidden() { public final boolean isLocalOrExternalOrHidden() {
return VmModifier.isLocalOrExternalOrHidden(modifiers); return VmModifier.isLocalOrExternalOrHidden(modifiers);
} }
@TruffleBoundary
public final boolean isConstOrFixed() { public final boolean isConstOrFixed() {
return VmModifier.isConstOrFixed(modifiers); return VmModifier.isConstOrFixed(modifiers);
} }
@TruffleBoundary
public final boolean isLocalOrExternalOrAbstract() { public final boolean isLocalOrExternalOrAbstract() {
return VmModifier.isLocalOrExternalOrAbstract(modifiers); return VmModifier.isLocalOrExternalOrAbstract(modifiers);
} }