Skip to content
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

Refactor import resolver to allow for dynamic loading of modules. #1822

Merged
merged 4 commits into from
Jun 28, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
- Implemented changes to the import and export syntax, requiring to provide the
project namespace, or use the new `project` keyword to import from the current
project ([#1806](https://github.com/enso-org/enso/pull/1806)).
- Fixed a bug where unresolved imports would crash the compiler
([#1822](https://github.com/enso-org/enso/pull/1822)).

## Tooling

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class ModuleManagementTest extends AnyFlatSpec with Matchers {
.allowExperimentalOptions(true)
.allowAllAccess(true)
.option(RuntimeOptions.PACKAGES_PATH, pkg.root.getAbsolutePath)
.option(RuntimeOptions.STRICT_ERRORS, "true")
.build()
)

Expand Down Expand Up @@ -97,7 +98,7 @@ class ModuleManagementTest extends AnyFlatSpec with Matchers {
subject should "allow to create new, importable modules" in {
val ctx = new TestContext("Test")
ctx.writeMain("""
|import MyLib.Foo
|import Enso_Test.Test.Foo
|
|main = Foo.foo + 1
|""".stripMargin)
Expand All @@ -111,7 +112,7 @@ class ModuleManagementTest extends AnyFlatSpec with Matchers {

val topScope = ctx.executionContext.getTopScope
topScope.registerModule(
"MyLib.Foo.Main",
"Enso_Test.Test.Foo",
ctx.mkFile("Foo.enso").getAbsolutePath
)

Expand All @@ -124,7 +125,7 @@ class ModuleManagementTest extends AnyFlatSpec with Matchers {
subject should "allow importing literal-source modules" in {
val ctx = new TestContext("Test")
ctx.writeMain("""
|import MyLib.Foo
|import Enso_Test.Test.Foo
|
|main = Foo.foo + 1
|""".stripMargin)
Expand All @@ -138,7 +139,7 @@ class ModuleManagementTest extends AnyFlatSpec with Matchers {

val topScope = ctx.executionContext.getTopScope
val fooModule = topScope.registerModule(
"MyLib.Foo.Main",
"Enso_Test.Test.Foo",
ctx.mkFile("Foo.enso").getAbsolutePath
)
fooModule.setSource("""
Expand Down Expand Up @@ -182,9 +183,6 @@ class ModuleManagementTest extends AnyFlatSpec with Matchers {
)
val exception =
the[PolyglotException] thrownBy mod2.getAssociatedConstructor
exception.getMessage shouldEqual "org.enso.compiler.exception." +
"CompilerError: Compiler Internal Error: Attempted to import the " +
"unresolved module Enso_Test.Test.Main during code generation. Defined " +
"at X2[2:1-2:26]."
exception.getMessage shouldEqual "Compilation aborted due to errors."
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;
import org.enso.compiler.Compiler;
import org.enso.compiler.PackageRepository;
import org.enso.compiler.data.CompilerConfig;
import org.enso.home.HomeManager;
import org.enso.interpreter.Language;
Expand Down Expand Up @@ -78,7 +79,8 @@ public Context(Language language, String home, Env environment) {

builtins = new Builtins(this);

this.compiler = new Compiler(this, builtins, compilerConfig);
this.compiler =
new Compiler(this, builtins, new PackageRepository.Default(this), compilerConfig);
}

/** Perform expensive initialization logic for the context. */
Expand Down Expand Up @@ -389,6 +391,11 @@ public List<ShadowedPackage> getShadowedPackages() {
return shadowedPackages;
}

/** @return the pre-loaded packages */
public List<Package<TruffleFile>> getPackages() {
return packages;
}
Comment on lines +395 to +397
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest renaming to getPreloadedPackages or something like that and indicating in the doc comment that this will be removed in the next iteration. Just to avoid someone using it in a wrong place by mistake.


/** @return the compiler configuration for this language */
public CompilerConfig getCompilerConfig() {
return compilerConfig;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@

/** Container class for static predefined atoms, methods, and their containing scope. */
public class Builtins {
public static final String SOURCE_NAME = "Builtins.enso";
public static final String MODULE_NAME = "Standard.Builtins.Main";
public static final String PACKAGE_NAME = "Builtins";
public static final String NAMESPACE = "Standard";
public static final String SOURCE_NAME = PACKAGE_NAME + ".enso";
public static final String MODULE_NAME = NAMESPACE + "." + PACKAGE_NAME + ".Main";

/** Container for method names needed outside this class. */
public static class MethodNames {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import scala.jdk.OptionConverters._
class Compiler(
val context: Context,
val builtins: Builtins,
val packageRepository: PackageRepository,
config: CompilerConfig
) {
private val freshNameSupply: FreshNameSupply = new FreshNameSupply
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package org.enso.compiler

import org.enso.interpreter.runtime.Context
import org.enso.interpreter.runtime.builtin.Builtins

import scala.jdk.CollectionConverters._

trait PackageRepository {

/** Informs the repository that it should populate the top scope with modules
* belonging to a given package.
*
* @param namespace the namespace of the package.
* @param name the package name.
* @return `Right(())` if the package was already loaded or successfully
* downloaded. A `Left` containing an error otherwise.
*/
def ensurePackageIsLoaded(
namespace: String,
name: String
): Either[PackageRepository.Error, Unit]
}

object PackageRepository {

/** A trait representing errors reported by this system */
sealed trait Error

object Error {

/** An error reported when the requested package does not exist.
*/
case object PackageDoesNotExist extends Error
}

/** A temporary package repository, only able to resolve packages known
* upfront to the language context.
*
* @param context the language context
*/
class Default(context: Context) extends PackageRepository {

/** @inheritdoc */
override def ensurePackageIsLoaded(
namespace: String,
name: String
): Either[PackageRepository.Error, Unit] =
if (
(name == Builtins.PACKAGE_NAME && namespace == Builtins.NAMESPACE) ||
context.getPackages.asScala
.exists(p => p.name == name && p.namespace == namespace)
) Right(())
else Left(Error.PackageDoesNotExist)
}
}
15 changes: 15 additions & 0 deletions engine/runtime/src/main/scala/org/enso/compiler/core/IR.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6630,6 +6630,21 @@ object IR {
s"The `project` keyword was used in an $statementType statement," +
" but the module does not belong to a project."
}

/** Used when an import statement triggers loading of a package that does not exist.
* @param name the module name.
*/
case class PackageDoesNotExist(name: String) extends Reason {
override def message: String = s"Package containing the module $name" +
s" could not be found."
}

/** Used when an import statement refers to a module that does not exist.
* @param name the module name.
*/
case class ModuleDoesNotExist(name: String) extends Reason {
override def message: String = s"The module $name does not exist."
}
}

/** An erroneous import or export statement.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,53 @@ class ImportResolver(compiler: Compiler) {
current.getCompilationStage
.isBefore(Module.CompilationStage.AFTER_IMPORT_RESOLUTION)
) {
val importedModules = ir.imports.flatMap {
case imp: IR.Module.Scope.Import.Module =>
val impName = imp.name.name
val exp = ir.exports
.collect { case ex: IR.Module.Scope.Export.Module => ex }
.find(_.name.name == impName)
compiler
.getModule(impName)
.map(BindingsMap.ResolvedImport(imp, exp, _))
case _ => None
}
currentLocal.resolvedImports = importedModules
val importedModules: List[
(IR.Module.Scope.Import, Option[BindingsMap.ResolvedImport])
] =
ir.imports.map {
case imp: IR.Module.Scope.Import.Module =>
val impName = imp.name.name
val exp = ir.exports
.collect { case ex: IR.Module.Scope.Export.Module => ex }
.find(_.name.name == impName)
val isLoaded = imp.name.parts match {
case namespace :: name :: _ =>
compiler.packageRepository.ensurePackageIsLoaded(
namespace.name,
name.name
).isRight
case _ => false
}
if (isLoaded) {
compiler.getModule(impName) match {
case Some(module) =>
(
imp,
Some(BindingsMap.ResolvedImport(imp, exp, module))
)
case None =>
(
IR.Error.ImportExport(
imp,
IR.Error.ImportExport.ModuleDoesNotExist(impName)
),
None
)
}
} else {
(
IR.Error.ImportExport(
imp,
IR.Error.ImportExport.PackageDoesNotExist(impName)
),
None
)
}
case other => (other, None)
}
currentLocal.resolvedImports = importedModules.flatMap(_._2)
val newIr = ir.copy(imports = importedModules.map(_._1))
current.unsafeSetIr(newIr)
current.unsafeSetCompilationStage(
Module.CompilationStage.AFTER_IMPORT_RESOLUTION
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
name: Test_Bad_Imports
namespace: Enso_Test
license: APLv2
enso-version: default
version: "0.0.1"
author: "Enso Team <[email protected]>"
maintainer: "Enso Team <[email protected]>"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import Surely_This.Does_Not_Exist.My_Module
import Enso_Test.Test_Bad_Imports.Oopsie

main = 10
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,20 @@ class ImportsTest extends PackageTest {
) should have message "Method `method` of X could not be found."
}

"Import statements" should "report errors when they cannot be resolved" in {
the[InterpreterException] thrownBy evalTestProject(
"Test_Bad_Imports"
) should have message "Compilation aborted due to errors."
val outLines = consumeOut
outLines(2) should include(
"Package containing the module Surely_This.Does_Not_Exist.My_Module could" +
" not be found."
)
outLines(3) should include(
"The module Enso_Test.Test_Bad_Imports.Oopsie does not exist."
)
}

"Symbols from imported modules" should "not be visible when imported qualified" in {
the[InterpreterException] thrownBy evalTestProject(
"Test_Qualified_Error"
Expand Down