Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions pkl-core/src/main/java/org/pkl/core/ast/builder/AstBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ public AstBuilder(
isMethodReturnTypeChecked = !isStdLibModule || IoUtils.isTestMode();
}

public ModuleInfo getModuleInfo() {
return moduleInfo;
}

public static AstBuilder create(
Source source,
VmLanguage language,
Expand All @@ -311,7 +315,14 @@ public static AstBuilder create(
var moduleName = IoUtils.inferModuleName(moduleKey);
moduleInfo =
new ModuleInfo(
sourceSection, headerSection, null, moduleName, moduleKey, resolvedModuleKey, false);
sourceSection,
headerSection,
null,
moduleName,
moduleKey,
resolvedModuleKey,
false,
null);
} else {
var declaredModuleName = moduleDecl.getName();
var moduleName =
Expand All @@ -320,6 +331,20 @@ public static AstBuilder create(
: IoUtils.inferModuleName(moduleKey);
var clause = moduleDecl.getExtendsOrAmendsDecl();
var isAmend = clause != null && clause.getType() == ExtendsOrAmendsClause.Type.AMENDS;

// if this is an amending module, resolve the module being amended
ModuleKey amendedModuleKey = null;
if (isAmend) {
try {
var amendedModuleUri = URI.create(clause.getUrl().getString());
if (!amendedModuleUri.isAbsolute()) {
amendedModuleUri = resolvedModuleKey.getUri().resolve(amendedModuleUri);
}
amendedModuleKey = moduleResolver.resolve(amendedModuleUri, null);
} catch (Exception ignored) {
}
}

moduleInfo =
new ModuleInfo(
sourceSection,
Expand All @@ -328,7 +353,8 @@ public static AstBuilder create(
moduleName,
moduleKey,
resolvedModuleKey,
isAmend);
isAmend,
amendedModuleKey);
}

return new AstBuilder(source, language, moduleInfo, moduleResolver);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,10 @@ public VmClass executeGeneric(VirtualFrame frame) {
// nodes
// via static final fields without having to fear recursive field initialization.
prototype = module;
prototype.setExtraStorage(moduleInfo);
// Only set ModuleInfo if not already set (to handle cyclic dependencies)
if (!prototype.hasExtraStorage()) {
prototype.setExtraStorage(moduleInfo);
}
prototype.addProperties(prototypeMembers);
} else {
prototype =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
* Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,7 @@
import com.oracle.truffle.api.source.SourceSection;
import org.pkl.core.ast.ExpressionNode;
import org.pkl.core.runtime.Identifier;
import org.pkl.core.runtime.VmLanguage;
import org.pkl.core.runtime.VmObjectLike;
import org.pkl.core.runtime.VmTyped;
import org.pkl.core.util.Nullable;
Expand Down Expand Up @@ -73,11 +74,36 @@ protected VmTyped getImport(
var result = module.getCachedValue(importName);
if (result == null) {
result = callNode.call(member.getCallTarget(), module, module, importName);

var importedModule = (VmTyped) result;
if (importedModule.isNotInitialized()
&& importedModule.isModuleObject()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should always be a module object, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Removed the check.

&& importedModule.getModuleInfo().isAmend()) {
// this is an amending module. Try to find the prototype
var prototypeModule = findPrototypeModule(importedModule);
if (prototypeModule != null) {
result = prototypeModule;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result of getImport should always be the prototype in all cases, even when the module has not yet been initialized.


module.setCachedValue(importName, result);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be cacheing this under the same name as the import value. Otherwise, this snippet breaks:

import "amendingFoo.pkl"

// here, `amendingFoo` should resolve to the prototype
res: amendingFoo.Foo

// here, `amendingFoo` should resolve to the import itself
bar = amendingFoo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. cycles2.pkl tests that (fails if you cache).

}
return (VmTyped) result;
}

private @Nullable VmTyped findPrototypeModule(VmTyped amendingModule) {
try {
var moduleInfo = amendingModule.getModuleInfo();
var amendedModuleKey = moduleInfo.getAmendedModuleKey();

if (amendedModuleKey != null) {
return VmLanguage.get(null).loadModule(amendedModuleKey);
Copy link
Member

@bioball bioball Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The amended module key itself may be amending another module; we need to keep going until we find the first non-amending module. Can we add a test for this?

Also, pass the node in when looking up the reference

Suggested change
return VmLanguage.get(null).loadModule(amendedModuleKey);
return VmLanguage.get(this).loadModule(amendedModuleKey, this);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed! I changed the test to have another layer of indirection.

}
} catch (Exception ignored) {
}
return null;
}

protected @Nullable Object getType(
VmTyped module, Identifier typeName, SourceSection typeNameSection) {
var member = module.getMember(typeName);
Expand Down
29 changes: 29 additions & 0 deletions pkl-core/src/main/java/org/pkl/core/runtime/ModuleInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public final class ModuleInfo {
private final ModuleKey moduleKey;
private final ResolvedModuleKey resolvedModuleKey;
private final boolean isAmend;
private final @Nullable ModuleKey amendedModuleKey;

@LateInit private List<VmTyped> annotations;

Expand All @@ -54,6 +55,26 @@ public ModuleInfo(
ModuleKey moduleKey,
ResolvedModuleKey resolvedModuleKey,
boolean isAmend) {
this(
sourceSection,
headerSection,
docComment,
moduleName,
moduleKey,
resolvedModuleKey,
isAmend,
null);
}

public ModuleInfo(
SourceSection sourceSection,
SourceSection headerSection,
SourceSection @Nullable [] docComment,
String moduleName,
ModuleKey moduleKey,
ResolvedModuleKey resolvedModuleKey,
boolean isAmend,
@Nullable ModuleKey amendedModuleKey) {

this.sourceSection = sourceSection;
this.headerSection = headerSection;
Expand All @@ -62,6 +83,7 @@ public ModuleInfo(
this.moduleKey = moduleKey;
this.resolvedModuleKey = resolvedModuleKey;
this.isAmend = isAmend;
this.amendedModuleKey = amendedModuleKey;
}

public void initAnnotations(List<VmTyped> annotations) {
Expand Down Expand Up @@ -179,4 +201,11 @@ public ModuleSchema getModuleSchema(VmTyped module) {
public boolean isAmend() {
return isAmend;
}

/**
* Returns the ModuleKey of the module being amended, or null if this is not an amending module.
*/
public @Nullable ModuleKey getAmendedModuleKey() {
return amendedModuleKey;
}
}
4 changes: 4 additions & 0 deletions pkl-core/src/main/java/org/pkl/core/runtime/VmLanguage.java
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ void initializeModule(
var builder =
AstBuilder.create(
source, this, moduleContext, moduleKey, resolvedModuleKey, moduleResolver);

// set ModuleInfo early to handle cyclic dependencies
emptyModule.setExtraStorage(builder.getModuleInfo());

var moduleNode = builder.visitModule(moduleContext);
moduleNode.getCallTarget().call(emptyModule, emptyModule);
MinPklVersionChecker.check(emptyModule, importNode);
Expand Down
6 changes: 5 additions & 1 deletion pkl-core/src/main/java/org/pkl/core/runtime/VmTyped.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
* Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -60,6 +60,10 @@ public VmClass getVmClass() {
return clazz;
}

public boolean isNotInitialized() {
return clazz == null;
}

public @Nullable VmTyped getParent() {
return (VmTyped) parent;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import "Qux.pkl"

prop: Qux?

typealias Bar = "bar"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import "amendsFoo.pkl"

res: amendsFoo.Bar
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
amends "Foo.pkl"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import "../../input-helper/cycles/amendsFoo.pkl"

prop: amendsFoo
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import "../../input-helper/cycles/amendsFoo.pkl"

foo: amendsFoo.Bar = "bar"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
–– Pkl Error ––
Module `Foo` cannot be extended or used as type because it amends another module.

x | prop: amendsFoo
^^^^^^^^^
at cyclicalAmendsType (file:///$snippetsDir/input/errors/cyclicalAmendsType.pkl)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo = "bar"