diff --git a/pkl-core/src/main/java/org/pkl/core/ast/member/ClassNode.java b/pkl-core/src/main/java/org/pkl/core/ast/member/ClassNode.java index 79a6f4cb3..ed5e3989d 100644 --- a/pkl-core/src/main/java/org/pkl/core/ast/member/ClassNode.java +++ b/pkl-core/src/main/java/org/pkl/core/ast/member/ClassNode.java @@ -123,37 +123,45 @@ public VmClass executeGeneric(VirtualFrame frame) { typeParameters, prototype); - if (unresolvedSupertypeNode != null) { - var supertypeNode = unresolvedSupertypeNode.execute(frame); - var superclass = supertypeNode.getVmClass(); + var localContext = VmLanguage.get(this).localContext.get(); + localContext.beginClassInit(cachedClass); - checkSupertype(supertypeNode, superclass); - cachedClass.initSupertype(supertypeNode, superclass); - } + try { + if (unresolvedSupertypeNode != null) { + var supertypeNode = unresolvedSupertypeNode.execute(frame); + var superclass = supertypeNode.getVmClass(); - // The superclass resolved above may not itself have completed the below initializations yet. - // That's because these initializations may have indirectly or directly triggered - // resolution of this class, in which case the `resolveSuperclass()` call above - // will have returned the partially initialized `cachedClass` of the superclass. - // As a consequence, initializations that require a fully initialized class hierarchy - // are done lazily in VmClass rather than here. - // A fully initialized class hierarchy is only required for initialization of internal caches, - // which is guaranteed to succeed (no impact on eager vs. lazy error reporting) and easy to - // defer. + checkSupertype(supertypeNode, superclass); + cachedClass.initSupertype(supertypeNode, superclass); + } - VmUtils.evaluateAnnotations(frame, annotationNodes, annotations); + // The superclass resolved above may not itself have completed the below initializations yet. + // That's because these initializations may have indirectly or directly triggered + // resolution of this class, in which case the `resolveSuperclass()` call above + // will have returned the partially initialized `cachedClass` of the superclass. + // As a consequence, initializations that require a fully initialized class hierarchy + // are done lazily in VmClass rather than here. + // A fully initialized class hierarchy is only required for initialization of internal caches, + // which is guaranteed to succeed (no impact on eager vs. lazy error reporting) and easy to + // defer. - for (var node : unresolvedPropertyNodes) { - cachedClass.addProperty(node.execute(frame, cachedClass)); - } + VmUtils.evaluateAnnotations(frame, annotationNodes, annotations); - for (var node : unresolvedMethodNodes) { - cachedClass.addMethod(node.execute(frame, cachedClass)); - } + for (var node : unresolvedPropertyNodes) { + cachedClass.addProperty(node.execute(frame, cachedClass)); + } - cachedClass.notifyInitialized(); + for (var node : unresolvedMethodNodes) { + cachedClass.addMethod(node.execute(frame, cachedClass)); + } - return cachedClass; + cachedClass.onOwnClassInitialized(); + localContext.endClassInit(); + return cachedClass; + } catch (Throwable e) { + localContext.clearClassInitState(); + throw e; + } } private void checkSupertype(TypeNode supertypeNode, @Nullable VmClass superclass) { diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmClass.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmClass.java index 68614aa86..a78d2b3d1 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmClass.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmClass.java @@ -33,6 +33,7 @@ import org.pkl.core.ast.*; import org.pkl.core.ast.member.*; import org.pkl.core.ast.type.TypeNode; +import org.pkl.core.runtime.VmExceptionBuilder.MultilineValue; import org.pkl.core.util.CollectionUtils; import org.pkl.core.util.EconomicMaps; import org.pkl.core.util.LateInit; @@ -150,6 +151,56 @@ public void initSupertype(TypeNode supertypeNode, VmClass superclass) { prototype.lateInitParent(superclass.getPrototype()); } + @TruffleBoundary + private void checkAbstractMethods() { + if (this.isAbstract()) return; + // minimize allocations in the non-error case + if (!hasAbstractMethod()) return; + var abstractMethods = getAbstractMethods(); + if (abstractMethods.size() == 1) { + throw new VmExceptionBuilder() + .evalError( + "noImplementationForAbstractMethod", + getDisplayName(), + abstractMethods.get(0).getName().toString()) + .withSourceSection(getHeaderSection()) + .build(); + } + var methodList = new ArrayList(abstractMethods.size()); + for (var method : abstractMethods) { + methodList.add(method.getCallSignature()); + } + throw new VmExceptionBuilder() + .evalError( + "noImplementationForAbstractMethods", getDisplayName(), MultilineValue.of(methodList)) + .withSourceSection(getHeaderSection()) + .build(); + } + + private boolean hasAbstractMethod() { + var methodCursor = getAllMethods().getEntries(); + while (methodCursor.advance()) { + var method = methodCursor.getValue(); + if (method.isAbstract()) { + return true; + } + } + return false; + } + + private List getAbstractMethods() { + assert this.superclass != null; + var result = new ArrayList(); + var methodCursor = getAllMethods().getEntries(); + while (methodCursor.advance()) { + var method = methodCursor.getValue(); + if (method.isAbstract()) { + result.add(method); + } + } + return result; + } + @TruffleBoundary public void addProperty(ClassProperty property) { prototype.addProperty(property.getInitializer()); @@ -190,11 +241,20 @@ public void addMethods(Iterable methods) { } } - // Note: Superclasses may not have finished their initialization when this method is called. - public void notifyInitialized() { + /** + * Called when this class itself has been initialized. + * + *

Superclasses may not have been initialized yet. + */ + public void onOwnClassInitialized() { isInitialized = true; } + /** Called when the entire class hierarchy is completely initialized, including superclasses. */ + public void onFullyInitialized() { + checkAbstractMethods(); + } + public int getTypeParameterCount() { return typeParameters.size(); } diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmLocalContext.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmLocalContext.java index f3f799ce8..36c213277 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmLocalContext.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmLocalContext.java @@ -15,6 +15,9 @@ */ package org.pkl.core.runtime; +import java.util.ArrayDeque; +import java.util.Deque; + /** A per-context thread-local value that can be used to influence execution. */ public class VmLocalContext { private boolean shouldEagerTypecheck = false; @@ -22,6 +25,12 @@ public class VmLocalContext { /** Whether we are currently inside a type test ({@code is} check). */ private boolean inTypeTest = false; + /** The number of classes currently being initialized. */ + private int classDepth = 0; + + /** The classes currently being initialized. */ + private final Deque pendingClasses = new ArrayDeque<>(); + /** * Number of active {@link VmValueTracker} instances. Used to determine if instrumentation is * already active. @@ -48,6 +57,27 @@ public boolean isInTypeTest() { return inTypeTest; } + public void beginClassInit(VmClass vmClass) { + classDepth++; + pendingClasses.add(vmClass); + } + + public void endClassInit() { + classDepth--; + if (classDepth > 0) { + return; + } + while (!pendingClasses.isEmpty()) { + var clazz = pendingClasses.pop(); + clazz.onFullyInitialized(); + } + } + + public void clearClassInitState() { + pendingClasses.clear(); + classDepth = 0; + } + public void enterTracker() { activeTrackerDepth++; instrumentationEverUsed = true; diff --git a/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties b/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties index b1e52a977..f75b8fbbf 100644 --- a/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties +++ b/pkl-core/src/main/resources/org/pkl/core/errorMessages.properties @@ -1208,3 +1208,10 @@ invalidReferenceTypeAnnotationWithConstraint=\ cannotInstallPackageWithNoCache=\ Cannot install package to module cache dir when module cache is disabled. + +noImplementationForAbstractMethod=\ +Class `{0}` should either be declared `abstract`, or should implement method `{1}`. + +noImplementationForAbstractMethods=\ +Class `{0}` should either be declared `abstract`, or implement the following methods:\n\ +{1} diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input-helper/classes/AbstractModule.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/classes/AbstractModule.pkl new file mode 100644 index 000000000..ab2e65b43 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input-helper/classes/AbstractModule.pkl @@ -0,0 +1,3 @@ +abstract module Foo + +abstract function bar(): Int diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented1.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented1.pkl new file mode 100644 index 000000000..7f8838864 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented1.pkl @@ -0,0 +1,8 @@ +abstract class AbstractMethod { + abstract function foo(): Int +} + +class MyClass extends AbstractMethod { +} + +foo: MyClass diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented2.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented2.pkl new file mode 100644 index 000000000..8a7dca4b5 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented2.pkl @@ -0,0 +1,9 @@ +abstract class AbstractMethods { + abstract function foo(): Int + abstract function bar(): Int +} + +class MyClass extends AbstractMethods { +} + +foo: MyClass diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented3.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented3.pkl new file mode 100644 index 000000000..d84a1a6e3 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented3.pkl @@ -0,0 +1 @@ +extends "../../input-helper/classes/AbstractModule.pkl" diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented4.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented4.pkl new file mode 100644 index 000000000..dd4b5f5ff --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/abstractMethodNotImplemented4.pkl @@ -0,0 +1,10 @@ +abstract class AbstractMethod { + abstract function foo(): Int +} + +abstract class AbstractIntermediate extends AbstractMethod + +class MyClass extends AbstractIntermediate { +} + +foo: MyClass diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented1.err b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented1.err new file mode 100644 index 000000000..f3cba3348 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented1.err @@ -0,0 +1,6 @@ +–– Pkl Error –– +Class `abstractMethodNotImplemented1#MyClass` should either be declared `abstract`, or should implement method `foo`. + +x | class MyClass extends AbstractMethod { + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at abstractMethodNotImplemented1 (file:///$snippetsDir/input/errors/abstractMethodNotImplemented1.pkl) diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented2.err b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented2.err new file mode 100644 index 000000000..2c6cd8b89 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented2.err @@ -0,0 +1,8 @@ +–– Pkl Error –– +Class `abstractMethodNotImplemented2#MyClass` should either be declared `abstract`, or implement the following methods: +foo() +bar() + +x | class MyClass extends AbstractMethods { + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at abstractMethodNotImplemented2 (file:///$snippetsDir/input/errors/abstractMethodNotImplemented2.pkl) diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented3.err b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented3.err new file mode 100644 index 000000000..76679da72 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented3.err @@ -0,0 +1,6 @@ +–– Pkl Error –– +Class `abstractMethodNotImplemented3` should either be declared `abstract`, or should implement method `bar`. + +x | extends "../../input-helper/classes/AbstractModule.pkl" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at abstractMethodNotImplemented3 (file:///$snippetsDir/input/errors/abstractMethodNotImplemented3.pkl) diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented4.err b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented4.err new file mode 100644 index 000000000..4a3b1ca53 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/abstractMethodNotImplemented4.err @@ -0,0 +1,6 @@ +–– Pkl Error –– +Class `abstractMethodNotImplemented4#MyClass` should either be declared `abstract`, or should implement method `foo`. + +x | class MyClass extends AbstractIntermediate { + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +at abstractMethodNotImplemented4 (file:///$snippetsDir/input/errors/abstractMethodNotImplemented4.pkl)