-
Notifications
You must be signed in to change notification settings - Fork 337
Fix cyclic dependencies bug #1180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
2d4f6bc
38c1873
70c34b0
4e1a856
bd74777
b68bfff
cb8059f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. | ||
|
@@ -16,9 +16,11 @@ | |
package org.pkl.core.ast.type; | ||
|
||
import com.oracle.truffle.api.nodes.IndirectCallNode; | ||
import com.oracle.truffle.api.nodes.Node; | ||
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; | ||
|
@@ -73,11 +75,38 @@ 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.getModuleInfo().isAmend()) { | ||
// this is an amending module. Try to find the prototype | ||
var proto = findPrototypeModule(this, importedModule); | ||
if (proto == null) { | ||
throw exceptionBuilder() | ||
.evalError("cannotFindModuleImport", importName) | ||
.withSourceSection(importNameSection) | ||
.build(); | ||
} | ||
return proto; | ||
} | ||
|
||
module.setCachedValue(importName, result); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. |
||
} | ||
return (VmTyped) result; | ||
} | ||
|
||
public static @Nullable VmTyped findPrototypeModule(Node node, VmTyped notInitializedModule) { | ||
VmTyped amendingModule = null; | ||
var moduleInfo = notInitializedModule.getModuleInfo(); | ||
var amendedModuleKey = moduleInfo.getAmendedModuleKey(); | ||
|
||
while (amendedModuleKey != null) { | ||
amendingModule = VmLanguage.get(node).loadModule(amendedModuleKey, node); | ||
moduleInfo = amendingModule.getModuleInfo(); | ||
amendedModuleKey = moduleInfo.getAmendedModuleKey(); | ||
} | ||
bioball marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return amendingModule; | ||
} | ||
|
||
protected @Nullable Object getType( | ||
VmTyped module, Identifier typeName, SourceSection typeNameSection) { | ||
var member = module.getMember(typeName); | ||
|
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 "amendsFooLv2.pkl" | ||
|
||
res: amendsFooLv2.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 @@ | ||
amends "amendsFoo.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,3 @@ | ||
import "../../input-helper/cycles/amendsFooLv2.pkl" | ||
|
||
foo: amendsFooLv2.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" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
foo = "bar" |
There was a problem hiding this comment.
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.