From ad6504d9013890f3af28d6bc2fbe05808e32402b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 6 Nov 2024 18:13:48 +0100 Subject: [PATCH 01/63] Move ScalaConversions from runtime to engine-common --- build.sbt | 1 + engine/common/src/main/java/module-info.java | 1 + .../src/main/java/org/enso/common}/ScalaConversions.java | 2 +- .../enso/interpreter/instrument/job/ExecuteExpressionJob.java | 2 +- .../java/org/enso/compiler/test/SerializationManagerTest.java | 2 +- .../test/java/org/enso/interpreter/runtime/ModuleTest.java | 2 +- .../java/org/enso/interpreter/test/meta/EnsoProjectTest.java | 2 +- .../org/enso/interpreter/runtime/TruffleCompilerContext.java | 4 ++-- .../org/enso/interpreter/runtime/scope/TopLevelScope.java | 2 +- 9 files changed, 10 insertions(+), 8 deletions(-) rename engine/{runtime/src/main/java/org/enso/interpreter/util => common/src/main/java/org/enso/common}/ScalaConversions.java (98%) diff --git a/build.sbt b/build.sbt index 2a7909868b8b..e6233bedc78b 100644 --- a/build.sbt +++ b/build.sbt @@ -2118,6 +2118,7 @@ lazy val `engine-common` = project .enablePlugins(JPMSPlugin) .settings( frgaalJavaCompilerSetting, + scalaModuleDependencySetting, Test / fork := true, commands += WithDebugCommand.withDebug, Test / envVars ++= distributionEnvironmentOverrides, diff --git a/engine/common/src/main/java/module-info.java b/engine/common/src/main/java/module-info.java index ab242d316c5e..d6404d8e6de2 100644 --- a/engine/common/src/main/java/module-info.java +++ b/engine/common/src/main/java/module-info.java @@ -1,4 +1,5 @@ module org.enso.engine.common { + requires scala.library; requires org.graalvm.polyglot; requires org.enso.logging.utils; requires org.enso.logging.config; diff --git a/engine/runtime/src/main/java/org/enso/interpreter/util/ScalaConversions.java b/engine/common/src/main/java/org/enso/common/ScalaConversions.java similarity index 98% rename from engine/runtime/src/main/java/org/enso/interpreter/util/ScalaConversions.java rename to engine/common/src/main/java/org/enso/common/ScalaConversions.java index aa21c1662039..bf19595cec1f 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/util/ScalaConversions.java +++ b/engine/common/src/main/java/org/enso/common/ScalaConversions.java @@ -1,4 +1,4 @@ -package org.enso.interpreter.util; +package org.enso.common; import java.util.List; import java.util.Optional; diff --git a/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java b/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java index 31db700f7a88..0355694d780b 100644 --- a/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java +++ b/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java @@ -4,7 +4,7 @@ import org.enso.interpreter.instrument.OneshotExpression; import org.enso.interpreter.instrument.execution.Executable; import org.enso.interpreter.instrument.execution.RuntimeContext; -import org.enso.interpreter.util.ScalaConversions; +import org.enso.common.ScalaConversions; /** The job that schedules the execution of the expression. */ public class ExecuteExpressionJob extends Job implements UniqueJob { diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java index 8c29b11ddaf1..12d545b6a3fa 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java @@ -15,7 +15,7 @@ import org.enso.interpreter.runtime.EnsoContext; import org.enso.interpreter.runtime.util.TruffleFileSystem; import org.enso.interpreter.test.InterpreterContext; -import org.enso.interpreter.util.ScalaConversions; +import org.enso.common.ScalaConversions; import org.enso.pkg.Package; import org.enso.pkg.PackageManager; import org.enso.polyglot.Suggestion; diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ModuleTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ModuleTest.java index ee710fb4e2cb..53b2f73dd1ef 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ModuleTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ModuleTest.java @@ -1,6 +1,6 @@ package org.enso.interpreter.runtime; -import static org.enso.interpreter.util.ScalaConversions.nil; +import static org.enso.common.ScalaConversions.nil; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/meta/EnsoProjectTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/meta/EnsoProjectTest.java index 7ba0eb5a1317..017bd4d7956a 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/meta/EnsoProjectTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/meta/EnsoProjectTest.java @@ -9,7 +9,7 @@ import java.util.Set; import org.enso.common.LanguageInfo; import org.enso.common.RuntimeOptions; -import org.enso.interpreter.util.ScalaConversions; +import org.enso.common.ScalaConversions; import org.enso.pkg.QualifiedName; import org.enso.polyglot.PolyglotContext; import org.enso.test.utils.ContextUtils; diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java index cfed0237247f..5669012209b9 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java @@ -1,7 +1,7 @@ package org.enso.interpreter.runtime; -import static org.enso.interpreter.util.ScalaConversions.cons; -import static org.enso.interpreter.util.ScalaConversions.nil; +import static org.enso.common.ScalaConversions.cons; +import static org.enso.common.ScalaConversions.nil; import com.oracle.truffle.api.TruffleFile; import com.oracle.truffle.api.TruffleLogger; diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java index c71f2c6315a6..c62144ce7c4a 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java @@ -24,7 +24,7 @@ import org.enso.interpreter.runtime.data.EnsoObject; import org.enso.interpreter.runtime.data.vector.ArrayLikeHelpers; import org.enso.interpreter.runtime.type.Types; -import org.enso.interpreter.util.ScalaConversions; +import org.enso.common.ScalaConversions; import org.enso.pkg.Package; import org.enso.pkg.QualifiedName; From f811f2ef7311a2369f8e8ddd83d0f03111dfb957 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 6 Nov 2024 19:36:32 +0100 Subject: [PATCH 02/63] Convert ImportSymbolAnalysis to mini pass --- .../pass/analyse/ImportSymbolAnalysis.java | 194 +++++++++++++++ .../pass/analyse/PrivateModuleAnalysis.java | 2 +- .../main/scala/org/enso/compiler/Passes.scala | 2 +- .../analyse/AmbiguousImportsAnalysis.scala | 6 +- .../pass/analyse/ImportSymbolAnalysis.scala | 227 ------------------ .../compiler/core/ir/MetadataStorage.java | 1 + 6 files changed, 200 insertions(+), 232 deletions(-) create mode 100644 engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java delete mode 100644 engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.scala diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java new file mode 100644 index 000000000000..82a29cbc84a4 --- /dev/null +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -0,0 +1,194 @@ +package org.enso.compiler.pass.analyse; + +import java.util.ArrayList; +import java.util.List; +import org.enso.common.ScalaConversions; +import org.enso.compiler.context.InlineContext; +import org.enso.compiler.context.ModuleContext; +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.MetadataStorage; +import org.enso.compiler.core.ir.Module; +import org.enso.compiler.core.ir.Name; +import org.enso.compiler.core.ir.expression.errors.ImportExport; +import org.enso.compiler.core.ir.module.scope.Import; +import org.enso.compiler.data.BindingsMap; +import org.enso.compiler.pass.IRProcessingPass; +import org.enso.compiler.pass.MiniIRPass; +import org.enso.compiler.pass.MiniPassFactory; +import org.enso.compiler.pass.desugar.GenerateMethodBodies$; +import scala.collection.immutable.Seq; +import scala.jdk.javaapi.CollectionConverters; + +/** + * Performs analysis of `from ... import sym1, sym2, ...` statements - checks that all the symbols + * imported from the module can be resolved, i.e., exists. In case of unresolved symbols, replaces + * the IR import with {@link org.enso.compiler.core.ir.expression.errors.ImportExport}. Reports only + * the first unresolved symbol. + */ +public class ImportSymbolAnalysis implements MiniPassFactory { + public static final ImportSymbolAnalysis INSTANCE = new ImportSymbolAnalysis(); + + private ImportSymbolAnalysis() {} + + @Override + public Seq precursorPasses() { + List passes = + List.of(BindingAnalysis$.MODULE$, GenerateMethodBodies$.MODULE$); + return ScalaConversions.seq(passes); + } + + @Override + public Seq invalidatedPasses() { + return ScalaConversions.seq(List.of()); + } + + @Override + public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { + var bindingsMap = moduleContext.bindingsAnalysis(); + return new Mini(bindingsMap); + } + + @Override + public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { + // TODO: Must not return null? + return null; + } + + private static final class Mini extends MiniIRPass { + private final BindingsMap bindingsMap; + + private Mini(BindingsMap bindingsMap) { + this.bindingsMap = bindingsMap; + } + + @Override + public Module transformModule(Module moduleIr) { + for (var imp : CollectionConverters.asJava(moduleIr.imports())) { + var encounteredErrors = analyseSymbolsFromImport((Import.Module) imp); + if (encounteredErrors != null) { + return moduleIr.copy( + encounteredErrors, + moduleIr.exports(), + moduleIr.bindings(), + moduleIr.isPrivate(), + moduleIr.location(), + moduleIr.passData(), + moduleIr.diagnostics(), + moduleIr.id()); + } + } + return moduleIr; + } + + @Override + public Expression transformExpression(Expression expr) { + return expr; + } + + /** Returns list of encountered errors, or null. */ + private scala.collection.immutable.List analyseSymbolsFromImport(Import.Module imp) { + if (imp.onlyNames().isDefined()) { + var resolvedImport = + bindingsMap.resolvedImports().find(resImp -> resImp.importDef() == imp); + if (resolvedImport.isEmpty()) { + return null; + } + var onlyNames = imp.onlyNames().get(); + var importedTargets = resolvedImport.get().targets(); + var unresolvedSymbols = + importedTargets.flatMap( + importedTarget -> onlyNames.filterNot(nm -> isSymbolResolved(importedTarget, nm))); + if (unresolvedSymbols.nonEmpty()) { + return unresolvedSymbols.map( + unresolvedSym -> + createErrorForUnresolvedSymbol(imp, importedTargets.head(), unresolvedSym)); + } + } + + // Importing symbols from methods is not allowed. The following code checks that if the + // import is importing all from a method, an error is reported. + if (imp.isAll() && !imp.isSynthetic()) { + var resolvedImport = + bindingsMap.resolvedImports().find(resImp -> resImp.importDef() == imp); + if (resolvedImport.isEmpty()) { + return null; + } + var importedTargets = resolvedImport.get().targets(); + var encounteredErrors = new ArrayList(); + for (var importedTarget : CollectionConverters.asJava(importedTargets)) { + switch (importedTarget) { + case BindingsMap.ResolvedModuleMethod resModMethod -> { + encounteredErrors.add( + createImportFromMethodError( + imp, + resModMethod.module().getName().toString(), + resModMethod.method().name())); + } + case BindingsMap.ResolvedExtensionMethod extMethod -> { + var staticMethod = extMethod.staticMethod(); + encounteredErrors.add( + createImportFromMethodError( + imp, + extMethod.module().getName().createChild(staticMethod.tpName()).toString(), + staticMethod.methodName())); + } + case BindingsMap.ResolvedConversionMethod resConvMethod -> { + var convMethod = resConvMethod.conversionMethod(); + var module = resConvMethod.module(); + encounteredErrors.add( + createImportFromMethodError( + imp, + module.getName().createChild(convMethod.targetTpName()).toString(), + convMethod.methodName())); + } + default -> {} + } + } + if (!encounteredErrors.isEmpty()) { + return CollectionConverters.asScala(encounteredErrors).toList(); + } + } + return null; + } + + private static boolean isSymbolResolved( + BindingsMap.ImportTarget importTarget, Name.Literal symbol) { + return importTarget.findExportedSymbolsFor(symbol.name()).nonEmpty(); + } + + private static ImportExport createErrorForUnresolvedSymbol( + Import imp, BindingsMap.ImportTarget importTarget, Name.Literal unresolvedSymbol) { + ImportExport.Reason errorReason = + switch (importTarget) { + case BindingsMap.ResolvedModule resMod -> new ImportExport.SymbolDoesNotExist( + unresolvedSymbol.name(), resMod.module().getName().toString()); + case BindingsMap.ResolvedType resType -> new ImportExport.NoSuchConstructor( + resType.tp().name(), unresolvedSymbol.name()); + case BindingsMap.ResolvedConstructor resCons -> new ImportExport.NoSuchConstructor( + resCons.cons().name(), unresolvedSymbol.name()); + case BindingsMap.ResolvedModuleMethod resMethod -> new ImportExport.NoSuchModuleMethod( + resMethod.method().name(), unresolvedSymbol.name()); + case BindingsMap.ResolvedExtensionMethod extMethod -> new ImportExport + .NoSuchStaticMethod( + extMethod.module().getName().toString(), + extMethod.staticMethod().tpName(), + unresolvedSymbol.name()); + case BindingsMap.ResolvedConversionMethod convMethod -> new ImportExport + .NoSuchConversionMethod( + convMethod.module().getName().toString(), + convMethod.conversionMethod().targetTpName(), + convMethod.conversionMethod().sourceTpName()); + default -> throw new IllegalStateException("Unexpected value: " + importTarget); + }; + return new ImportExport(imp, errorReason, MetadataStorage.EMPTY); + } + + private static ImportExport createImportFromMethodError( + Import imp, String moduleName, String methodName) { + return new ImportExport( + imp, + new ImportExport.IllegalImportFromMethod(moduleName, methodName), + MetadataStorage.EMPTY); + } + } +} diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java index 8b772a3a0f7a..bfbb476de4d4 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java @@ -37,7 +37,7 @@ private PrivateModuleAnalysis() {} @Override public Seq precursorPasses() { List passes = - List.of(BindingAnalysis$.MODULE$, ImportSymbolAnalysis$.MODULE$); + List.of(BindingAnalysis$.MODULE$, ImportSymbolAnalysis.INSTANCE); return CollectionConverters.asScala(passes).toList(); } diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala index acdf7361e1e6..47b2b1e51e56 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala @@ -44,7 +44,7 @@ class Passes(config: CompilerConfig) { SectionsToBinOp.INSTANCE, OperatorToFunction, LambdaShorthandToLambda, - ImportSymbolAnalysis, + ImportSymbolAnalysis.INSTANCE, AmbiguousImportsAnalysis ) ++ (if (config.privateCheckEnabled) { List( diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.scala index c27c211e806a..1c5cd1730596 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.scala @@ -9,7 +9,7 @@ import org.enso.compiler.core.ir.module.scope.imports import org.enso.compiler.data.BindingsMap import org.enso.compiler.core.CompilerError import org.enso.compiler.data.BindingsMap.ResolvedName -import org.enso.compiler.pass.IRPass +import org.enso.compiler.pass.{IRPass, IRProcessingPass} import scala.collection.mutable import scala.collection.mutable.ListBuffer @@ -39,8 +39,8 @@ case object AmbiguousImportsAnalysis extends IRPass { override type Config = IRPass.Configuration.Default - override lazy val precursorPasses: Seq[IRPass] = - Seq(BindingAnalysis, ImportSymbolAnalysis) + override lazy val precursorPasses: Seq[IRProcessingPass] = + Seq(BindingAnalysis, ImportSymbolAnalysis.INSTANCE) override lazy val invalidatedPasses: Seq[IRPass] = Seq() diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.scala deleted file mode 100644 index 2f390b5cd039..000000000000 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.scala +++ /dev/null @@ -1,227 +0,0 @@ -package org.enso.compiler.pass.analyse - -import org.enso.compiler.context.{InlineContext, ModuleContext} -import org.enso.compiler.core.Implicits.AsMetadata -import org.enso.compiler.core.ir.{Expression, Module, Name} -import org.enso.compiler.core.ir.module.scope.Import -import org.enso.compiler.core.ir.expression.errors -import org.enso.compiler.data.BindingsMap -import org.enso.compiler.pass.IRPass -import org.enso.compiler.pass.desugar.GenerateMethodBodies - -/** Performs analysis of `from ... import sym1, sym2, ...` statements - checks that all - * the symbols imported from the module can be resolved, i.e., exists. - * In case of unresolved symbols, replaces the IR import with [[errors.ImportExport]]. - * Reports only the first unresolved symbol. - */ -case object ImportSymbolAnalysis extends IRPass { - - override type Metadata = BindingsMap - - override type Config = IRPass.Configuration.Default - - override lazy val precursorPasses: Seq[IRPass] = - Seq(BindingAnalysis, GenerateMethodBodies) - - override lazy val invalidatedPasses: Seq[IRPass] = - Seq() - - /** @inheritdoc - */ - override def runModule( - ir: Module, - moduleContext: ModuleContext - ): Module = { - val bindingMap = ir.unsafeGetMetadata( - BindingAnalysis, - "BindingMap should already be present" - ) - ir.copy( - imports = ir.imports.flatMap(analyseSymbolsFromImport(_, bindingMap)) - ) - } - - /** @inheritdoc - */ - override def runExpression( - ir: Expression, - inlineContext: InlineContext - ): Expression = ir - - /** @return May return multiple [[errors.ImportExport]] in case of multiple unresolved symbols. - */ - private def analyseSymbolsFromImport( - imp: Import, - bindingMap: BindingsMap - ): List[Import] = { - imp match { - case imp @ Import.Module( - _, - _, - _, - Some(onlyNames), - _, - _, - _, - _ - ) => - bindingMap.resolvedImports.find(_.importDef == imp) match { - case Some(resolvedImport) => - val importedTargets = resolvedImport.targets - val unresolvedSymbols = importedTargets.flatMap { importedTarget => - onlyNames.filterNot(isSymbolResolved(importedTarget, _)) - } - if (unresolvedSymbols.nonEmpty) { - unresolvedSymbols - .map( - createErrorForUnresolvedSymbol( - imp, - importedTargets.head, - _ - ) - ) - } else { - List(imp) - } - case None => List(imp) - } - // Importing symbols from methods is not allowed. The following code checks that if the - // import is importing all from a method, an error is reported. - case imp @ Import.Module( - _, - _, - isAll, - _, - _, - _, - isSynthetic, - _ - ) if isAll && !isSynthetic => - bindingMap.resolvedImports.find(_.importDef == imp) match { - case Some(resolvedImport) => - val importedTargets = resolvedImport.targets - val errors = importedTargets.flatMap { importedTarget => - importedTarget match { - case BindingsMap.ResolvedModuleMethod(module, method) => - val err = createImportFromMethodError( - imp, - module.getName.toString, - method.name - ) - Some(err) - case BindingsMap.ResolvedExtensionMethod( - module, - staticMethod - ) => - val err = createImportFromMethodError( - imp, - module.getName.createChild(staticMethod.tpName).toString, - staticMethod.methodName - ) - Some(err) - case BindingsMap.ResolvedConversionMethod( - module, - conversionMethod - ) => - val err = createImportFromMethodError( - imp, - module.getName - .createChild(conversionMethod.targetTpName) - .toString, - conversionMethod.methodName - ) - Some(err) - case _ => None - } - } - if (errors.nonEmpty) { - errors - } else { - List(imp) - } - case None => List(imp) - } - case _ => List(imp) - } - } - - private def createImportFromMethodError( - imp: Import, - moduleName: String, - methodName: String - ): errors.ImportExport = { - errors.ImportExport( - imp, - errors.ImportExport.IllegalImportFromMethod( - moduleName, - methodName - ) - ) - } - - private def createErrorForUnresolvedSymbol( - imp: Import, - importTarget: BindingsMap.ImportTarget, - unresolvedSymbol: Name.Literal - ): errors.ImportExport = { - importTarget match { - case BindingsMap.ResolvedModule(module) => - errors.ImportExport( - imp, - errors.ImportExport.SymbolDoesNotExist( - unresolvedSymbol.name, - module.getName.toString - ) - ) - case BindingsMap.ResolvedType(_, tp) => - errors.ImportExport( - imp, - errors.ImportExport.NoSuchConstructor( - tp.name, - unresolvedSymbol.name - ) - ) - case BindingsMap.ResolvedConstructor(_, cons) => - errors.ImportExport( - imp, - errors.ImportExport.NoSuchConstructor( - cons.name, - unresolvedSymbol.name - ) - ) - case BindingsMap.ResolvedModuleMethod(_, method) => - errors.ImportExport( - imp, - errors.ImportExport.NoSuchModuleMethod( - method.name, - unresolvedSymbol.name - ) - ) - case BindingsMap.ResolvedExtensionMethod(mod, staticMethod) => - errors.ImportExport( - imp, - errors.ImportExport.NoSuchStaticMethod( - moduleName = mod.getName.toString, - typeName = staticMethod.tpName, - methodName = unresolvedSymbol.name - ) - ) - case BindingsMap.ResolvedConversionMethod(mod, conversionMethod) => - errors.ImportExport( - imp, - errors.ImportExport.NoSuchConversionMethod( - moduleName = mod.getName.toString, - targetTypeName = conversionMethod.targetTpName, - sourceTypeName = conversionMethod.sourceTpName - ) - ) - } - } - - private def isSymbolResolved( - importTarget: BindingsMap.ImportTarget, - symbol: Name.Literal - ): Boolean = { - importTarget.findExportedSymbolsFor(symbol.name).nonEmpty - } -} diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java index e0ddd703ae47..4308c90ccea4 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java @@ -16,6 +16,7 @@ /** Stores metadata for the various passes. */ public final class MetadataStorage { private Map metadata; + public static final MetadataStorage EMPTY = new MetadataStorage(); public MetadataStorage() { this(Collections.emptyMap()); From 410bd5592c34450d823e95190f1d931b51112da8 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 6 Nov 2024 19:36:43 +0100 Subject: [PATCH 03/63] fmt --- .../enso/interpreter/instrument/job/ExecuteExpressionJob.java | 2 +- .../java/org/enso/compiler/test/SerializationManagerTest.java | 2 +- .../java/org/enso/interpreter/runtime/scope/TopLevelScope.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java b/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java index 0355694d780b..859ddc19a59c 100644 --- a/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java +++ b/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java @@ -1,10 +1,10 @@ package org.enso.interpreter.instrument.job; import java.util.UUID; +import org.enso.common.ScalaConversions; import org.enso.interpreter.instrument.OneshotExpression; import org.enso.interpreter.instrument.execution.Executable; import org.enso.interpreter.instrument.execution.RuntimeContext; -import org.enso.common.ScalaConversions; /** The job that schedules the execution of the expression. */ public class ExecuteExpressionJob extends Job implements UniqueJob { diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java index 12d545b6a3fa..65d058a20ec9 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java @@ -11,11 +11,11 @@ import org.apache.commons.io.FileUtils; import org.enso.common.LanguageInfo; import org.enso.common.MethodNames; +import org.enso.common.ScalaConversions; import org.enso.editions.LibraryName; import org.enso.interpreter.runtime.EnsoContext; import org.enso.interpreter.runtime.util.TruffleFileSystem; import org.enso.interpreter.test.InterpreterContext; -import org.enso.common.ScalaConversions; import org.enso.pkg.Package; import org.enso.pkg.PackageManager; import org.enso.polyglot.Suggestion; diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java index c62144ce7c4a..ecd7ab3a24d7 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java @@ -15,6 +15,7 @@ import java.util.Optional; import java.util.concurrent.ExecutionException; import org.enso.common.MethodNames; +import org.enso.common.ScalaConversions; import org.enso.compiler.PackageRepository; import org.enso.editions.LibraryName; import org.enso.interpreter.EnsoLanguage; @@ -24,7 +25,6 @@ import org.enso.interpreter.runtime.data.EnsoObject; import org.enso.interpreter.runtime.data.vector.ArrayLikeHelpers; import org.enso.interpreter.runtime.type.Types; -import org.enso.common.ScalaConversions; import org.enso.pkg.Package; import org.enso.pkg.QualifiedName; From fd3814aa56ce5cda5b62e568c3e3ff2051286451 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 8 Nov 2024 14:58:24 +0100 Subject: [PATCH 04/63] createForInlineCompilation cannot return null --- .../org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java index 82a29cbc84a4..e962dac031ee 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -50,8 +50,8 @@ public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { @Override public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { - // TODO: Must not return null? - return null; + var bindingsMap = inlineContext.bindingsAnalysis(); + return new Mini(bindingsMap); } private static final class Mini extends MiniIRPass { From 76af72e0f662d066914954e40a30f15ac73a1831 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 8 Nov 2024 16:17:24 +0100 Subject: [PATCH 05/63] ChainedMiniPass delegates to transformModule --- .../java/org/enso/compiler/pass/ChainedMiniPass.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java index 1fceeecbcdd2..564d47be0607 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java @@ -1,8 +1,8 @@ package org.enso.compiler.pass; -import java.util.Objects; import org.enso.compiler.core.IR; import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.Module; /** Utility class for chaining mini passes together. */ final class ChainedMiniPass extends MiniIRPass { @@ -39,6 +39,13 @@ public Expression transformExpression(Expression ir) { return sndIr; } + @Override + public Module transformModule(Module moduleIr) { + var first = firstPass.transformModule(moduleIr); + var second = secondPass.transformModule(first); + return second; + } + @Override public boolean checkPostCondition(IR ir) { return firstPass.checkPostCondition(ir) && secondPass.checkPostCondition(ir); @@ -46,6 +53,6 @@ public boolean checkPostCondition(IR ir) { @Override public String toString() { - return Objects.toString(firstPass) + ":" + Objects.toString(secondPass); + return "{" + firstPass + " + " + secondPass + "}"; } } From 207946f6c0e7fc3d0475d9193576159858787b53 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 8 Nov 2024 16:18:02 +0100 Subject: [PATCH 06/63] ImportSymbolAnalysis copies unchanged imports --- .../pass/analyse/ImportSymbolAnalysis.java | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java index e962dac031ee..4a92f4ac8f1f 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -63,21 +63,26 @@ private Mini(BindingsMap bindingsMap) { @Override public Module transformModule(Module moduleIr) { + var newImports = new ArrayList(); for (var imp : CollectionConverters.asJava(moduleIr.imports())) { - var encounteredErrors = analyseSymbolsFromImport((Import.Module) imp); - if (encounteredErrors != null) { - return moduleIr.copy( - encounteredErrors, - moduleIr.exports(), - moduleIr.bindings(), - moduleIr.isPrivate(), - moduleIr.location(), - moduleIr.passData(), - moduleIr.diagnostics(), - moduleIr.id()); + if (imp instanceof Import.Module modImp) { + var encounteredErrors = analyseSymbolsFromImport(modImp); + if (encounteredErrors != null) { + newImports.addAll(encounteredErrors); + continue; + } } + newImports.add(imp); } - return moduleIr; + return moduleIr.copy( + CollectionConverters.asScala(newImports).toList(), + moduleIr.exports(), + moduleIr.bindings(), + moduleIr.isPrivate(), + moduleIr.location(), + moduleIr.passData(), + moduleIr.diagnostics(), + moduleIr.id()); } @Override @@ -86,7 +91,7 @@ public Expression transformExpression(Expression expr) { } /** Returns list of encountered errors, or null. */ - private scala.collection.immutable.List analyseSymbolsFromImport(Import.Module imp) { + private List analyseSymbolsFromImport(Import.Module imp) { if (imp.onlyNames().isDefined()) { var resolvedImport = bindingsMap.resolvedImports().find(resImp -> resImp.importDef() == imp); @@ -99,9 +104,11 @@ private scala.collection.immutable.List analyseSymbolsFromImport(Import. importedTargets.flatMap( importedTarget -> onlyNames.filterNot(nm -> isSymbolResolved(importedTarget, nm))); if (unresolvedSymbols.nonEmpty()) { - return unresolvedSymbols.map( - unresolvedSym -> - createErrorForUnresolvedSymbol(imp, importedTargets.head(), unresolvedSym)); + scala.collection.immutable.List errs = + unresolvedSymbols.map( + unresolvedSym -> + createErrorForUnresolvedSymbol(imp, importedTargets.head(), unresolvedSym)); + return CollectionConverters.asJava(errs); } } @@ -145,7 +152,7 @@ private scala.collection.immutable.List analyseSymbolsFromImport(Import. } } if (!encounteredErrors.isEmpty()) { - return CollectionConverters.asScala(encounteredErrors).toList(); + return encounteredErrors; } } return null; From 2e4106bd2c58077d3b1fb5f805c59807e265d168 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 8 Nov 2024 16:18:11 +0100 Subject: [PATCH 07/63] Fix compilation of a test --- .../src/test/scala/org/enso/compiler/test/PassesTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala index 2ef5cf27e8e7..fbbc284a6b72 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala @@ -64,7 +64,7 @@ class PassesTest extends CompilerTest { SectionsToBinOp.INSTANCE, OperatorToFunction, LambdaShorthandToLambda, - ImportSymbolAnalysis, + ImportSymbolAnalysis.INSTANCE, AmbiguousImportsAnalysis, PrivateModuleAnalysis.INSTANCE, PrivateConstructorAnalysis.INSTANCE, From eb20aaff95170168fe5ec2fb2122fdcc14d10bb2 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 8 Nov 2024 16:34:19 +0100 Subject: [PATCH 08/63] Combining mini passes can result in null --- .../main/java/org/enso/compiler/pass/ChainedMiniPass.java | 5 ++--- .../src/main/java/org/enso/compiler/pass/MiniIRPass.java | 8 +++++++- .../main/java/org/enso/compiler/pass/MiniPassFactory.java | 3 ++- .../main/scala/org/enso/compiler/pass/PassManager.scala | 8 ++++++-- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java index 564d47be0607..f20f7c8508e7 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java @@ -15,9 +15,8 @@ private ChainedMiniPass(MiniIRPass firstPass, MiniIRPass secondPass) { } static MiniIRPass chain(MiniIRPass firstPass, MiniIRPass secondPass) { - if (firstPass == null) { - return secondPass; - } + assert firstPass != null; + assert secondPass != null; return new ChainedMiniPass(firstPass, secondPass); } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java index af921af41cca..cb68d62f3946 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java @@ -103,10 +103,16 @@ public String toString() { * Combines two mini IR passes into one that delegates to both of them. * * @param first first mini pass (can be {@code null}) - * @param second second mini pass + * @param second second mini pass (can be {@code null}) * @return a combined pass that calls both non-{@code null} of the provided passes */ public static MiniIRPass combine(MiniIRPass first, MiniIRPass second) { + if (first == null && second == null) { + return null; + } + if (first == null) { + return second; + } return ChainedMiniPass.chain(first, second); } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassFactory.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassFactory.java index 8c98d1d87bf4..29fce1af76ca 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassFactory.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassFactory.java @@ -16,7 +16,8 @@ public interface MiniPassFactory extends IRProcessingPass { * a module. * * @param moduleContext A mini pass can optionally save reference to this module context. - * @return May return {@code null} if module compilation is not supported. + * @return May return {@code null} if module compilation is not supported, or if the compilation + * for the given {@code moduleContext} should be skipped. */ MiniIRPass createForModuleCompilation(ModuleContext moduleContext); diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 5bb0732e5734..cf6796babca3 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -105,9 +105,13 @@ class PassManager( factory.createForModuleCompilation(newContext) ) val combinedPass = miniPasses.fold(null)(MiniIRPass.combine) - logger.trace(" flushing pending mini pass: {}", combinedPass) pendingMiniPasses = List() - MiniIRPass.compile(classOf[Module], in, combinedPass) + if (combinedPass != null) { + logger.trace(" flushing pending mini pass: {}", combinedPass) + MiniIRPass.compile(classOf[Module], in, combinedPass) + } else { + in + } } else { in } From 8d3c4b30597bef09b27c02819d0d3576bca79268 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Nov 2024 16:08:00 +0100 Subject: [PATCH 09/63] Add consistency validation to PassManager --- .../org/enso/compiler/pass/PassManager.scala | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index cf6796babca3..372a14b1a8e4 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -1,9 +1,11 @@ package org.enso.compiler.pass +import org.enso.common.{Asserts, CompilationStage} import org.slf4j.LoggerFactory import org.enso.compiler.context.{InlineContext, ModuleContext} import org.enso.compiler.core.ir.{Expression, Module} import org.enso.compiler.core.CompilerError +import org.enso.compiler.pass.analyse.BindingAnalysis // TODO [AA] In the future, the pass ordering should be _computed_ from the list // of available passes, rather than just verified. @@ -67,6 +69,8 @@ class PassManager( ir: Module, moduleContext: ModuleContext ): Module = { + Asserts.assertInJvm(validateConsistency(ir, moduleContext)) + passes.foldLeft(ir)((ir, group) => runPassesOnModule(ir, moduleContext, group) ) @@ -84,6 +88,8 @@ class PassManager( moduleContext: ModuleContext, passGroup: PassGroup ): Module = { + Asserts.assertInJvm(validateConsistency(ir, moduleContext)) + if (!passes.contains(passGroup)) { throw new CompilerError("Cannot run an unvalidated pass group.") } @@ -235,6 +241,34 @@ class PassManager( ix - totalLength == indexOfPassInGroup } + private def validateConsistency( + ir: Module, + moduleContext: ModuleContext + ): Boolean = { + if ( + moduleContext.module.getCompilationStage.isAtLeast( + CompilationStage.AFTER_PARSING + ) + ) { + if (!(moduleContext.module.getIr eq ir)) { + throw new AssertionError( + "Mismatch of IR between ModuleContext and IR in " + moduleContext + .getName() + ) + } + val bmFromCtx = moduleContext.bindingsAnalysis() + val bmFromMeta = ir.passData.get(BindingAnalysis) + if (bmFromCtx != null) { + Asserts.assertInJvm(bmFromMeta.isDefined) + Asserts.assertInJvm(bmFromCtx == bmFromMeta.get) + } + if (bmFromMeta.isDefined) { + Asserts.assertInJvm(bmFromCtx == bmFromMeta.get) + } + } + true + } + /** Updates the metadata in a copy of the IR when updating that metadata * requires global state. * From cab74dc2c22093864af6f077fe91b9fa6b1df187 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 15 Nov 2024 16:08:28 +0100 Subject: [PATCH 10/63] Add assert that bindingsMap != null --- .../org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java index 4a92f4ac8f1f..1ecc067fa980 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -45,6 +45,7 @@ public Seq invalidatedPasses() { @Override public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { var bindingsMap = moduleContext.bindingsAnalysis(); + assert bindingsMap != null; return new Mini(bindingsMap); } From 1935b9e4866c650ad23f0f3422193be8cad8a0ad Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 19 Nov 2024 14:26:55 +0100 Subject: [PATCH 11/63] Consistency validation has better error message. --- .../org/enso/compiler/pass/PassManager.scala | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 372a14b1a8e4..28dc1e22d08e 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -241,10 +241,25 @@ class PassManager( ix - totalLength == indexOfPassInGroup } + /** Validates consistency between the IR accessible via `moduleContext` and `ir`. + * There is no way how to enforce this consistency statically. + * Should be called only iff assertions are enabled. + * @return true if they are consistent, otherwise throws [[AssertionError]]. + */ private def validateConsistency( ir: Module, moduleContext: ModuleContext ): Boolean = { + def hex(obj: Object): String = { + if (obj != null) { + val hexStr = Integer.toHexString(System.identityHashCode(obj)) + val className = obj.getClass.getSimpleName + s"$className@${hexStr}" + } else { + "null" + } + } + if ( moduleContext.module.getCompilationStage.isAtLeast( CompilationStage.AFTER_PARSING @@ -252,18 +267,20 @@ class PassManager( ) { if (!(moduleContext.module.getIr eq ir)) { throw new AssertionError( - "Mismatch of IR between ModuleContext and IR in " + moduleContext - .getName() + "Mismatch of IR between ModuleContext and IR in module '" + moduleContext + .getName() + "'. " + + s"IR from moduleContext: ${hex(moduleContext.module.getIr)}, IR from module: ${hex(ir)}" ) } val bmFromCtx = moduleContext.bindingsAnalysis() val bmFromMeta = ir.passData.get(BindingAnalysis) - if (bmFromCtx != null) { - Asserts.assertInJvm(bmFromMeta.isDefined) - Asserts.assertInJvm(bmFromCtx == bmFromMeta.get) - } - if (bmFromMeta.isDefined) { - Asserts.assertInJvm(bmFromCtx == bmFromMeta.get) + if (bmFromMeta.isDefined || bmFromCtx != null) { + Asserts.assertInJvm( + bmFromCtx eq bmFromMeta.get, + s"BindingsMap mismatch between ModuleContext and IR in module '" + + moduleContext.getName() + "'. " + + s"BindingsMap from moduleContext: ${hex(bmFromCtx)}, BindingsMap from IR: ${hex(bmFromMeta.get)}" + ) } } true From 546f17cde22689d78255def45038d13a96d25ebe Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 19 Nov 2024 15:07:50 +0100 Subject: [PATCH 12/63] CompilerTestSetup explicitly changes IR in ModuleContext. --- .../org/enso/compiler/pass/PassManager.scala | 2 +- .../compiler/pass/PassManagerTestUtils.java | 11 +++++++++++ .../enso/compiler/test/CompilerTestSetup.scala | 17 +++++++++++++++-- .../runtime/TruffleCompilerContext.java | 4 +++- 4 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 engine/runtime-instrument-common/src/test/java/org/enso/compiler/pass/PassManagerTestUtils.java diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 28dc1e22d08e..cfd2ffde9395 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -20,7 +20,7 @@ import org.enso.compiler.pass.analyse.BindingAnalysis */ //noinspection DuplicatedCode class PassManager( - passes: List[PassGroup], + protected val passes: List[PassGroup], passConfiguration: PassConfiguration ) { private val logger = LoggerFactory.getLogger(classOf[PassManager]) diff --git a/engine/runtime-instrument-common/src/test/java/org/enso/compiler/pass/PassManagerTestUtils.java b/engine/runtime-instrument-common/src/test/java/org/enso/compiler/pass/PassManagerTestUtils.java new file mode 100644 index 000000000000..3e0e0314b2e9 --- /dev/null +++ b/engine/runtime-instrument-common/src/test/java/org/enso/compiler/pass/PassManagerTestUtils.java @@ -0,0 +1,11 @@ +package org.enso.compiler.pass; + +import scala.collection.immutable.List; + +public final class PassManagerTestUtils { + private PassManagerTestUtils() {} + + public static List getPasses(PassManager passManager) { + return passManager.passes(); + } +} diff --git a/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala b/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala index 01c42dc4683a..216260e9eca1 100644 --- a/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala +++ b/engine/runtime-instrument-common/src/test/scala/org/enso/compiler/test/CompilerTestSetup.scala @@ -8,7 +8,11 @@ import org.enso.compiler.core.ir.MetadataStorage.MetadataPair import org.enso.compiler.data.BindingsMap.ModuleReference import org.enso.compiler.data.{BindingsMap, CompilerConfig} import org.enso.compiler.pass.analyse.BindingAnalysis -import org.enso.compiler.pass.{PassConfiguration, PassManager} +import org.enso.compiler.pass.{ + PassConfiguration, + PassManager, + PassManagerTestUtils +} import org.enso.interpreter.runtime import org.enso.interpreter.runtime.ModuleTestUtils import org.enso.compiler.context.LocalScope @@ -38,7 +42,16 @@ trait CompilerTestSetup { passManager: PassManager, moduleContext: ModuleContext ): Module = { - passManager.runPassesOnModule(ir, moduleContext) + val passGroups = PassManagerTestUtils.getPasses(passManager) + val runtimeMod = runtime.Module.fromCompilerModule(moduleContext.module) + passGroups.foldLeft(ir)((curIr, group) => { + // Before a PassGroup is run on a module, we need to explicitly set the + // IR on the runtime module, as the pass manager will not do this for us. + // This is to ensure consistency between the curIr and IR stored in moduleContext + ModuleTestUtils.unsafeSetIr(runtimeMod, curIr) + val newIr = passManager.runPassesOnModule(curIr, moduleContext, group) + newIr + }) } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java index 5669012209b9..8af87d0696db 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java @@ -791,7 +791,9 @@ public BindingsMap getBindingsMap() { var meta = module.getIr().passData(); var pass = meta.get(BindingAnalysis$.MODULE$); emitIOException(); - return (BindingsMap) pass.get(); + if (pass.isDefined()) { + return (BindingsMap) pass.get(); + } } catch (IOException ex) { var logger = TruffleLogger.getLogger(LanguageInfo.ID, org.enso.interpreter.runtime.Module.class); From 289a255f4b22c3e9f568701d30045ce8eafec237 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 19 Nov 2024 18:10:53 +0100 Subject: [PATCH 13/63] runtime-integration-tests accesses PassManager field via reflection --- build.sbt | 10 +++++++ .../org/enso/compiler/test/CompilerTest.scala | 27 +++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 6bf7918a1edf..950e1bfe3a95 100644 --- a/build.sbt +++ b/build.sbt @@ -2909,6 +2909,16 @@ lazy val `runtime-integration-tests` = (`runtime` / javaModuleName).value -> Seq(javaSrcDir, testClassesDir) ) }, + Test / addOpens := { + val compilerModName = (`runtime-compiler` / javaModuleName).value + // In the tests, we access a private field of org.enso.compiler.pass.PassManager via reflection. + Map( + compilerModName + "/org.enso.compiler.pass" -> Seq( + (`runtime` / javaModuleName).value, + "ALL-UNNAMED" + ) + ) + }, // runtime-integration-tests does not have module descriptor on its own, so we have // to explicitly add some modules to the resolution. Test / addModules := Seq( diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala index b7bd14a235c0..0e51fd5fada4 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala @@ -16,7 +16,7 @@ import org.enso.compiler.core.ir.MetadataStorage.MetadataPair import org.enso.compiler.data.BindingsMap.ModuleReference import org.enso.compiler.data.{BindingsMap, CompilerConfig} import org.enso.compiler.pass.analyse.BindingAnalysis -import org.enso.compiler.pass.{PassConfiguration, PassManager} +import org.enso.compiler.pass.{PassConfiguration, PassGroup, PassManager} import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpecLike import org.enso.interpreter.runtime @@ -31,6 +31,20 @@ trait CompilerTest extends AnyWordSpecLike with Matchers with CompilerRunner trait CompilerRunner { // === IR Utilities ========================================================= + /** Utility method to access private field of [[PassManager]]. We cannot use + * a class in the same package because patching up the JPMS modules would + * be too complex. Let's just hack it via reflection here. + * @return + */ + private def getPassesViaReflection( + passManager: PassManager + ): List[PassGroup] = { + val passesField = passManager.getClass.getDeclaredField("passes") + passesField.setAccessible(true) + val passes = passesField.get(passManager) + passes.asInstanceOf[List[PassGroup]] + } + /** Provides an extension method allowing the running of a specified list of * passes on the provided IR. * @@ -48,7 +62,16 @@ trait CompilerRunner { passManager: PassManager, moduleContext: ModuleContext ): Module = { - passManager.runPassesOnModule(ir, moduleContext) + val passGroups = getPassesViaReflection(passManager) + val runtimeMod = runtime.Module.fromCompilerModule(moduleContext.module) + passGroups.foldLeft(ir)((curIr, group) => { + // Before a PassGroup is run on a module, we need to explicitly set the + // IR on the runtime module, as the pass manager will not do this for us. + // This is to ensure consistency between the curIr and IR stored in moduleContext + ModuleTestUtils.unsafeSetIr(runtimeMod, curIr) + val newIr = passManager.runPassesOnModule(curIr, moduleContext, group) + newIr + }) } } From 494975dd01ce1c10663767a8d18abf6f69604ad4 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 20 Nov 2024 14:23:33 +0100 Subject: [PATCH 14/63] Add MiniPassTraverserTest --- build.sbt | 8 +- .../enso/compiler/pass/MiniPassTraverser.java | 2 +- .../pass/test/MiniPassTraverserTest.java | 45 +++++++ .../compiler/pass/test/MockExpression.java | 114 ++++++++++++++++++ .../enso/compiler/pass/test/MockMiniPass.java | 35 ++++++ 5 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java create mode 100644 engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java create mode 100644 engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java diff --git a/build.sbt b/build.sbt index 950e1bfe3a95..7730ef436cb9 100644 --- a/build.sbt +++ b/build.sbt @@ -3202,7 +3202,8 @@ lazy val `runtime-compiler` = "org.yaml" % "snakeyaml" % snakeyamlVersion % Test, "org.jline" % "jline" % jlineVersion % Test, "com.typesafe" % "config" % typesafeConfigVersion % Test, - "org.graalvm.polyglot" % "polyglot" % graalMavenPackagesVersion % Test + "org.graalvm.polyglot" % "polyglot" % graalMavenPackagesVersion % Test, + "org.hamcrest" % "hamcrest-all" % hamcrestVersion % Test ), Compile / moduleDependencies ++= Seq( "org.slf4j" % "slf4j-api" % slf4jVersion, @@ -3241,9 +3242,14 @@ lazy val `runtime-compiler` = javaModuleName.value ), Test / patchModules := { + // Patch test-classes into the runtime module. This is standard way to deal with the + // split package problem in unit tests. For example, Maven's surefire plugin does this. val testClassDir = (Test / productDirectories).value.head + // Patching with sources is useful for compilation, patching with compiled classes for runtime. + val javaSrcDir = (Test / javaSource).value Map( javaModuleName.value -> Seq( + javaSrcDir, testClassDir ) ) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassTraverser.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassTraverser.java index 4243b0fce642..ab596881e60d 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassTraverser.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassTraverser.java @@ -71,7 +71,7 @@ static IR compileDeep(IR root, MiniIRPass miniPass) { * @param queue queue to put objects in * @param ir IR to process * @param miniPass process with this mini pass - * @return {@code true} if the has been modified with new tries to process first + * @return {@code true} if the {@code queue} has been modified with new tries to process first */ private static List enqueueSubExpressions( Collection queue, IR ir, MiniIRPass miniPass) { diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java new file mode 100644 index 000000000000..48f2963bfc01 --- /dev/null +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java @@ -0,0 +1,45 @@ +package org.enso.compiler.pass.test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import java.util.List; +import org.enso.compiler.pass.MiniIRPass; +import org.junit.Test; + +public class MiniPassTraverserTest { + @Test + public void traversesOneExpression() { + var expr = new MockExpression(false); + var miniPass = new MockMiniPass(); + MiniIRPass.compile(MockExpression.class, expr, miniPass); + assertThat("Prepare is called only for trees with depth > 1", expr.isPrepared(), is(false)); + assertThat(expr.isTransformed(), is(true)); + } + + @Test + public void traversesExpressionWithOneChild() { + var parentExpr = new MockExpression(false); + var childExpr = new MockExpression(true); + parentExpr.addChild(childExpr); + var miniPass = new MockMiniPass(); + MiniIRPass.compile(MockExpression.class, parentExpr, miniPass); + assertThat("Prepare must be called on a child expression", childExpr.isPrepared(), is(true)); + assertThat(childExpr.isTransformed(), is(true)); + assertThat(parentExpr.isTransformed(), is(true)); + } + + @Test + public void traversesExpressionWithManyChildren() { + var parentExpr = new MockExpression(false); + var children = List.of(new MockExpression(true), new MockExpression(true)); + children.forEach(parentExpr::addChild); + var miniPass = new MockMiniPass(); + MiniIRPass.compile(MockExpression.class, parentExpr, miniPass); + for (var ch : children) { + assertThat("Prepare must be called on a child expression", ch.isPrepared(), is(true)); + assertThat(ch.isTransformed(), is(true)); + } + assertThat(parentExpr.isTransformed(), is(true)); + } +} diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java new file mode 100644 index 000000000000..fde06f628f0e --- /dev/null +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java @@ -0,0 +1,114 @@ +package org.enso.compiler.pass.test; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.function.Function; +import org.enso.compiler.core.IR; +import org.enso.compiler.core.Identifier; +import org.enso.compiler.core.ir.DiagnosticStorage; +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.IdentifiedLocation; +import org.enso.compiler.core.ir.MetadataStorage; +import scala.Option; +import scala.PartialFunction; +import scala.jdk.javaapi.CollectionConverters; + +final class MockExpression implements Expression { + private boolean isTransformed; + private boolean isPrepared; + private final boolean hasParent; + private List exprChildren = new ArrayList<>(); + + MockExpression(boolean hasParent) { + this.hasParent = hasParent; + } + + boolean isTransformed() { + return isTransformed; + } + + void setTransformed(boolean transformed) { + this.isTransformed = transformed; + } + + boolean hasParent() { + return hasParent; + } + + void addChild(MockExpression child) { + exprChildren.add(child); + } + + boolean isPrepared() { + return isPrepared; + } + + void setPrepared(boolean prepared) { + this.isPrepared = prepared; + } + + @Override + public Expression transformExpressions(PartialFunction fn) { + return this; + } + + @Override + public Expression mapExpressions(Function fn) { + for (var child : exprChildren) { + fn.apply(child); + } + return this; + } + + @Override + public scala.collection.immutable.List children() { + var lst = CollectionConverters.asScala(exprChildren).toList(); + var ret = lst.map(item -> (IR) item); + return ret; + } + + @Override + public @Identifier UUID getId() { + return null; + } + + @Override + public DiagnosticStorage diagnostics() { + return null; + } + + @Override + public DiagnosticStorage getDiagnostics() { + return null; + } + + @Override + public MetadataStorage passData() { + return null; + } + + @Override + public IdentifiedLocation identifiedLocation() { + return null; + } + + @Override + public Expression setLocation(Option location) { + throw new UnsupportedOperationException("unimplemented"); + } + + @Override + public Expression duplicate( + boolean keepLocations, + boolean keepMetadata, + boolean keepDiagnostics, + boolean keepIdentifiers) { + throw new UnsupportedOperationException("unimplemented"); + } + + @Override + public String showCode(int indent) { + throw new UnsupportedOperationException("unimplemented"); + } +} diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java new file mode 100644 index 000000000000..56e4b66e25e2 --- /dev/null +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java @@ -0,0 +1,35 @@ +package org.enso.compiler.pass.test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import org.enso.compiler.core.IR; +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.pass.MiniIRPass; + +final class MockMiniPass extends MiniIRPass { + + @Override + public Expression transformExpression(Expression expr) { + if (expr instanceof MockExpression mockExpr) { + if (mockExpr.hasParent()) { + assertThat( + "Prepare must be called on an expression with a parent", + mockExpr.isPrepared(), + is(true)); + } + assertThat("Transform is called just once", mockExpr.isTransformed(), is(false)); + mockExpr.setTransformed(true); + } + return expr; + } + + @Override + public MiniIRPass prepare(IR parent, Expression child) { + if (child instanceof MockExpression mockExpr) { + assertThat("Prepare is called just once", mockExpr.isPrepared(), is(false)); + mockExpr.setPrepared(true); + } + return this; + } +} From deadad5c1456ba0b9d114d371ecc76f82cb72c0b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 20 Nov 2024 14:35:54 +0100 Subject: [PATCH 15/63] MiniPassTraverser supports end of the traversal --- .../org/enso/compiler/pass/MiniIRPass.java | 4 +++- .../enso/compiler/pass/MiniPassTraverser.java | 4 +++- .../pass/test/MiniPassTraverserTest.java | 23 ++++++++++++++++--- .../enso/compiler/pass/test/MockMiniPass.java | 15 +++++++++++- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java index cb68d62f3946..3097e2eacf35 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java @@ -57,7 +57,9 @@ public abstract class MiniIRPass { * * @param parent the the parent of the edge * @param child the child expression element to be be processed. - * @return an instance of the pass to process the child's element subtree + * @return an instance of the pass to process the child's element subtree. If null is returned, + * the subtree of the child element is not processed, including {@code child} (i.e. {@code + * child} is not processed as well). */ public MiniIRPass prepare(IR parent, Expression child) { return this; diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassTraverser.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassTraverser.java index ab596881e60d..3fde07bb8553 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassTraverser.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassTraverser.java @@ -81,7 +81,9 @@ private static List enqueueSubExpressions( (ch) -> { var preparedMiniPass = miniPass.prepare(ir, ch); childExpressions.add(ch); - queue.add(new MiniPassTraverser(preparedMiniPass, childExpressions, i[0]++)); + if (preparedMiniPass != null) { + queue.add(new MiniPassTraverser(preparedMiniPass, childExpressions, i[0]++)); + } return ch; }); return childExpressions; diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java index 48f2963bfc01..c69d57fd19fb 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java @@ -11,7 +11,7 @@ public class MiniPassTraverserTest { @Test public void traversesOneExpression() { var expr = new MockExpression(false); - var miniPass = new MockMiniPass(); + var miniPass = new MockMiniPass(null); MiniIRPass.compile(MockExpression.class, expr, miniPass); assertThat("Prepare is called only for trees with depth > 1", expr.isPrepared(), is(false)); assertThat(expr.isTransformed(), is(true)); @@ -22,7 +22,7 @@ public void traversesExpressionWithOneChild() { var parentExpr = new MockExpression(false); var childExpr = new MockExpression(true); parentExpr.addChild(childExpr); - var miniPass = new MockMiniPass(); + var miniPass = new MockMiniPass(null); MiniIRPass.compile(MockExpression.class, parentExpr, miniPass); assertThat("Prepare must be called on a child expression", childExpr.isPrepared(), is(true)); assertThat(childExpr.isTransformed(), is(true)); @@ -34,7 +34,7 @@ public void traversesExpressionWithManyChildren() { var parentExpr = new MockExpression(false); var children = List.of(new MockExpression(true), new MockExpression(true)); children.forEach(parentExpr::addChild); - var miniPass = new MockMiniPass(); + var miniPass = new MockMiniPass(null); MiniIRPass.compile(MockExpression.class, parentExpr, miniPass); for (var ch : children) { assertThat("Prepare must be called on a child expression", ch.isPrepared(), is(true)); @@ -42,4 +42,21 @@ public void traversesExpressionWithManyChildren() { } assertThat(parentExpr.isTransformed(), is(true)); } + + @Test + public void stopTraversingWhenPrepareReturnsNull() { + var e1 = new MockExpression(false); + var e2 = new MockExpression(true); + var e3 = new MockExpression(true); + e1.addChild(e2); + e2.addChild(e3); + // Should stop traversing when e3 is encountered. + // Should only process e1 and e2, not e3 + var miniPass = new MockMiniPass(e3); + MiniIRPass.compile(MockExpression.class, e1, miniPass); + assertThat("e3 should not be processed", e3.isPrepared(), is(false)); + assertThat("e3 should not be processed", e3.isTransformed(), is(false)); + assertThat("e2 should still be processed", e2.isPrepared(), is(true)); + assertThat("e2 should still be processed", e2.isTransformed(), is(true)); + } } diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java index 56e4b66e25e2..12079a6a366f 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java @@ -8,6 +8,15 @@ import org.enso.compiler.pass.MiniIRPass; final class MockMiniPass extends MiniIRPass { + private final MockExpression stopExpr; + + /** + * @param stopExpr When encountered this expression, {@code prepare} method will return null to + * signal that the traversal should stop. Can be null. + */ + MockMiniPass(MockExpression stopExpr) { + this.stopExpr = stopExpr; + } @Override public Expression transformExpression(Expression expr) { @@ -30,6 +39,10 @@ public MiniIRPass prepare(IR parent, Expression child) { assertThat("Prepare is called just once", mockExpr.isPrepared(), is(false)); mockExpr.setPrepared(true); } - return this; + if (stopExpr == child) { + return null; + } else { + return this; + } } } From abc32ad8c4f964865639fdf346030088281017b9 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 20 Nov 2024 14:37:47 +0100 Subject: [PATCH 16/63] Fix MockMiniPass - stopExpression is not even prepared --- .../java/org/enso/compiler/pass/test/MockMiniPass.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java index 12079a6a366f..03399126e53c 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java @@ -35,14 +35,13 @@ public Expression transformExpression(Expression expr) { @Override public MiniIRPass prepare(IR parent, Expression child) { + if (stopExpr == child) { + return null; + } if (child instanceof MockExpression mockExpr) { assertThat("Prepare is called just once", mockExpr.isPrepared(), is(false)); mockExpr.setPrepared(true); } - if (stopExpr == child) { - return null; - } else { - return this; - } + return this; } } From 806406cbd567047ef3db73a84c7c413daf7de9cc Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 20 Nov 2024 17:48:47 +0100 Subject: [PATCH 17/63] ImportSymbolAnalysis skips processing --- .../compiler/pass/analyse/ImportSymbolAnalysis.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java index 1ecc067fa980..5b8dbd64f41c 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -5,6 +5,7 @@ import org.enso.common.ScalaConversions; import org.enso.compiler.context.InlineContext; import org.enso.compiler.context.ModuleContext; +import org.enso.compiler.core.IR; import org.enso.compiler.core.ir.Expression; import org.enso.compiler.core.ir.MetadataStorage; import org.enso.compiler.core.ir.Module; @@ -86,9 +87,16 @@ public Module transformModule(Module moduleIr) { moduleIr.id()); } + @Override + public MiniIRPass prepare(IR parent, Expression child) { + // return null - do not traverse any children of the root - we just + // need to transform the module IR. + return null; + } + @Override public Expression transformExpression(Expression expr) { - return expr; + throw new IllegalStateException("Should not be called - prepare returns null"); } /** Returns list of encountered errors, or null. */ From edcd10ba30e26e61f6cd29ff91051e5ee5a5baf6 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 20 Nov 2024 19:07:22 +0100 Subject: [PATCH 18/63] Migrate AmbiguousImportsAnalysis to mini pass and to Java --- .../analyse/AmbiguousImportsAnalysis.java | 355 ++++++++++++++ .../main/scala/org/enso/compiler/Passes.scala | 2 +- .../analyse/AmbiguousImportsAnalysis.scala | 445 ------------------ 3 files changed, 356 insertions(+), 446 deletions(-) create mode 100644 engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java delete mode 100644 engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.scala diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java new file mode 100644 index 000000000000..d2078ec0c616 --- /dev/null +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java @@ -0,0 +1,355 @@ +package org.enso.compiler.pass.analyse; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.enso.common.ScalaConversions; +import org.enso.compiler.context.InlineContext; +import org.enso.compiler.context.ModuleContext; +import org.enso.compiler.core.CompilerError; +import org.enso.compiler.core.IR; +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.MetadataStorage; +import org.enso.compiler.core.ir.Module; +import org.enso.compiler.core.ir.Name.Literal; +import org.enso.compiler.core.ir.Warning; +import org.enso.compiler.core.ir.expression.errors.ImportExport; +import org.enso.compiler.core.ir.module.scope.Import; +import org.enso.compiler.core.ir.module.scope.imports.Polyglot; +import org.enso.compiler.data.BindingsMap; +import org.enso.compiler.data.BindingsMap.ImportTarget; +import org.enso.compiler.data.BindingsMap.ResolvedName; +import org.enso.compiler.pass.IRProcessingPass; +import org.enso.compiler.pass.MiniIRPass; +import org.enso.compiler.pass.MiniPassFactory; +import scala.collection.immutable.Seq; +import scala.jdk.javaapi.CollectionConverters; + +/** + * A pass that checks for ambiguous and duplicated symbols from imports. A duplicated import is an + * import of a symbol that has already been imported and refers to the same object (entity). On the + * other hand, an ambiguous import is an import of a symbol that has already been imported but + * refers to a different object. For every duplicated import, a warning is attached to the IR, and + * for every ambiguous import, the IR is replaced with an error. To identify an object, this pass + * uses physical path of the object instead of the object itself. + * + *

One import IR can be replaced with multiple error IRs. This is the case for {@code from ... + * import ...} import statements. + * + *

The original import is saved in the error and warning so that the user can see from which + * location the symbol was originally imported. + * + *

Also iterates polyglot imports. + * + *

All synthetic imports and exports, as well as synthetic modules are ignored by this pass. + * + *

This pass does not alter any metadata. + */ +public final class AmbiguousImportsAnalysis implements MiniPassFactory { + public static final AmbiguousImportsAnalysis INSTANCE = new AmbiguousImportsAnalysis(); + + private AmbiguousImportsAnalysis() {} + + @Override + public Seq precursorPasses() { + List passes = + List.of(BindingAnalysis$.MODULE$, ImportSymbolAnalysis.INSTANCE); + return ScalaConversions.seq(passes); + } + + @Override + public Seq invalidatedPasses() { + return ScalaConversions.seq(List.of()); + } + + @Override + public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { + if (moduleContext.isSynthetic()) { + return null; + } + return new Mini(moduleContext.bindingsAnalysis()); + } + + @Override + public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { + throw new UnsupportedOperationException("unimplemented"); + } + + + private static final class Mini extends MiniIRPass { + private final EncounteredSymbols encounteredSymbols = new EncounteredSymbols(); + private final BindingsMap bindingsMap; + + private Mini(BindingsMap bindingsMap) { + assert bindingsMap != null; + this.bindingsMap = bindingsMap; + } + + @Override + public MiniIRPass prepare(IR parent, Expression child) { + // return null - do not traverse any children of the root - we just + // need to transform the module IR. + return null; + } + + @Override + public Expression transformExpression(Expression expr) { + throw new IllegalStateException("Should not be called - prepare returns null"); + } + + @Override + public Module transformModule(Module moduleIr) { + var newImports = new ArrayList(); + moduleIr.imports().foreach(imp -> { + if (imp instanceof Import.Module impMod) { + var errs = analyseAmbiguousSymbols(impMod); + newImports.addAll(errs); + } else { + newImports.add(imp); + } + return null; + }); + return moduleIr.copy( + CollectionConverters.asScala(newImports).toList(), + moduleIr.exports(), + moduleIr.bindings(), + moduleIr.isPrivate(), + moduleIr.location(), + moduleIr.passData(), + moduleIr.diagnostics(), + moduleIr.id()); + } + + /** + * @param imp Current import to analyse. May attach warnings. + * @return List of collected errors. Potentially empty. Not null + */ + private List analyseAmbiguousSymbols(Import imp) { + var errorsForImport = new ArrayList(); + + switch (imp) { + // Import multiple symbols + case Import.Module impMod when impMod.onlyNames().isDefined() && !impMod.isSynthetic() -> { + for (var symbol : CollectionConverters.asJava(impMod.onlyNames().get())) { + var symbolName = symbol.name(); + for (var importTarget : getImportTargets(impMod)) { + var resolution = importTarget.resolveExportedSymbol(symbolName); + if (resolution.isLeft()) { + throw new CompilerError( + "Unreachable: (should have been resolved in previous passes): " + resolution); + } + var resolvedNames = resolution.toOption().get(); + for (var resolvedName : CollectionConverters.asJava(resolvedNames)) { + var symbolPath = resolvedName.qualifiedName().toString(); + tryAddEncounteredSymbol(impMod, symbolName, symbolPath, resolvedName, errorsForImport); + } + } + } + } + + // Import all symbols + case Import.Module impMod when impMod.isAll() && !impMod.isSynthetic() -> { + var importTargets = getImportTargets(impMod); + // Names of the symbols that are exported by a module or a type referred to via importTarget + var exportedSymbolNames = importTargets.stream() + .flatMap(target -> { + var expSymbols = target.exportedSymbols().keySet().toList(); + return CollectionConverters.asJava(expSymbols).stream(); + }); + List symbolsToIterate; + if (impMod.hiddenNames().isDefined()) { + var hiddenNames = impMod.hiddenNames().get().map(Literal::name); + symbolsToIterate = exportedSymbolNames + .filter(exportedSymName -> !hiddenNames.contains(exportedSymName)) + .toList(); + } else { + symbolsToIterate = exportedSymbolNames.toList(); + } + for (var symbolName : symbolsToIterate) { + for (var importTarget : importTargets) { + var resolution = importTarget.resolveExportedSymbol(symbolName); + if (resolution.isLeft()) { + throw new CompilerError( + "Unreachable: (should have been resolved in previous passes): " + resolution); + } + var resolvedNames = resolution.toOption().get(); + if (resolvedNames.size() > 1) { + // If the symbolName is resolved to multiple objects, we ignore it. + continue; + } + var resolvedName = resolvedNames.head(); + var symbolPath = resolvedName.qualifiedName().toString(); + tryAddEncounteredSymbol(impMod, symbolName, symbolPath, resolvedName, errorsForImport); + } + } + } + + // Import a renamed symbol + case Import.Module impMod when impMod.rename().isDefined() -> { + var symbolPath = impMod.name().name(); + var symbolName = impMod.rename().get().name(); + tryAddEncounteredSymbol(impMod, symbolName, symbolPath, null, errorsForImport); + } + + // Import one symbol + case Import.Module impMod when !impMod.isSynthetic() -> { + var symbolPath = impMod.name().name(); + var symbolName = impMod.name().parts().last().name(); + tryAddEncounteredSymbol(impMod, symbolName, symbolPath, null, errorsForImport); + } + + case Polyglot polyglotImp -> { + var symbolName = polyglotImp.rename().getOrElse(() -> polyglotImp.entity().getVisibleName()); + String symbolPath; + if (polyglotImp.entity() instanceof Polyglot.Java javaEntity) { + symbolPath = javaEntity.packageName() + "." + javaEntity.className(); + } else { + throw new IllegalStateException("Unsupported polyglot entity: " + polyglotImp.entity()); + } + tryAddEncounteredSymbol(polyglotImp, symbolName, symbolPath, null, errorsForImport); + return List.of(); + } + + default -> {} + } + return errorsForImport; + } + + /** + * Tries to add the encountered symbol to the encountered symbols map. If it is already contained + * in the map, checks whether the underlying entity path is the same as the original entity path. + * Based on that, either attaches a warning for a duplicated import, or returns an {@link ImportExport}. + * + * @param currentImport Currently iterated import + * @param symbolName Name of the symbol that is about to be processed + * @param symbolPath physical path of the symbol that is about to be processed + * @param errors Into this list, a potential error is appended. + */ + private void tryAddEncounteredSymbol( + Import currentImport, + String symbolName, + String symbolPath, + ResolvedName resolvedName, + List errors) { + if (!encounteredSymbols.containsSymbol(symbolName)) { + encounteredSymbols.addSymbol(currentImport, symbolName, symbolPath, resolvedName); + } else { + var encounteredFullName = encounteredSymbols.getPathForSymbol(symbolName); + var originalImport = encounteredSymbols.getOriginalImportForSymbol(symbolName); + if (symbolPath.equals(encounteredFullName)) { + // symbolName is already imported with the same symbolPath --> attach warning. + var warn = createWarningForDuplicatedImport(originalImport, currentImport, symbolName); + currentImport.diagnostics().add(warn); + } else { + // There is an encountered symbol with different physical path than symbolPath. + var resolution = encounteredSymbols.getResolvedNameForSymbol(symbolName); + if (resolution instanceof BindingsMap.ResolvedMethod resMethod && + resMethod.methodName().equals(symbolName)) { + // This is a valid ambiguous case - in previously encountered import, the symbol was resolved + // to either an extension, static, or conversion method. + return; + } else { + var error = createErrorForAmbiguousImport( + originalImport, + encounteredFullName, + currentImport, + symbolName, + symbolPath); + errors.add(error); + } + } + } + } + + private static Warning createWarningForDuplicatedImport( + Import originalImport, + Import duplicatingImport, + String duplicatedSymbol + ) { + return new Warning.DuplicatedImport( + duplicatingImport.identifiedLocation(), + originalImport, + duplicatedSymbol + ); + } + + private ImportExport createErrorForAmbiguousImport( + Import originalImport, + String originalSymbolPath, + Import duplicatingImport, + String ambiguousSymbol, + String ambiguousSymbolPath) { + return ImportExport.apply( + duplicatingImport, + new ImportExport.AmbiguousImport( + originalImport, + originalSymbolPath, + ambiguousSymbol, + ambiguousSymbolPath + ), + MetadataStorage.EMPTY + ); + } + + private List getImportTargets(Import imp) { + var found = bindingsMap.resolvedImports().find(resImp -> resImp.importDef() == imp); + if (found.isDefined()) { + return CollectionConverters.asJava(found.get().targets()); + } else { + return List.of(); + } + } + } + + /** @param symbolPath Fully qualified name of the symbol, i.e., its physical path. + * @param resolvedName The optional resolved name of the symbol. + * @param originalImport The import IR from which the symbol was originally imported. + * i.e. the first encountered import IR that imports the symbol. + */ + private record SymbolTarget( + String symbolPath, + ResolvedName resolvedName, + Import originalImport + ) { + private SymbolTarget { + assert symbolPath != null; + assert originalImport != null; + } + } + + + /** For every encountered symbol name, we keep track of the original import from which it was imported, + * along with the entity path. The entity path is vital to decide whether an imported symbol is duplicated + * or ambiguous. + * Note that there are some exceptions that are allowed to be ambiguous, like extension methods. + */ + private static final class EncounteredSymbols { + private final Map symbols = new HashMap<>(); + + boolean containsSymbol(String symbolName) { + return symbols.containsKey(symbolName); + } + + void addSymbol(Import imp, String symbol, String symbolPath, ResolvedName resolvedName) { + symbols.put(symbol, new SymbolTarget(symbolPath, resolvedName, imp)); + } + + String getPathForSymbol(String symbol) { + return symbols.get(symbol).symbolPath; + } + + ResolvedName getResolvedNameForSymbol(String symbol) { + return symbols.get(symbol).resolvedName; + } + + Import getOriginalImportForSymbol(String symbol) { + var val = symbols.get(symbol); + if (val != null) { + return val.originalImport; + } else { + return null; + } + } + } +} diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala index 47b2b1e51e56..13b99b7e480a 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala @@ -45,7 +45,7 @@ class Passes(config: CompilerConfig) { OperatorToFunction, LambdaShorthandToLambda, ImportSymbolAnalysis.INSTANCE, - AmbiguousImportsAnalysis + AmbiguousImportsAnalysis.INSTANCE ) ++ (if (config.privateCheckEnabled) { List( PrivateModuleAnalysis.INSTANCE, diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.scala deleted file mode 100644 index 1c5cd1730596..000000000000 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.scala +++ /dev/null @@ -1,445 +0,0 @@ -package org.enso.compiler.pass.analyse - -import org.enso.compiler.context.{InlineContext, ModuleContext} -import org.enso.compiler.core.Implicits.{AsDiagnostics, AsMetadata} -import org.enso.compiler.core.ir.{Expression, Module, Warning} -import org.enso.compiler.core.ir.expression.errors -import org.enso.compiler.core.ir.module.scope.Import -import org.enso.compiler.core.ir.module.scope.imports -import org.enso.compiler.data.BindingsMap -import org.enso.compiler.core.CompilerError -import org.enso.compiler.data.BindingsMap.ResolvedName -import org.enso.compiler.pass.{IRPass, IRProcessingPass} - -import scala.collection.mutable -import scala.collection.mutable.ListBuffer - -/** A pass that checks for ambiguous and duplicated symbols from imports. - * A duplicated import is an import of a symbol that has already been imported and refers to the - * same object (entity). On the other hand, an ambiguous import is an import of a symbol that has already - * been imported but refers to a different object. For every duplicated import, a warning is attached - * to the IR, and for every ambiguous import, the IR is replaced with an error. - * To identify an object, this pass uses physical path of the object instead of the object itself. - * - * One import IR can be replaced with multiple error IRs. This is the case for `from ... import ...` - * import statements. - * - * The original import is saved in the error and warning so that the user can see from which location - * the symbol was originally imported. - * - * Also iterates polyglot imports. - * - * All synthetic imports and exports, as well as synthetic modules are ignored by this pass. - * - * This pass does not alter any metadata. - */ -case object AmbiguousImportsAnalysis extends IRPass { - - override type Metadata = IRPass.Metadata.Empty - - override type Config = IRPass.Configuration.Default - - override lazy val precursorPasses: Seq[IRProcessingPass] = - Seq(BindingAnalysis, ImportSymbolAnalysis.INSTANCE) - - override lazy val invalidatedPasses: Seq[IRPass] = - Seq() - - /** @inheritdoc - */ - override def runExpression( - ir: Expression, - inlineContext: InlineContext - ): Expression = ir - - /** @inheritdoc - */ - override def runModule( - ir: Module, - moduleContext: ModuleContext - ): Module = { - if (moduleContext.isSynthetic()) { - ir - } else { - val bindingMap = ir.unsafeGetMetadata( - BindingAnalysis, - "BindingMap should already be present" - ) - val encounteredSymbols = new EncounteredSymbols() - ir.copy( - imports = ir.imports.flatMap(imp => { - analyseAmbiguousSymbols( - imp, - bindingMap, - encounteredSymbols - ).fold(identity, imp => List(imp)) - }) - ) - } - } - - /** Analyses ambiguous symbols in the given import. - * @param imp current import - * @param bindingMap binding map of the current module - * @param encounteredSymbols already encountered symbols - * @return A list of errors, if any encountered, or an import IR, potentially with some - * warnings attached. - */ - private def analyseAmbiguousSymbols( - imp: Import, - bindingMap: BindingsMap, - encounteredSymbols: EncounteredSymbols - ): Either[List[errors.ImportExport], Import] = { - imp match { - // Import multiple symbols - case moduleImport @ Import.Module( - _, - _, - _, - Some(onlyNames), - _, - _, - false, - _ - ) => - getImportTargets(moduleImport, bindingMap) match { - case Some(importTargets) => - val encounteredErrors: ListBuffer[errors.ImportExport] = - ListBuffer() - onlyNames.foreach { symbol => - val symbolName = symbol.name - importTargets.foreach { importTarget => - importTarget.resolveExportedSymbol(symbolName) match { - case Right(resolvedNames) => - resolvedNames.foreach { resolvedName => - val symbolPath = resolvedName.qualifiedName.toString - tryAddEncounteredSymbol( - encounteredSymbols, - moduleImport, - symbolName, - symbolPath, - Some(resolvedName) - ) match { - case Left(error) => - encounteredErrors += error - case Right(_) => () - } - } - case Left(resolutionError) => - throw new CompilerError( - s"Unreachable: (should have been resolved in previous passes) $resolutionError" - ) - } - } - } - if (encounteredErrors.nonEmpty) { - Left(encounteredErrors.toList) - } else { - Right(moduleImport) - } - - case None => - Right(moduleImport) - } - - // Import all symbols - case moduleImport @ Import.Module( - _, - _, - true, - _, - hiddenNames, - _, - false, - _ - ) => - getImportTargets(moduleImport, bindingMap) match { - case Some(importTargets) => - // Names of the symbols that are exported by a module or a type referred to via importTarget - val exportedSymbolNames: List[String] = - importTargets.flatMap(_.exportedSymbols.keySet.toList) - val symbolsToIterate = hiddenNames match { - case None => exportedSymbolNames - case Some(hiddenNamesLiterals) => - val hiddenNames = hiddenNamesLiterals.map(_.name) - exportedSymbolNames.filterNot(hiddenNames.contains) - } - val encounteredErrors: ListBuffer[errors.ImportExport] = - ListBuffer() - symbolsToIterate.foreach { symbolName => - importTargets.foreach { importTarget => - importTarget.resolveExportedSymbol(symbolName) match { - case Left(resolutionError) => - throw new CompilerError( - s"Unreachable: (should have been resolved in previous passes) $resolutionError" - ) - case Right(List(resolvedName)) => - tryAddEncounteredSymbol( - encounteredSymbols, - moduleImport, - symbolName, - resolvedName.qualifiedName.toString, - Some(resolvedName) - ) match { - case Left(error) => - encounteredErrors += error - case Right(_) => () - } - // If the symbolName is resolved to multiple objects, we ignore it. - case Right(_) => () - } - } - } - if (encounteredErrors.nonEmpty) { - Left(encounteredErrors.toList) - } else { - Right(moduleImport) - } - - case None => - Right(moduleImport) - } - - // Import a renamed symbol - case moduleImport @ Import.Module( - importPath, - Some(rename), - _, - _, - _, - _, - false, - _ - ) => - val symbolPath = importPath.name - tryAddEncounteredSymbol( - encounteredSymbols, - moduleImport, - rename.name, - symbolPath, - None - ) match { - case Left(error) => Left(List(error)) - case Right(imp) => Right(imp) - } - - // Import one symbol - case moduleImport @ Import.Module( - importPath, - _, - _, - _, - _, - _, - false, - _ - ) => - tryAddEncounteredSymbol( - encounteredSymbols, - moduleImport, - importPath.parts.last.name, - importPath.name, - None - ) match { - case Left(err) => Left(List(err)) - case Right(imp) => Right(imp) - } - - // Polyglot import - case polyImport @ imports.Polyglot(entity, rename, _, _) => - val symbolName = rename.getOrElse(entity.getVisibleName) - val symbolPath = entity match { - case imports.Polyglot.Java(packageName, className) => - packageName + "." + className - } - tryAddEncounteredSymbol( - encounteredSymbols, - polyImport, - symbolName, - symbolPath, - None - ) match { - case Left(err) => Left(List(err)) - case Right(imp) => Right(imp) - } - - case _ => Right(imp) - } - } - - private def getImportTargets( - imp: Import, - bindingMap: BindingsMap - ): Option[List[BindingsMap.ImportTarget]] = { - bindingMap.resolvedImports.find(_.importDef == imp) match { - case Some(resolvedImport) => - Some(resolvedImport.targets) - case None => - None - } - } - - /** Tries to add the encountered symbol to the encountered symbols map. If it is already contained - * in the map, checks whether the underlying entity path is the same as the original entity path. - * Based on that, either attaches a warning for a duplicated import, or returns an [[errors.ImportExport]]. - * - * @param encounteredSymbols Encountered symbols in the current module - * @param currentImport Currently iterated import - * @param symbolName Name of the symbol that is about to be processed - * @param symbolPath physical path of the symbol that is about to be processed - * @return - */ - private def tryAddEncounteredSymbol( - encounteredSymbols: EncounteredSymbols, - currentImport: Import, - symbolName: String, - symbolPath: String, - resolvedName: Option[ResolvedName] - ): Either[errors.ImportExport, Import] = { - if (encounteredSymbols.containsSymbol(symbolName)) { - val encounteredFullName = - encounteredSymbols.getPathForSymbol(symbolName) - val originalImport = - encounteredSymbols.getOriginalImportForSymbol(symbolName).get - if (symbolPath == encounteredFullName) { - val warn = - createWarningForDuplicatedImport( - originalImport, - currentImport, - symbolName - ) - Right(currentImport.addDiagnostic(warn)) - } else { - // The symbol was encountered before and the physical path is different. - val ambiguousImpErr = createErrorForAmbiguousImport( - originalImport, - encounteredFullName, - currentImport, - symbolName, - symbolPath - ) - encounteredSymbols.getResolvedNameForSymbol(symbolName) match { - case Some(resolvedMethod: BindingsMap.ResolvedMethod) - if resolvedMethod.methodName == symbolName => - // This is a valid ambiguous case - in previously encountered import, the symbol was resolved - // to either an extension, static, or conversion method. - Right(currentImport) - case _ => - Left(ambiguousImpErr) - } - } - } else { - encounteredSymbols.addSymbol( - currentImport, - symbolName, - symbolPath, - resolvedName - ) - Right(currentImport) - } - } - - private def createErrorForAmbiguousImport( - originalImport: Import, - originalSymbolPath: String, - duplicatingImport: Import, - ambiguousSymbol: String, - ambiguousSymbolPath: String - ): errors.ImportExport = { - errors.ImportExport( - duplicatingImport, - errors.ImportExport.AmbiguousImport( - originalImport, - originalSymbolPath, - ambiguousSymbol, - ambiguousSymbolPath - ) - ) - } - - private def createWarningForDuplicatedImport( - originalImport: Import, - duplicatingImport: Import, - duplicatedSymbol: String - ): Warning = { - Warning.DuplicatedImport( - duplicatingImport.identifiedLocation(), - originalImport, - duplicatedSymbol - ) - } - - /** @param symbolPath Fully qualified name of the symbol, i.e., its physical path. - * @param resolvedName The optinal resolved name of the symbol. - * @param originalImport The import IR from which the symbol was originally imported. - * i.e. the first encountered import IR that imports the symbol. - */ - private case class SymbolTarget( - symbolPath: String, - resolvedName: Option[ResolvedName], - originalImport: Import - ) - - /** For every encountered symbol name, we keep track of the original import from which it was imported, - * along with the entity path. The entity path is vital to decide whether an imported symbol is duplicated - * or ambiguous. - * Note that there are some exceptions that are allowed to be ambiguous, like extension methods. - */ - private class EncounteredSymbols( - private val encounteredSymbols: mutable.Map[ - String, - SymbolTarget - ] = mutable.HashMap.empty - ) { - - def containsSymbol(symbolName: String): Boolean = { - encounteredSymbols.contains(symbolName) - } - - /** @param imp Import where the symbol is located - */ - def addSymbol( - imp: Import, - symbol: String, - symbolPath: String, - resolvedName: Option[ResolvedName] - ): Unit = { - encounteredSymbols.put( - symbol, - SymbolTarget(symbolPath, resolvedName, imp) - ) - } - - /** Returns the entity path for the symbol. - */ - def getPathForSymbol( - symbol: String - ): String = { - encounteredSymbols.get(symbol) match { - case Some(symbolTarget) => - symbolTarget.symbolPath - case None => - throw new IllegalStateException("unreachable") - } - } - - def getResolvedNameForSymbol( - symbol: String - ): Option[ResolvedName] = { - encounteredSymbols.get(symbol) match { - case Some(symbolTarget) => - symbolTarget.resolvedName - case None => - throw new IllegalStateException("unreachable") - } - } - - /** Returns the original import from which the symbol was imported, if any. - */ - def getOriginalImportForSymbol( - symbol: String - ): Option[Import] = { - encounteredSymbols.get(symbol).map(_.originalImport) - } - } -} From 549358614d1bb7b1a9e6b6022aa8a5b0d7faefcd Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 20 Nov 2024 19:38:12 +0100 Subject: [PATCH 19/63] Add some tests for chained passes --- .../pass/test/MiniPassTraverserTest.java | 70 +++++++++++++++---- .../compiler/pass/test/MockExpression.java | 34 +++++---- .../enso/compiler/pass/test/MockMiniPass.java | 11 +-- 3 files changed, 86 insertions(+), 29 deletions(-) diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java index c69d57fd19fb..f88cf7810b6d 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java @@ -13,8 +13,9 @@ public void traversesOneExpression() { var expr = new MockExpression(false); var miniPass = new MockMiniPass(null); MiniIRPass.compile(MockExpression.class, expr, miniPass); - assertThat("Prepare is called only for trees with depth > 1", expr.isPrepared(), is(false)); - assertThat(expr.isTransformed(), is(true)); + assertThat( + "Prepare is called only for trees with depth > 1", expr.isPreparedByAny(), is(false)); + assertThat(expr.isTransformedByAny(), is(true)); } @Test @@ -24,9 +25,10 @@ public void traversesExpressionWithOneChild() { parentExpr.addChild(childExpr); var miniPass = new MockMiniPass(null); MiniIRPass.compile(MockExpression.class, parentExpr, miniPass); - assertThat("Prepare must be called on a child expression", childExpr.isPrepared(), is(true)); - assertThat(childExpr.isTransformed(), is(true)); - assertThat(parentExpr.isTransformed(), is(true)); + assertThat( + "Prepare must be called on a child expression", childExpr.isPreparedByAny(), is(true)); + assertThat(childExpr.isTransformedByAny(), is(true)); + assertThat(parentExpr.isTransformedByAny(), is(true)); } @Test @@ -37,10 +39,10 @@ public void traversesExpressionWithManyChildren() { var miniPass = new MockMiniPass(null); MiniIRPass.compile(MockExpression.class, parentExpr, miniPass); for (var ch : children) { - assertThat("Prepare must be called on a child expression", ch.isPrepared(), is(true)); - assertThat(ch.isTransformed(), is(true)); + assertThat("Prepare must be called on a child expression", ch.isPreparedByAny(), is(true)); + assertThat(ch.isTransformedByAny(), is(true)); } - assertThat(parentExpr.isTransformed(), is(true)); + assertThat(parentExpr.isTransformedByAny(), is(true)); } @Test @@ -54,9 +56,53 @@ public void stopTraversingWhenPrepareReturnsNull() { // Should only process e1 and e2, not e3 var miniPass = new MockMiniPass(e3); MiniIRPass.compile(MockExpression.class, e1, miniPass); - assertThat("e3 should not be processed", e3.isPrepared(), is(false)); - assertThat("e3 should not be processed", e3.isTransformed(), is(false)); - assertThat("e2 should still be processed", e2.isPrepared(), is(true)); - assertThat("e2 should still be processed", e2.isTransformed(), is(true)); + assertThat("e3 should not be processed", e3.isPreparedByAny(), is(false)); + assertThat("e3 should not be processed", e3.isTransformedByAny(), is(false)); + assertThat("e2 should still be processed", e2.isPreparedByAny(), is(true)); + assertThat("e2 should still be processed", e2.isTransformedByAny(), is(true)); + } + + @Test + public void chainedMiniPass_TraversesSingleExpression() { + var parentExpr = new MockExpression(false); + var childExpr = new MockExpression(true); + parentExpr.addChild(childExpr); + var miniPass1 = new MockMiniPass(null); + var miniPass2 = new MockMiniPass(null); + var chainedPass = MiniIRPass.combine(miniPass1, miniPass2); + MiniIRPass.compile(MockExpression.class, parentExpr, chainedPass); + assertThat( + "Child expression is transformed by both passes", + childExpr.isTransformedBy(miniPass1), + is(true)); + assertThat( + "Child expression is transformed by both passes", + childExpr.isTransformedBy(miniPass2), + is(true)); + assertThat( + "Child expression is prepared by both passes", childExpr.isPreparedBy(miniPass1), is(true)); + assertThat( + "Child expression is prepared by both passes", childExpr.isPreparedBy(miniPass2), is(true)); + } + + @Test + public void chainedMiniPass_StopsTraversingWhenPrepareReturnsNull() { + var e1 = new MockExpression(false); + var e2 = new MockExpression(true); + var e3 = new MockExpression(true); + e1.addChild(e2); + e2.addChild(e3); + // miniPass1 stops traversing on e2. + var miniPass1 = new MockMiniPass(e3); + // miniPass2 traverses everything. + var miniPass2 = new MockMiniPass(null); + var chainedPass = MiniIRPass.combine(miniPass1, miniPass2); + MiniIRPass.compile(MockExpression.class, e1, chainedPass); + assertThat("e3 should be prepared only by miniPass2", e3.isPreparedBy(miniPass2), is(true)); + assertThat( + "e3 should be transformed only by miniPass2", e3.isTransformedBy(miniPass2), is(true)); + assertThat("e3 must not be transformed by miniPass1", e3.isTransformedBy(miniPass1), is(false)); + assertThat( + "e2 should still be transformed by miniPass1", e2.isTransformedBy(miniPass1), is(true)); } } diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java index fde06f628f0e..e80661f00a72 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java @@ -1,7 +1,9 @@ package org.enso.compiler.pass.test; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.UUID; import java.util.function.Function; import org.enso.compiler.core.IR; @@ -15,8 +17,8 @@ import scala.jdk.javaapi.CollectionConverters; final class MockExpression implements Expression { - private boolean isTransformed; - private boolean isPrepared; + private final Set transformedBy = new HashSet<>(); + private final Set preparedBy = new HashSet<>(); private final boolean hasParent; private List exprChildren = new ArrayList<>(); @@ -24,12 +26,24 @@ final class MockExpression implements Expression { this.hasParent = hasParent; } - boolean isTransformed() { - return isTransformed; + boolean isTransformedBy(MockMiniPass pass) { + return transformedBy.contains(pass); } - void setTransformed(boolean transformed) { - this.isTransformed = transformed; + boolean isTransformedByAny() { + return !transformedBy.isEmpty(); + } + + boolean isPreparedBy(MockMiniPass pass) { + return preparedBy.contains(pass); + } + + boolean isPreparedByAny() { + return !preparedBy.isEmpty(); + } + + void setTransformedByPass(MockMiniPass pass) { + transformedBy.add(pass); } boolean hasParent() { @@ -40,12 +54,8 @@ void addChild(MockExpression child) { exprChildren.add(child); } - boolean isPrepared() { - return isPrepared; - } - - void setPrepared(boolean prepared) { - this.isPrepared = prepared; + void setPreparedBy(MockMiniPass pass) { + preparedBy.add(pass); } @Override diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java index 03399126e53c..801d47412e31 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java @@ -24,11 +24,12 @@ public Expression transformExpression(Expression expr) { if (mockExpr.hasParent()) { assertThat( "Prepare must be called on an expression with a parent", - mockExpr.isPrepared(), + mockExpr.isPreparedBy(this), is(true)); } - assertThat("Transform is called just once", mockExpr.isTransformed(), is(false)); - mockExpr.setTransformed(true); + assertThat( + "Transform is called just once by one pass", mockExpr.isTransformedBy(this), is(false)); + mockExpr.setTransformedByPass(this); } return expr; } @@ -39,8 +40,8 @@ public MiniIRPass prepare(IR parent, Expression child) { return null; } if (child instanceof MockExpression mockExpr) { - assertThat("Prepare is called just once", mockExpr.isPrepared(), is(false)); - mockExpr.setPrepared(true); + assertThat("Prepare is called just once by one pass", mockExpr.isPreparedBy(this), is(false)); + mockExpr.setPreparedBy(this); } return this; } From 84a02d9bcb3d4e54a574c869ac039efabd9a3b42 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 11:32:42 +0100 Subject: [PATCH 20/63] ChainedMiniPass handles null passes --- .../enso/compiler/pass/ChainedMiniPass.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java index f20f7c8508e7..78d7cb215023 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java @@ -15,39 +15,39 @@ private ChainedMiniPass(MiniIRPass firstPass, MiniIRPass secondPass) { } static MiniIRPass chain(MiniIRPass firstPass, MiniIRPass secondPass) { - assert firstPass != null; - assert secondPass != null; return new ChainedMiniPass(firstPass, secondPass); } @Override public MiniIRPass prepare(IR parent, Expression current) { - var first = firstPass.prepare(parent, current); - var second = secondPass.prepare(parent, current); - if (first == firstPass && second == secondPass) { + var firstPrepared = firstPass == null ? null : firstPass.prepare(parent, current); + var secondPrepared = secondPass == null ? null : secondPass.prepare(parent, current); + if (firstPrepared == firstPass && secondPrepared == secondPass) { return this; } else { - return new ChainedMiniPass(first, second); + return new ChainedMiniPass(firstPrepared, secondPrepared); } } @Override public Expression transformExpression(Expression ir) { - var fstIr = firstPass.transformExpression(ir); - var sndIr = secondPass.transformExpression(fstIr); + var fstIr = firstPass == null ? ir : firstPass.transformExpression(ir); + var sndIr = secondPass == null ? fstIr : secondPass.transformExpression(fstIr); return sndIr; } @Override public Module transformModule(Module moduleIr) { - var first = firstPass.transformModule(moduleIr); - var second = secondPass.transformModule(first); + var first = firstPass == null ? moduleIr : firstPass.transformModule(moduleIr); + var second = secondPass == null ? first : secondPass.transformModule(first); return second; } @Override public boolean checkPostCondition(IR ir) { - return firstPass.checkPostCondition(ir) && secondPass.checkPostCondition(ir); + var firstCheck = firstPass == null || firstPass.checkPostCondition(ir); + var secondCheck = secondPass == null || secondPass.checkPostCondition(ir); + return firstCheck && secondCheck; } @Override From e0eba27073e9c974d88b90ef6a21be2f108dfe33 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 11:35:08 +0100 Subject: [PATCH 21/63] Fix compilation error in PasesTest --- .../src/test/scala/org/enso/compiler/test/PassesTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala index fbbc284a6b72..d141d3dcb702 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala @@ -65,7 +65,7 @@ class PassesTest extends CompilerTest { OperatorToFunction, LambdaShorthandToLambda, ImportSymbolAnalysis.INSTANCE, - AmbiguousImportsAnalysis, + AmbiguousImportsAnalysis.INSTANCE, PrivateModuleAnalysis.INSTANCE, PrivateConstructorAnalysis.INSTANCE, ShadowedPatternFields, From a204ed2db357c9ee156bc2c691a7570531842158 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 11:56:14 +0100 Subject: [PATCH 22/63] Fix NPE --- .../enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java index d2078ec0c616..a7156969927c 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java @@ -240,7 +240,7 @@ private void tryAddEncounteredSymbol( if (symbolPath.equals(encounteredFullName)) { // symbolName is already imported with the same symbolPath --> attach warning. var warn = createWarningForDuplicatedImport(originalImport, currentImport, symbolName); - currentImport.diagnostics().add(warn); + currentImport.getDiagnostics().add(warn); } else { // There is an encountered symbol with different physical path than symbolPath. var resolution = encounteredSymbols.getResolvedNameForSymbol(symbolName); From 5a74cc1e0c036e9b644a53f0e689ff873b927100 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 11:56:26 +0100 Subject: [PATCH 23/63] Fix errors in polyglot imports --- .../org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java | 1 - 1 file changed, 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java index a7156969927c..bfbe7aabd1dd 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java @@ -208,7 +208,6 @@ private List analyseAmbiguousSymbols(Import imp) { throw new IllegalStateException("Unsupported polyglot entity: " + polyglotImp.entity()); } tryAddEncounteredSymbol(polyglotImp, symbolName, symbolPath, null, errorsForImport); - return List.of(); } default -> {} From 92a07254c76310f1a2c8403d6cd329ca3839fab2 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 11:56:46 +0100 Subject: [PATCH 24/63] Remove unnecessary isntancdeof --- .../enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java index bfbe7aabd1dd..1bbbdd054a14 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java @@ -102,8 +102,8 @@ public Expression transformExpression(Expression expr) { public Module transformModule(Module moduleIr) { var newImports = new ArrayList(); moduleIr.imports().foreach(imp -> { - if (imp instanceof Import.Module impMod) { - var errs = analyseAmbiguousSymbols(impMod); + var errs = analyseAmbiguousSymbols(imp); + if (!errs.isEmpty()) { newImports.addAll(errs); } else { newImports.add(imp); From 62cecbb55849b3f3b99136d20fdb62cc6352f6ab Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 12:19:09 +0100 Subject: [PATCH 25/63] MiniPassTest does not call PassManager.runModule directly. This violates the consistency between IR from ModuleContext. --- .../scala/org/enso/compiler/test/CompilerTest.scala | 2 +- .../scala/org/enso/compiler/test/MiniPassTest.scala | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala index 0e51fd5fada4..ad7d12663e0b 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala @@ -36,7 +36,7 @@ trait CompilerRunner { * be too complex. Let's just hack it via reflection here. * @return */ - private def getPassesViaReflection( + protected def getPassesViaReflection( passManager: PassManager ): List[PassGroup] = { val passesField = passManager.getClass.getDeclaredField("passes") diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/MiniPassTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/MiniPassTest.scala index d26dcd80ca9a..4b4c2203a5b2 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/MiniPassTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/MiniPassTest.scala @@ -84,9 +84,8 @@ trait MiniPassTest extends CompilerTest { source: String, moduleCtx: ModuleContext ): Module = { - val module = parseModule(source) - val preprocessedModule = - megaPassManager.runPassesOnModule(module, moduleCtx) + val module = parseModule(source) + val preprocessedModule = module.runPasses(megaPassManager, moduleCtx) megaPass.runModule(preprocessedModule, moduleCtx) } @@ -94,10 +93,9 @@ trait MiniPassTest extends CompilerTest { source: String, moduleCtx: ModuleContext ): Module = { - val module = parseModule(source) - val miniPass = miniPassFactory.createForModuleCompilation(moduleCtx) - val preprocessedModule = - megaPassManager.runPassesOnModule(module, moduleCtx) + val module = parseModule(source) + val miniPass = miniPassFactory.createForModuleCompilation(moduleCtx) + val preprocessedModule = module.runPasses(megaPassManager, moduleCtx) MiniIRPass.compile(classOf[Module], preprocessedModule, miniPass) } From e27a59f1904c5748a9fccd3d3d776e90575258b3 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 14:49:48 +0100 Subject: [PATCH 26/63] Move tests from runtime-compiler to runtime-integration-tests --- .../enso/compiler/pass/test/MockMiniPass.java | 48 -------- .../mini/passes}/MiniPassTraverserTest.java | 2 +- .../test/mini/passes}/MockExpression.java | 2 +- .../test/mini/passes/MockMiniPass.java | 114 ++++++++++++++++++ 4 files changed, 116 insertions(+), 50 deletions(-) delete mode 100644 engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java rename engine/{runtime-compiler/src/test/java/org/enso/compiler/pass/test => runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes}/MiniPassTraverserTest.java (99%) rename engine/{runtime-compiler/src/test/java/org/enso/compiler/pass/test => runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes}/MockExpression.java (98%) create mode 100644 engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java deleted file mode 100644 index 801d47412e31..000000000000 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockMiniPass.java +++ /dev/null @@ -1,48 +0,0 @@ -package org.enso.compiler.pass.test; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; - -import org.enso.compiler.core.IR; -import org.enso.compiler.core.ir.Expression; -import org.enso.compiler.pass.MiniIRPass; - -final class MockMiniPass extends MiniIRPass { - private final MockExpression stopExpr; - - /** - * @param stopExpr When encountered this expression, {@code prepare} method will return null to - * signal that the traversal should stop. Can be null. - */ - MockMiniPass(MockExpression stopExpr) { - this.stopExpr = stopExpr; - } - - @Override - public Expression transformExpression(Expression expr) { - if (expr instanceof MockExpression mockExpr) { - if (mockExpr.hasParent()) { - assertThat( - "Prepare must be called on an expression with a parent", - mockExpr.isPreparedBy(this), - is(true)); - } - assertThat( - "Transform is called just once by one pass", mockExpr.isTransformedBy(this), is(false)); - mockExpr.setTransformedByPass(this); - } - return expr; - } - - @Override - public MiniIRPass prepare(IR parent, Expression child) { - if (stopExpr == child) { - return null; - } - if (child instanceof MockExpression mockExpr) { - assertThat("Prepare is called just once by one pass", mockExpr.isPreparedBy(this), is(false)); - mockExpr.setPreparedBy(this); - } - return this; - } -} diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java similarity index 99% rename from engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java rename to engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java index f88cf7810b6d..f62b3c70e9a5 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MiniPassTraverserTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java @@ -1,4 +1,4 @@ -package org.enso.compiler.pass.test; +package org.enso.compiler.test.mini.passes; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockExpression.java similarity index 98% rename from engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java rename to engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockExpression.java index e80661f00a72..943d5cd3d6eb 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/pass/test/MockExpression.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockExpression.java @@ -1,4 +1,4 @@ -package org.enso.compiler.pass.test; +package org.enso.compiler.test.mini.passes; import java.util.ArrayList; import java.util.HashSet; diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java new file mode 100644 index 000000000000..7c0fdc12def1 --- /dev/null +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java @@ -0,0 +1,114 @@ +package org.enso.compiler.test.mini.passes; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import java.util.HashSet; +import java.util.Objects; +import java.util.Set; +import org.enso.compiler.core.IR; +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.pass.IRProcessingPass; +import org.enso.compiler.pass.MiniIRPass; +import org.enso.compiler.test.mini.passes.Event.Kind; +import scala.collection.immutable.Seq; +import scala.jdk.javaapi.CollectionConverters; + +final class MockMiniPass extends MiniIRPass implements IRProcessingPass { + private final MockExpression stopExpr; + private final EventRecorder eventRecorder; + private final Set precursorPasses; + private final Set invalidatedPasses = new HashSet<>(); + + /** + * @param stopExpr When encountered this expression, {@code prepare} method will return + * null to signal that the traversal should stop. Can be null. + * @param eventRecorder Recorder of {@code prepare} and {@code transform} events. May be null + * @param precursorPasses + */ + private MockMiniPass(MockExpression stopExpr, EventRecorder eventRecorder, + Set precursorPasses) { + this.stopExpr = stopExpr; + this.eventRecorder = eventRecorder; + this.precursorPasses = precursorPasses; + } + + static Builder builder() { + return new Builder(); + } + + void addInvalidatedPass(MockMiniPass pass) { + invalidatedPasses.add(pass); + } + + @Override + public Expression transformExpression(Expression expr) { + if (expr instanceof MockExpression mockExpr) { + if (mockExpr.hasParent()) { + assertThat( + "Prepare must be called on an expression with a parent", + mockExpr.isPreparedBy(this), + is(true)); + } + assertThat( + "Transform is called just once by one pass", mockExpr.isTransformedBy(this), is(false)); + mockExpr.setTransformedByPass(this); + if (eventRecorder != null) { + eventRecorder.record(Kind.Transform, this, mockExpr); + } + } + return expr; + } + + @Override + public MiniIRPass prepare(IR parent, Expression child) { + if (stopExpr == child) { + return null; + } + if (child instanceof MockExpression mockExpr) { + assertThat("Prepare is called just once by one pass", mockExpr.isPreparedBy(this), is(false)); + mockExpr.setPreparedBy(this); + if (eventRecorder != null) { + eventRecorder.record(Kind.Prepare, this, mockExpr); + } + } + return this; + } + + @Override + public Seq precursorPasses() { + return CollectionConverters.asScala(precursorPasses).toSeq(); + } + + @Override + public Seq invalidatedPasses() { + return CollectionConverters.asScala(invalidatedPasses).toSeq(); + } + + + static final class Builder { + private MockExpression stopExpr; + private EventRecorder eventRecorder; + private Set precursorPasses = new HashSet<>(); + + Builder stopExpr(MockExpression stopExpr) { + this.stopExpr = stopExpr; + return this; + } + + Builder eventRecorder(EventRecorder eventRecorder) { + this.eventRecorder = eventRecorder; + return this; + } + + Builder precursorPasses(Set precursorPasses) { + this.precursorPasses = precursorPasses; + return this; + } + + MockMiniPass build() { + Objects.requireNonNull(precursorPasses); + return new MockMiniPass(stopExpr, eventRecorder, precursorPasses); + } + } +} From 2b5c76c238e325ff1cd988a200e55436f77fbea4 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 14:50:21 +0100 Subject: [PATCH 27/63] MockMiniPass uses builder pattern --- .../test/mini/passes/MiniPassTraverserTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java index f62b3c70e9a5..e94d3bec687e 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java @@ -11,7 +11,7 @@ public class MiniPassTraverserTest { @Test public void traversesOneExpression() { var expr = new MockExpression(false); - var miniPass = new MockMiniPass(null); + var miniPass = MockMiniPass.builder().build(); MiniIRPass.compile(MockExpression.class, expr, miniPass); assertThat( "Prepare is called only for trees with depth > 1", expr.isPreparedByAny(), is(false)); @@ -23,7 +23,7 @@ public void traversesExpressionWithOneChild() { var parentExpr = new MockExpression(false); var childExpr = new MockExpression(true); parentExpr.addChild(childExpr); - var miniPass = new MockMiniPass(null); + var miniPass = MockMiniPass.builder().build(); MiniIRPass.compile(MockExpression.class, parentExpr, miniPass); assertThat( "Prepare must be called on a child expression", childExpr.isPreparedByAny(), is(true)); @@ -36,7 +36,7 @@ public void traversesExpressionWithManyChildren() { var parentExpr = new MockExpression(false); var children = List.of(new MockExpression(true), new MockExpression(true)); children.forEach(parentExpr::addChild); - var miniPass = new MockMiniPass(null); + var miniPass = MockMiniPass.builder().build(); MiniIRPass.compile(MockExpression.class, parentExpr, miniPass); for (var ch : children) { assertThat("Prepare must be called on a child expression", ch.isPreparedByAny(), is(true)); @@ -54,7 +54,7 @@ public void stopTraversingWhenPrepareReturnsNull() { e2.addChild(e3); // Should stop traversing when e3 is encountered. // Should only process e1 and e2, not e3 - var miniPass = new MockMiniPass(e3); + var miniPass = MockMiniPass.builder().stopExpr(e3).build(); MiniIRPass.compile(MockExpression.class, e1, miniPass); assertThat("e3 should not be processed", e3.isPreparedByAny(), is(false)); assertThat("e3 should not be processed", e3.isTransformedByAny(), is(false)); @@ -67,8 +67,8 @@ public void chainedMiniPass_TraversesSingleExpression() { var parentExpr = new MockExpression(false); var childExpr = new MockExpression(true); parentExpr.addChild(childExpr); - var miniPass1 = new MockMiniPass(null); - var miniPass2 = new MockMiniPass(null); + var miniPass1 = MockMiniPass.builder().build(); + var miniPass2 = MockMiniPass.builder().build(); var chainedPass = MiniIRPass.combine(miniPass1, miniPass2); MiniIRPass.compile(MockExpression.class, parentExpr, chainedPass); assertThat( @@ -93,9 +93,9 @@ public void chainedMiniPass_StopsTraversingWhenPrepareReturnsNull() { e1.addChild(e2); e2.addChild(e3); // miniPass1 stops traversing on e2. - var miniPass1 = new MockMiniPass(e3); + var miniPass1 = MockMiniPass.builder().stopExpr(e3).build(); // miniPass2 traverses everything. - var miniPass2 = new MockMiniPass(null); + var miniPass2 = MockMiniPass.builder().build(); var chainedPass = MiniIRPass.combine(miniPass1, miniPass2); MiniIRPass.compile(MockExpression.class, e1, chainedPass); assertThat("e3 should be prepared only by miniPass2", e3.isPreparedBy(miniPass2), is(true)); From 30334624982de71f94f450f3fa78e77d7f865646 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 17:36:28 +0100 Subject: [PATCH 28/63] Remove EventRecorder, MockMiniPass is not IRProcessingPass --- .../test/mini/passes/MockMiniPass.java | 55 +------------------ 1 file changed, 3 insertions(+), 52 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java index 7c0fdc12def1..c10c49bfec3d 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java @@ -3,44 +3,25 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; -import java.util.HashSet; -import java.util.Objects; -import java.util.Set; import org.enso.compiler.core.IR; import org.enso.compiler.core.ir.Expression; -import org.enso.compiler.pass.IRProcessingPass; import org.enso.compiler.pass.MiniIRPass; -import org.enso.compiler.test.mini.passes.Event.Kind; -import scala.collection.immutable.Seq; -import scala.jdk.javaapi.CollectionConverters; -final class MockMiniPass extends MiniIRPass implements IRProcessingPass { +final class MockMiniPass extends MiniIRPass { private final MockExpression stopExpr; - private final EventRecorder eventRecorder; - private final Set precursorPasses; - private final Set invalidatedPasses = new HashSet<>(); /** * @param stopExpr When encountered this expression, {@code prepare} method will return * null to signal that the traversal should stop. Can be null. - * @param eventRecorder Recorder of {@code prepare} and {@code transform} events. May be null - * @param precursorPasses */ - private MockMiniPass(MockExpression stopExpr, EventRecorder eventRecorder, - Set precursorPasses) { + private MockMiniPass(MockExpression stopExpr) { this.stopExpr = stopExpr; - this.eventRecorder = eventRecorder; - this.precursorPasses = precursorPasses; } static Builder builder() { return new Builder(); } - void addInvalidatedPass(MockMiniPass pass) { - invalidatedPasses.add(pass); - } - @Override public Expression transformExpression(Expression expr) { if (expr instanceof MockExpression mockExpr) { @@ -53,9 +34,6 @@ public Expression transformExpression(Expression expr) { assertThat( "Transform is called just once by one pass", mockExpr.isTransformedBy(this), is(false)); mockExpr.setTransformedByPass(this); - if (eventRecorder != null) { - eventRecorder.record(Kind.Transform, this, mockExpr); - } } return expr; } @@ -68,47 +46,20 @@ public MiniIRPass prepare(IR parent, Expression child) { if (child instanceof MockExpression mockExpr) { assertThat("Prepare is called just once by one pass", mockExpr.isPreparedBy(this), is(false)); mockExpr.setPreparedBy(this); - if (eventRecorder != null) { - eventRecorder.record(Kind.Prepare, this, mockExpr); - } } return this; } - @Override - public Seq precursorPasses() { - return CollectionConverters.asScala(precursorPasses).toSeq(); - } - - @Override - public Seq invalidatedPasses() { - return CollectionConverters.asScala(invalidatedPasses).toSeq(); - } - - static final class Builder { private MockExpression stopExpr; - private EventRecorder eventRecorder; - private Set precursorPasses = new HashSet<>(); Builder stopExpr(MockExpression stopExpr) { this.stopExpr = stopExpr; return this; } - Builder eventRecorder(EventRecorder eventRecorder) { - this.eventRecorder = eventRecorder; - return this; - } - - Builder precursorPasses(Set precursorPasses) { - this.precursorPasses = precursorPasses; - return this; - } - MockMiniPass build() { - Objects.requireNonNull(precursorPasses); - return new MockMiniPass(stopExpr, eventRecorder, precursorPasses); + return new MockMiniPass(stopExpr); } } } From 0a4ac1f922f63063af524ab8be8e5734969b237a Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 17:39:17 +0100 Subject: [PATCH 29/63] Move MiniPassTraverserTest back into runtime-compiler/Test --- .../org/enso/compiler/test/pass}/MiniPassTraverserTest.java | 2 +- .../java/org/enso/compiler/test/pass}/MockExpression.java | 2 +- .../java/org/enso/compiler/test/pass}/MockMiniPass.java | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) rename engine/{runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes => runtime-compiler/src/test/java/org/enso/compiler/test/pass}/MiniPassTraverserTest.java (99%) rename engine/{runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes => runtime-compiler/src/test/java/org/enso/compiler/test/pass}/MockExpression.java (98%) rename engine/{runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes => runtime-compiler/src/test/java/org/enso/compiler/test/pass}/MockMiniPass.java (87%) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MiniPassTraverserTest.java similarity index 99% rename from engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java rename to engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MiniPassTraverserTest.java index e94d3bec687e..de50a8eb68cf 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MiniPassTraverserTest.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MiniPassTraverserTest.java @@ -1,4 +1,4 @@ -package org.enso.compiler.test.mini.passes; +package org.enso.compiler.test.pass; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockExpression.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MockExpression.java similarity index 98% rename from engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockExpression.java rename to engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MockExpression.java index 943d5cd3d6eb..244e02eeba16 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockExpression.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MockExpression.java @@ -1,4 +1,4 @@ -package org.enso.compiler.test.mini.passes; +package org.enso.compiler.test.pass; import java.util.ArrayList; import java.util.HashSet; diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MockMiniPass.java similarity index 87% rename from engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java rename to engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MockMiniPass.java index c10c49bfec3d..e0792a9bc27c 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/mini/passes/MockMiniPass.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MockMiniPass.java @@ -1,4 +1,4 @@ -package org.enso.compiler.test.mini.passes; +package org.enso.compiler.test.pass; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -11,8 +11,8 @@ final class MockMiniPass extends MiniIRPass { private final MockExpression stopExpr; /** - * @param stopExpr When encountered this expression, {@code prepare} method will return - * null to signal that the traversal should stop. Can be null. + * @param stopExpr When encountered this expression, {@code prepare} method will return null to + * signal that the traversal should stop. Can be null. */ private MockMiniPass(MockExpression stopExpr) { this.stopExpr = stopExpr; From 6d2f6c673cfba54ecd9f87a492402b103e5b1a4e Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 18:36:00 +0100 Subject: [PATCH 30/63] PassManager.runInline also chains mini passes. Make the algorithm for running passes generic --- .../enso/compiler/pass/IRProcessingPass.java | 7 +- .../org/enso/compiler/pass/PassManager.scala | 169 ++++++++++-------- 2 files changed, 103 insertions(+), 73 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/IRProcessingPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/IRProcessingPass.java index 2d38422d488f..8de4d16b8c5f 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/IRProcessingPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/IRProcessingPass.java @@ -11,6 +11,11 @@ public interface IRProcessingPass extends ProcessingPass { /** The passes that this pass depends _directly_ on to run. */ public Seq precursorPasses(); - /** The passes that are invalidated by running this pass. */ + /** + * The passes that are invalidated by running this pass. + * + *

If {@code P1} invalidates {@code P2}, and {@code P1} is a precursor of {@code P2}, then + * {@code P1} must finish before {@code P2} starts. + */ public Seq invalidatedPasses(); } diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index cfd2ffde9395..168b9c776520 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -4,9 +4,11 @@ import org.enso.common.{Asserts, CompilationStage} import org.slf4j.LoggerFactory import org.enso.compiler.context.{InlineContext, ModuleContext} import org.enso.compiler.core.ir.{Expression, Module} -import org.enso.compiler.core.CompilerError +import org.enso.compiler.core.{CompilerError, IR} import org.enso.compiler.pass.analyse.BindingAnalysis +import scala.collection.mutable.ListBuffer + // TODO [AA] In the future, the pass ordering should be _computed_ from the list // of available passes, rather than just verified. @@ -65,6 +67,7 @@ class PassManager( * @param moduleContext the module context in which the passes are executed * @return the result of executing `passGroup` on `ir` */ + // TODO: Remove this method def runPassesOnModule( ir: Module, moduleContext: ModuleContext @@ -94,77 +97,29 @@ class PassManager( throw new CompilerError("Cannot run an unvalidated pass group.") } - val newContext = - moduleContext.copy(passConfiguration = Some(passConfiguration)) - - val passesWithIndex = passGroup.passes.zipWithIndex - logger.debug( "runPassesOnModule[{}@{}]", moduleContext.getName(), moduleContext.module.getCompilationStage() ) - var pendingMiniPasses: List[MiniPassFactory] = List() - def flushMiniPass(in: Module): Module = { - if (pendingMiniPasses.nonEmpty) { - val miniPasses = pendingMiniPasses.map(factory => - factory.createForModuleCompilation(newContext) - ) - val combinedPass = miniPasses.fold(null)(MiniIRPass.combine) - pendingMiniPasses = List() - if (combinedPass != null) { - logger.trace(" flushing pending mini pass: {}", combinedPass) - MiniIRPass.compile(classOf[Module], in, combinedPass) - } else { - in - } - } else { - in - } - } - val res = passesWithIndex.foldLeft(ir) { - case (intermediateIR, (pass, index)) => { - pass match { - case miniFactory: MiniPassFactory => - logger.trace( - " mini collected: {}", - pass - ) - val combiningPreventedBy = pendingMiniPasses.find { p => - p.invalidatedPasses.contains(miniFactory) - } - val irForRemainingMiniPasses = if (combiningPreventedBy.isDefined) { - logger.trace( - " pass {} forces flush before {}", - combiningPreventedBy.orNull, - miniFactory - ) - flushMiniPass(intermediateIR) - } else { - intermediateIR - } - pendingMiniPasses = pendingMiniPasses.appended(miniFactory) - irForRemainingMiniPasses - case megaPass: IRPass => - // TODO [AA, MK] This is a possible race condition. - passConfiguration - .get(megaPass) - .foreach(c => - c.shouldWriteToContext = isLastRunOf(index, megaPass, passGroup) - ) - val flushedIR = flushMiniPass(intermediateIR) - logger.trace( - " mega running: {}", - megaPass - ) - megaPass.runModule(flushedIR, newContext) - } - } - } - flushMiniPass(res) + + val newContext = + moduleContext.copy(passConfiguration = Some(passConfiguration)) + + runPasses[Module, ModuleContext]( + ir, + newContext, + passGroup, + createMiniPass = + (factory, ctx) => factory.createForModuleCompilation(ctx), + miniPassCompile = (miniPass, ir) => + MiniIRPass.compile[Module](classOf[Module], ir, miniPass), + megaPassCompile = (megaPass, ir, ctx) => megaPass.runModule(ir, ctx) + ) } /** Executes all passes on the [[Expression]]. + * TODO: Remove this method? * * @param ir the expression to execute the compiler passes on * @param inlineContext the inline context in which the passes are executed @@ -198,15 +153,80 @@ class PassManager( val newContext = inlineContext.copy(passConfiguration = Some(passConfiguration)) - val passesWithIndex = passGroup.passes.zipWithIndex + runPasses[Expression, InlineContext]( + ir, + newContext, + passGroup, + createMiniPass = + (factory, ctx) => factory.createForInlineCompilation(ctx), + miniPassCompile = (miniPass, ir) => + MiniIRPass.compile[Expression](classOf[Expression], ir, miniPass), + megaPassCompile = (megaPass, ir, ctx) => megaPass.runExpression(ir, ctx) + ) + } - passesWithIndex.foldLeft(ir) { - case (intermediateIR, (pass, index)) => { + /** Runs all the passes in the given `passGroup` on `ir` with `context`. + * @param createMiniPass Function that creates a minipass. + * @param miniPassCompile Function that compiles IR with mini pass. + * @param megaPassCompile Function that compiles IR with mega pass. + * @tparam IRType Type of the [[IR]] that is being compiled. + * @tparam ContextType Type of the context for the compilation. + * Either [[ModuleContext]] or [[InlineContext]] + * @return Compiled IR. Might be the same reference as `ir` if no compilation was done. + */ + private def runPasses[IRType <: IR, ContextType]( + ir: IRType, + context: ContextType, + passGroup: PassGroup, + createMiniPass: (MiniPassFactory, ContextType) => MiniIRPass, + miniPassCompile: (MiniIRPass, IRType) => IRType, + megaPassCompile: (IRPass, IRType, ContextType) => IRType + ): IRType = { + val pendingMiniPasses: ListBuffer[MiniPassFactory] = ListBuffer() + def flushMiniPasses(in: IRType): IRType = { + if (pendingMiniPasses.nonEmpty) { + val miniPasses = + pendingMiniPasses.map(factory => createMiniPass(factory, context)) + val combinedPass = miniPasses.fold(null)(MiniIRPass.combine) + pendingMiniPasses.clear() + if (combinedPass != null) { + logger.trace(" flushing pending mini pass: {}", combinedPass) + miniPassCompile(combinedPass, in) + } else { + in + } + } else { + in + } + } + + val passesWithIndex = passGroup.passes.zipWithIndex + val res = passesWithIndex.foldLeft(ir) { + case (intermediateIR, (pass, index)) => pass match { case miniFactory: MiniPassFactory => - val miniPass = miniFactory.createForInlineCompilation(newContext) - MiniIRPass.compile(classOf[Expression], intermediateIR, miniPass) + logger.trace( + " mini collected: {}", + pass + ) + val combiningPreventedByOpt = pendingMiniPasses.find { p => + p.invalidatedPasses.contains(miniFactory) + } + val irForRemainingMiniPasses = combiningPreventedByOpt match { + case Some(combiningPreventedBy) => + logger.trace( + " pass {} forces flush before (invalidates) {}", + combiningPreventedBy, + miniFactory + ) + flushMiniPasses(intermediateIR) + case None => + intermediateIR + } + pendingMiniPasses.addOne(miniFactory) + irForRemainingMiniPasses + case megaPass: IRPass => // TODO [AA, MK] This is a possible race condition. passConfiguration @@ -214,11 +234,16 @@ class PassManager( .foreach(c => c.shouldWriteToContext = isLastRunOf(index, megaPass, passGroup) ) - megaPass.runExpression(intermediateIR, newContext) + val flushedIR = flushMiniPasses(intermediateIR) + logger.trace( + " mega running: {}", + megaPass + ) + megaPassCompile(megaPass, flushedIR, context) } - - } } + + flushMiniPasses(res) } /** Determines whether the run at index `indexOfPassInGroup` is the last run From 239029a15a00880fa3338d7dcfb6dedf05dbf4cf Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 21 Nov 2024 18:44:53 +0100 Subject: [PATCH 31/63] MiniPassFactory.createForInlineCompilation can return null. --- .../src/main/java/org/enso/compiler/pass/MiniIRPass.java | 9 +++++---- .../java/org/enso/compiler/pass/MiniPassFactory.java | 2 +- .../compiler/pass/analyse/AmbiguousImportsAnalysis.java | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java index 3097e2eacf35..46ad0a580700 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java @@ -106,15 +106,16 @@ public String toString() { * * @param first first mini pass (can be {@code null}) * @param second second mini pass (can be {@code null}) - * @return a combined pass that calls both non-{@code null} of the provided passes + * @return a combined pass that calls both non-{@code null} of the provided passes. {@code null} + * if both provided passes are {@code null}. */ public static MiniIRPass combine(MiniIRPass first, MiniIRPass second) { - if (first == null && second == null) { - return null; - } if (first == null) { return second; } + if (second == null) { + return first; + } return ChainedMiniPass.chain(first, second); } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassFactory.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassFactory.java index 29fce1af76ca..8a57b8ccc2e1 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassFactory.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniPassFactory.java @@ -26,7 +26,7 @@ public interface MiniPassFactory extends IRProcessingPass { * an inline compilation. * * @param inlineContext A mini pass can optionally save reference to this inline context. - * @return Must not return {@code null}. Inline compilation should always be supported. + * @return May return {@code null} if the inline compilation is not supported. */ MiniIRPass createForInlineCompilation(InlineContext inlineContext); } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java index 1bbbdd054a14..8eb987d39b4f 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java @@ -73,7 +73,7 @@ public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { @Override public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { - throw new UnsupportedOperationException("unimplemented"); + return null; } From f2687dbdc34392d25b1701c3357772855f6c8ff8 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 11:48:41 +0100 Subject: [PATCH 32/63] Fix IR inconsistency in TypeInferenceTest --- .../java/org/enso/compiler/test/TypeInferenceTest.java | 3 ++- .../scala/org/enso/compiler/test/CompilerTest.scala | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/TypeInferenceTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/TypeInferenceTest.java index 038ae28e76d5..33aa8b37b74f 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/TypeInferenceTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/TypeInferenceTest.java @@ -1184,7 +1184,8 @@ public CompilerConfig defaultConfig() { ModuleContext moduleContext = compilerRunner.buildModuleContext( moduleName, Option.apply(new FreshNameSupply()), Option.empty(), compilerConfig, false); - Module processedModule = passManager.runPassesOnModule(rawModule, moduleContext); + Module processedModule = + compilerRunner.runPassesOnModule(rawModule, passManager, moduleContext); return processedModule; } } diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala index ad7d12663e0b..cff592f1941d 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala @@ -75,6 +75,16 @@ trait CompilerRunner { } } + /** Wrapper for the implicit method. Callable from Java. + */ + protected def runPassesOnModule( + ir: Module, + passManager: PassManager, + moduleContext: ModuleContext + ): Module = { + ir.runPasses(passManager, moduleContext) + } + /** Provides an extensions to work with diagnostic information attached to IR. * * @param ir the IR node From 6ead57e3be5120df8ed90da3007970e7d16f5cc6 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 11:49:06 +0100 Subject: [PATCH 33/63] Remove unused method from PassManager --- .../org/enso/compiler/pass/PassManager.scala | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 168b9c776520..9de06659266f 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -61,24 +61,6 @@ class PassManager( passes } - /** Executes all pass groups on the [[Module]]. - * - * @param ir the module to execute the compiler passes on - * @param moduleContext the module context in which the passes are executed - * @return the result of executing `passGroup` on `ir` - */ - // TODO: Remove this method - def runPassesOnModule( - ir: Module, - moduleContext: ModuleContext - ): Module = { - Asserts.assertInJvm(validateConsistency(ir, moduleContext)) - - passes.foldLeft(ir)((ir, group) => - runPassesOnModule(ir, moduleContext, group) - ) - } - /** Executes the provided `passGroup` on the [[Module]]. * * @param ir the module to execute the compiler passes on From 2bcdede83dc8a02e2bb9e32eb0128f5cbb81ec33 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 11:56:39 +0100 Subject: [PATCH 34/63] ImportSymbolAnalysis does not support inline compilation. This fixes std-benchmarks/compile. --- .../org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java index 5b8dbd64f41c..78a895e4fa5e 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -52,8 +52,8 @@ public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { @Override public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { - var bindingsMap = inlineContext.bindingsAnalysis(); - return new Mini(bindingsMap); + // Does not make sense for inline compilation. + return null; } private static final class Mini extends MiniIRPass { From 04181f9543bef97f67c8cb9a8aab2e9e41bf1a5d Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 12:11:18 +0100 Subject: [PATCH 35/63] Convert PrivateModuleAnalysis to mini pass --- .../pass/analyse/PrivateModuleAnalysis.java | 203 ++++++++++-------- 1 file changed, 118 insertions(+), 85 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java index bfbb476de4d4..f5cf1c040c25 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java @@ -11,8 +11,10 @@ import org.enso.compiler.core.ir.module.scope.Export; import org.enso.compiler.core.ir.module.scope.Import; import org.enso.compiler.data.BindingsMap; -import org.enso.compiler.pass.IRPass; import org.enso.compiler.pass.IRProcessingPass; +import org.enso.compiler.pass.MiniIRPass; +import org.enso.compiler.pass.MiniPassFactory; +import org.enso.pkg.Package; import org.enso.pkg.QualifiedName; import scala.Option; import scala.collection.immutable.Seq; @@ -29,7 +31,7 @@ * * Inserts errors into imports/exports IRs if the above conditions are violated. */ -public final class PrivateModuleAnalysis implements IRPass { +public final class PrivateModuleAnalysis implements MiniPassFactory { public static final PrivateModuleAnalysis INSTANCE = new PrivateModuleAnalysis(); private PrivateModuleAnalysis() {} @@ -49,89 +51,125 @@ public Seq invalidatedPasses() { } @Override - public Module runModule(Module moduleIr, ModuleContext moduleContext) { - var bindingsMap = (BindingsMap) moduleIr.passData().get(BindingAnalysis$.MODULE$).get(); - var currentPackage = moduleContext.getPackage(); - List importErrors = new ArrayList<>(); - List exportErrors = new ArrayList<>(); - var isCurrentModulePrivate = moduleIr.isPrivate(); - var isCurrentModuleSynthetic = moduleContext.isSynthetic(); - - // Ensure that imported modules from a different project are not private. - bindingsMap - .resolvedImports() - .foreach( - resolvedImp -> { - var importedTargets = resolvedImp.targets(); - importedTargets.foreach( - importedTarget -> { - var importedModule = importedTarget.module().unsafeAsModule("should succeed"); - var importedModuleName = importedModule.getName().toString(); - var importedModulePackage = importedModule.getPackage(); - if (currentPackage != null - && !currentPackage.equals(importedModulePackage) - && importedModule.isPrivate()) { - importErrors.add( - ImportExport.apply( - resolvedImp.importDef(), - new ImportExport.ImportPrivateModule(importedModuleName), - ImportExport.apply$default$3())); - } - return null; - }); - return null; - }); + public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { + return new Mini( + moduleContext.bindingsAnalysis(), + moduleContext.getName().toString(), + moduleContext.getPackage(), + moduleContext.isSynthetic()); + } + + @Override + public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { + return null; + } - // Ensure that no symbols are exported from a private module. - if (isCurrentModulePrivate && containsExport(moduleIr)) { - exportErrors.add( - ImportExport.apply( - moduleIr.exports().apply(0), - new ImportExport.ExportSymbolsFromPrivateModule(moduleContext.getName().toString()), - ImportExport.apply$default$3())); + private static final class Mini extends MiniIRPass { + private final BindingsMap bindingsMap; + private final String moduleName; + private final Package currentPackage; + private final boolean isSynthetic; + + private Mini( + BindingsMap bindingsMap, + String moduleName, + Package currentPackage, + boolean isSynthetic) { + assert bindingsMap != null; + assert moduleName != null; + this.moduleName = moduleName; + this.currentPackage = currentPackage; + this.isSynthetic = isSynthetic; + this.bindingsMap = bindingsMap; } - // Ensure that private modules are not exported - bindingsMap - .getDirectlyExportedModules() - .foreach( - expModule -> { - var expModuleRef = expModule.module().module().unsafeAsModule("should succeed"); - if (expModuleRef.isPrivate() && !isCurrentModuleSynthetic) { - var associatedExportIR = findExportIRByName(moduleIr, expModuleRef.getName()); - assert associatedExportIR.isDefined(); - exportErrors.add( - ImportExport.apply( - associatedExportIR.get(), - new ImportExport.ExportPrivateModule(expModuleRef.getName().toString()), - ImportExport.apply$default$3())); - } - return null; - }); + @Override + public MiniIRPass prepare(IR parent, Expression child) { + return null; + } - scala.collection.immutable.List convertedImports = - importErrors.isEmpty() - ? moduleIr.imports() - : CollectionConverters.asScala(importErrors).toList(); - scala.collection.immutable.List convertedExports = - exportErrors.isEmpty() - ? moduleIr.exports() - : CollectionConverters.asScala(exportErrors).toList(); - - return moduleIr.copy( - convertedImports, - convertedExports, - moduleIr.copy$default$3(), - moduleIr.copy$default$4(), - moduleIr.copy$default$5(), - moduleIr.copy$default$6(), - moduleIr.copy$default$7(), - moduleIr.copy$default$8()); - } + @Override + public Expression transformExpression(Expression expr) { + throw new IllegalStateException("Should not be called - prepare returns null"); + } - @Override - public Expression runExpression(Expression ir, InlineContext inlineContext) { - return ir; + @Override + public Module transformModule(Module moduleIr) { + List importErrors = new ArrayList<>(); + List exportErrors = new ArrayList<>(); + var isCurrentModulePrivate = moduleIr.isPrivate(); + + // Ensure that imported modules from a different project are not private. + bindingsMap + .resolvedImports() + .foreach( + resolvedImp -> { + var importedTargets = resolvedImp.targets(); + importedTargets.foreach( + importedTarget -> { + var importedModule = importedTarget.module().unsafeAsModule("should succeed"); + var importedModuleName = importedModule.getName().toString(); + var importedModulePackage = importedModule.getPackage(); + if (currentPackage != null + && !currentPackage.equals(importedModulePackage) + && importedModule.isPrivate()) { + importErrors.add( + ImportExport.apply( + resolvedImp.importDef(), + new ImportExport.ImportPrivateModule(importedModuleName), + ImportExport.apply$default$3())); + } + return null; + }); + return null; + }); + + // Ensure that no symbols are exported from a private module. + if (isCurrentModulePrivate && containsExport(moduleIr)) { + exportErrors.add( + ImportExport.apply( + moduleIr.exports().apply(0), + new ImportExport.ExportSymbolsFromPrivateModule(moduleName), + ImportExport.apply$default$3())); + } + + // Ensure that private modules are not exported + bindingsMap + .getDirectlyExportedModules() + .foreach( + expModule -> { + var expModuleRef = expModule.module().module().unsafeAsModule("should succeed"); + if (expModuleRef.isPrivate() && !isSynthetic) { + var associatedExportIR = findExportIRByName(moduleIr, expModuleRef.getName()); + assert associatedExportIR.isDefined(); + exportErrors.add( + ImportExport.apply( + associatedExportIR.get(), + new ImportExport.ExportPrivateModule(expModuleRef.getName().toString()), + ImportExport.apply$default$3())); + } + return null; + }); + + scala.collection.immutable.List convertedImports = + importErrors.isEmpty() + ? moduleIr.imports() + : CollectionConverters.asScala(importErrors).toList(); + scala.collection.immutable.List convertedExports = + exportErrors.isEmpty() + ? moduleIr.exports() + : CollectionConverters.asScala(exportErrors).toList(); + + return moduleIr.copy( + convertedImports, + convertedExports, + moduleIr.copy$default$3(), + moduleIr.copy$default$4(), + moduleIr.copy$default$5(), + moduleIr.copy$default$6(), + moduleIr.copy$default$7(), + moduleIr.copy$default$8()); + } } /** Returns true iff the given Module's IR contains an export that is not synthetic. */ @@ -164,9 +202,4 @@ private static Option findExportIRByName(Module moduleIr, QualifiedName return null; }); } - - @Override - public T updateMetadataInDuplicate(T sourceIr, T copyOfIr) { - return IRPass.super.updateMetadataInDuplicate(sourceIr, copyOfIr); - } } From 7032b4a7915768ab5ee89b55c0fc5513a378a629 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 12:13:24 +0100 Subject: [PATCH 36/63] Fix spurious errors --- .../pass/analyse/PrivateModuleAnalysis.java | 17 +++++------------ .../org/enso/compiler/core/ir/Module.scala | 7 +++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java index f5cf1c040c25..c90a61d8a2e6 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java @@ -6,6 +6,7 @@ import org.enso.compiler.context.ModuleContext; import org.enso.compiler.core.IR; import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.MetadataStorage; import org.enso.compiler.core.ir.Module; import org.enso.compiler.core.ir.expression.errors.ImportExport; import org.enso.compiler.core.ir.module.scope.Export; @@ -117,7 +118,7 @@ public Module transformModule(Module moduleIr) { ImportExport.apply( resolvedImp.importDef(), new ImportExport.ImportPrivateModule(importedModuleName), - ImportExport.apply$default$3())); + MetadataStorage.EMPTY)); } return null; }); @@ -130,7 +131,7 @@ public Module transformModule(Module moduleIr) { ImportExport.apply( moduleIr.exports().apply(0), new ImportExport.ExportSymbolsFromPrivateModule(moduleName), - ImportExport.apply$default$3())); + MetadataStorage.EMPTY)); } // Ensure that private modules are not exported @@ -146,7 +147,7 @@ public Module transformModule(Module moduleIr) { ImportExport.apply( associatedExportIR.get(), new ImportExport.ExportPrivateModule(expModuleRef.getName().toString()), - ImportExport.apply$default$3())); + MetadataStorage.EMPTY)); } return null; }); @@ -160,15 +161,7 @@ public Module transformModule(Module moduleIr) { ? moduleIr.exports() : CollectionConverters.asScala(exportErrors).toList(); - return moduleIr.copy( - convertedImports, - convertedExports, - moduleIr.copy$default$3(), - moduleIr.copy$default$4(), - moduleIr.copy$default$5(), - moduleIr.copy$default$6(), - moduleIr.copy$default$7(), - moduleIr.copy$default$8()); + return moduleIr.copyWithImportsAndExports(convertedImports, convertedExports); } } diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala index b0e95f373572..3e22f842fb67 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala @@ -99,6 +99,13 @@ final case class Module( } else this } + def copyWithImportsAndExports( + imports: List[Import], + exports: List[Export] + ) = { + copy(imports, exports) + } + /** @inheritdoc */ override def duplicate( keepLocations: Boolean = true, From 303c9fe545c7968d57810e39499c40da357075f0 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 12:26:18 +0100 Subject: [PATCH 37/63] Convert PrivateConstructorAnalysis to mini pass --- .../analyse/PrivateConstructorAnalysis.java | 84 ++++++++++--------- .../org/enso/compiler/core/ir/Module.scala | 6 ++ 2 files changed, 51 insertions(+), 39 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateConstructorAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateConstructorAnalysis.java index 443f65d59492..7765ec628d2d 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateConstructorAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateConstructorAnalysis.java @@ -9,16 +9,19 @@ import org.enso.compiler.core.ir.expression.errors.Syntax; import org.enso.compiler.core.ir.expression.errors.Syntax.InconsistentConstructorVisibility$; import org.enso.compiler.core.ir.module.scope.Definition; -import org.enso.compiler.pass.IRPass; import org.enso.compiler.pass.IRProcessingPass; +import org.enso.compiler.pass.MiniIRPass; +import org.enso.compiler.pass.MiniPassFactory; import scala.collection.immutable.Seq; import scala.jdk.javaapi.CollectionConverters; /** * Ensures that all type definitions have either all constructors public, or all constructors * private. + * + *

Does not support inline compilation. */ -public final class PrivateConstructorAnalysis implements IRPass { +public final class PrivateConstructorAnalysis implements MiniPassFactory { public static final PrivateConstructorAnalysis INSTANCE = new PrivateConstructorAnalysis(); private PrivateConstructorAnalysis() {} @@ -36,47 +39,50 @@ public Seq invalidatedPasses() { return (scala.collection.immutable.List) obj; } - @Override - public Module runModule(Module ir, ModuleContext moduleContext) { - var newBindings = - ir.bindings() - .map( - binding -> { - if (binding instanceof Definition.Type type) { - var partitions = type.members().partition(Definition.Data::isPrivate); - var privateCtorsCnt = partitions._1.size(); - var publicCtorsCnt = partitions._2.size(); - var ctorsCnt = type.members().size(); - if (!(privateCtorsCnt == ctorsCnt || publicCtorsCnt == ctorsCnt)) { - assert type.location().isDefined(); - return new Syntax( - type.location().get(), - InconsistentConstructorVisibility$.MODULE$, - type.passData(), - type.diagnostics()); - } - } - return binding; - }); - return ir.copy( - ir.copy$default$1(), - ir.copy$default$2(), - newBindings, - ir.copy$default$4(), - ir.copy$default$5(), - ir.copy$default$6(), - ir.copy$default$7(), - ir.copy$default$8()); + public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { + return new Mini(); } - /** Not supported on a single expression. */ @Override - public Expression runExpression(Expression ir, InlineContext inlineContext) { - return ir; + public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { + return null; } - @Override - public T updateMetadataInDuplicate(T sourceIr, T copyOfIr) { - return IRPass.super.updateMetadataInDuplicate(sourceIr, copyOfIr); + private static final class Mini extends MiniIRPass { + @Override + public Expression transformExpression(Expression expr) { + throw new IllegalStateException("Should not be called - prepare returns null"); + } + + @Override + public MiniIRPass prepare(IR parent, Expression child) { + return null; + } + + @Override + public Module transformModule(Module moduleIr) { + var newBindings = + moduleIr + .bindings() + .map( + binding -> { + if (binding instanceof Definition.Type type) { + var partitions = type.members().partition(Definition.Data::isPrivate); + var privateCtorsCnt = partitions._1.size(); + var publicCtorsCnt = partitions._2.size(); + var ctorsCnt = type.members().size(); + if (!(privateCtorsCnt == ctorsCnt || publicCtorsCnt == ctorsCnt)) { + assert type.location().isDefined(); + return new Syntax( + type.location().get(), + InconsistentConstructorVisibility$.MODULE$, + type.passData(), + type.diagnostics()); + } + } + return binding; + }); + return moduleIr.copyWithBindings(newBindings); + } } } diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala index 3e22f842fb67..a0bd539fd773 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Module.scala @@ -106,6 +106,12 @@ final case class Module( copy(imports, exports) } + def copyWithBindings( + bindings: List[Definition] + ) = { + copy(bindings = bindings) + } + /** @inheritdoc */ override def duplicate( keepLocations: Boolean = true, From 557f35648919e6a30b26ae84cae45cf31a2fdbd3 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 15:52:44 +0100 Subject: [PATCH 38/63] FIx compilation error in a test --- .../compiler/test/pass/analyse/PrivateModifierTest.scala | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/PrivateModifierTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/PrivateModifierTest.scala index fb8af0bf9c76..f864f7f2ed06 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/PrivateModifierTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/PrivateModifierTest.scala @@ -30,15 +30,11 @@ class PrivateModifierTest extends CompilerTest { */ implicit class AnalyseModule(ir: Module) { - /** Runs alias analysis on a module. - * - * @return [[ir]], with attached aliasing information - */ def analyse: Module = { - PrivateConstructorAnalysis.INSTANCE.runModule( - ir, + val miniPass = PrivateConstructorAnalysis.INSTANCE.createForModuleCompilation( buildModuleContext(passConfiguration = Some(passConfig)) ) + miniPass.transformModule(ir) } } From 4e6837f9dad4b8baaf1140774126348c662c5388 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 16:45:24 +0100 Subject: [PATCH 39/63] Convert MethodDefinitions to Java --- .../enso/compiler/MetadataInteropHelpers.java | 7 + .../pass/analyse/PassPersistance.java | 4 +- .../pass/resolve/MethodDefinitions.java | 339 ++++++++++++++++++ .../main/scala/org/enso/compiler/Passes.scala | 2 +- .../org/enso/compiler/data/BindingsMap.scala | 2 +- .../pass/analyse/BindingAnalysis.scala | 2 +- .../pass/resolve/FullyQualifiedNames.scala | 5 +- .../compiler/pass/resolve/GlobalNames.scala | 5 +- .../pass/resolve/MethodDefinitions.scala | 281 --------------- .../enso/compiler/pass/resolve/Patterns.scala | 5 +- .../compiler/pass/resolve/TypeNames.scala | 5 +- .../org/enso/compiler/core/Implicits.scala | 15 + .../compiler/core/ir/DefinitionArgument.scala | 6 + .../org/enso/compiler/core/ir/Function.scala | 6 + .../org/enso/compiler/core/ir/Name.scala | 6 + .../suggestions/SuggestionBuilder.scala | 10 +- .../interpreter/runtime/IrToTruffle.scala | 69 ++-- 17 files changed, 445 insertions(+), 324 deletions(-) create mode 100644 engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java delete mode 100644 engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/MethodDefinitions.scala diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/MetadataInteropHelpers.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/MetadataInteropHelpers.java index 72f75289790b..74d2dbe808a7 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/MetadataInteropHelpers.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/MetadataInteropHelpers.java @@ -1,6 +1,7 @@ package org.enso.compiler; import org.enso.compiler.core.IR; +import org.enso.compiler.core.ir.MetadataStorage.MetadataPair; import org.enso.compiler.core.ir.ProcessingPass; import org.enso.compiler.pass.IRPass; import scala.Option; @@ -39,5 +40,11 @@ public static T getMetadata(IR ir, IRPass pass, Class expectedType) { return metadataOrNull; } + public static void updateMetadata( + IR ir, T pass, ProcessingPass.Metadata metadata) { + var metaPair = new MetadataPair<>(pass, metadata); + ir.passData().update(metaPair); + } + private MetadataInteropHelpers() {} } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java index 8d758ce72e57..7de99f29b66e 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java @@ -16,7 +16,7 @@ import org.enso.compiler.pass.resolve.IgnoredBindings; import org.enso.compiler.pass.resolve.IgnoredBindings$; import org.enso.compiler.pass.resolve.MethodCalls$; -import org.enso.compiler.pass.resolve.MethodDefinitions$; +import org.enso.compiler.pass.resolve.MethodDefinitions; import org.enso.compiler.pass.resolve.ModuleAnnotations; import org.enso.compiler.pass.resolve.ModuleAnnotations$; import org.enso.compiler.pass.resolve.Patterns$; @@ -54,7 +54,7 @@ @Persistable(clazz = ModuleAnnotations$.class, id = 1212) @Persistable(clazz = GatherDiagnostics$.class, id = 1213) @Persistable(clazz = MethodCalls$.class, id = 1214) -@Persistable(clazz = MethodDefinitions$.class, id = 1215) +@Persistable(clazz = MethodDefinitions.class, id = 1215) @Persistable(clazz = GenericAnnotations$.class, id = 1216) @Persistable(clazz = ExpressionAnnotations$.class, id = 1217) @Persistable(clazz = FullyQualifiedNames$.class, id = 1218) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java new file mode 100644 index 000000000000..37f8e334649a --- /dev/null +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java @@ -0,0 +1,339 @@ +package org.enso.compiler.pass.resolve; + +import java.util.ArrayList; +import org.enso.compiler.MetadataInteropHelpers; +import org.enso.compiler.context.InlineContext; +import org.enso.compiler.context.ModuleContext; +import org.enso.compiler.core.CompilerError; +import org.enso.compiler.core.IR; +import org.enso.compiler.core.ir.DefinitionArgument; +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.Function; +import org.enso.compiler.core.ir.MetadataStorage; +import org.enso.compiler.core.ir.Module; +import org.enso.compiler.core.ir.Name; +import org.enso.compiler.core.ir.expression.errors.Conversion; +import org.enso.compiler.core.ir.expression.errors.Conversion.UnsupportedSourceType$; +import org.enso.compiler.core.ir.module.scope.Definition; +import org.enso.compiler.core.ir.module.scope.definition.Method; +import org.enso.compiler.data.BindingsMap; +import org.enso.compiler.data.BindingsMap.Resolution; +import org.enso.compiler.data.BindingsMap.ResolvedConstructor; +import org.enso.compiler.data.BindingsMap.ResolvedConversionMethod; +import org.enso.compiler.data.BindingsMap.ResolvedExtensionMethod; +import org.enso.compiler.data.BindingsMap.ResolvedModule; +import org.enso.compiler.data.BindingsMap.ResolvedModuleMethod; +import org.enso.compiler.data.BindingsMap.ResolvedPolyglotField; +import org.enso.compiler.data.BindingsMap.ResolvedPolyglotSymbol; +import org.enso.compiler.data.BindingsMap.ResolvedType; +import org.enso.compiler.data.BindingsMap.Type; +import org.enso.compiler.pass.IRPass; +import org.enso.compiler.pass.IRProcessingPass; +import org.enso.compiler.pass.analyse.BindingAnalysis$; +import org.enso.compiler.pass.desugar.ComplexType$; +import org.enso.compiler.pass.desugar.FunctionBinding$; +import org.enso.compiler.pass.desugar.GenerateMethodBodies$; +import scala.Option; +import scala.collection.immutable.List; +import scala.collection.immutable.Seq; +import scala.jdk.javaapi.CollectionConverters; + +/** + * Resolves the correct {@code self} argument type for method definitions and stores the resolution + * in the method's metadata. + * + *

Metadata type is {@link BindingsMap.Resolution} + */ +public class MethodDefinitions implements IRPass { + public static final MethodDefinitions INSTANCE = new MethodDefinitions(); + + @Override + public Seq precursorPasses() { + java.util.List passes = + java.util.List.of( + ComplexType$.MODULE$, + FunctionBinding$.MODULE$, + GenerateMethodBodies$.MODULE$, + BindingAnalysis$.MODULE$); + return CollectionConverters.asScala(passes).toList(); + } + + @Override + @SuppressWarnings("unchecked") + public Seq invalidatedPasses() { + Object obj = scala.collection.immutable.Nil$.MODULE$; + return (scala.collection.immutable.List) obj; + } + + @Override + public Module runModule(Module ir, ModuleContext moduleContext) { + BindingsMap availableSymbolsMap = + MetadataInteropHelpers.getMetadata(ir, BindingAnalysis$.MODULE$, BindingsMap.class); + var newDefs = + ir.bindings() + .map( + def -> { + if (def instanceof Method method) { + var methodRef = method.methodReference(); + Option resolvedTypeRef = + methodRef + .typePointer() + .map(tp -> resolveType(tp, availableSymbolsMap)) + .orElse(null); + var resolvedMethodRef = methodRef.copyWithTypePointer(resolvedTypeRef); + + return switch (method) { + case Method.Explicit explicitMethod -> { + var isStatic = computeIsStatic(explicitMethod.body()); + var resolvedMethod = + explicitMethod.copy( + resolvedMethodRef, + explicitMethod.body(), + isStatic, + explicitMethod.isPrivate(), + explicitMethod.isStaticWrapperForInstanceMethod(), + explicitMethod.location(), + explicitMethod.passData(), + explicitMethod.diagnostics(), + explicitMethod.id()); + yield resolvedMethod; + } + case Method.Conversion conversionMethod -> { + var sourceTypeExpr = conversionMethod.sourceTypeName(); + Name resolvedName = + switch (sourceTypeExpr) { + case Name name -> resolveType(name, availableSymbolsMap); + default -> new Conversion( + sourceTypeExpr, + UnsupportedSourceType$.MODULE$, + MetadataStorage.EMPTY); + }; + var resolvedMethod = + conversionMethod.copy( + resolvedMethodRef, + resolvedName, + conversionMethod.body(), + conversionMethod.location(), + conversionMethod.passData(), + conversionMethod.diagnostics(), + conversionMethod.id()); + yield resolvedMethod; + } + default -> throw new CompilerError( + "Unexpected method type in MethodDefinitions pass."); + }; + } else { + return def; + } + }); + + java.util.List withStaticAliases = new ArrayList<>(); + for (var def : CollectionConverters.asJava(newDefs)) { + withStaticAliases.add(def); + if (def instanceof Method.Explicit method && !method.isStatic()) { + var staticAlias = generateStaticAliasMethod(method); + if (staticAlias != null) { + withStaticAliases.add(staticAlias); + } + } + } + + return ir.copyWithBindings(CollectionConverters.asScala(withStaticAliases).toList()); + } + + /** + * Returns null if there is no suitable static alias method that can be generated for the given + * {@code method}. + * + * @param method Non-static method from which a static alias method is generated. + * @return Static alias method for the given {@code method} or null. + */ + private Method.Explicit generateStaticAliasMethod(Method.Explicit method) { + assert !method.isStatic(); + var typePointer = method.methodReference().typePointer(); + if (typePointer.isEmpty()) { + return null; + } + var resolution = + MetadataInteropHelpers.getMetadataOrNull(typePointer.get(), this, Resolution.class); + if (resolution == null) { + return null; + } + if (resolution.target() instanceof ResolvedType resType + && canGenerateStaticWrappers(resType.tp())) { + assert method.body() instanceof Function.Lambda; + var dup = method.duplicate(true, true, true, false); + // This is the self argument that will receive the `SelfType.type` value upon dispatch, it is + // added to avoid modifying the dispatch mechanism. + var syntheticModuleSelfArg = + new DefinitionArgument.Specified( + new Name.Self(null, true, MetadataStorage.EMPTY), + Option.empty(), + Option.empty(), + false, + null, + MetadataStorage.EMPTY); + var newBody = + new Function.Lambda( + // This is the synthetic Self argument that gets the static module + list(syntheticModuleSelfArg), + // Here we add the type ascription ensuring that the 'proper' self argument only + // accepts _instances_ of the type (or triggers conversions) + addTypeAscriptionToSelfArgument(dup.body()), + null, + true, + MetadataStorage.EMPTY); + // The actual `self` argument that is referenced inside of method body is the second one in + // the lambda. + // This is the argument that will hold the actual instance of the object we are calling on, + // e.g. `My_Type.method instance`. + // We add a type check to it to ensure only `instance` of `My_Type` can be passed to it. + var staticMethod = + dup.copy( + method.methodReference(), + newBody, + true, + method.isPrivate(), + true, + method.location(), + method.passData(), + method.diagnostics(), + method.id()); + return staticMethod; + } + return null; + } + + private static Expression addTypeAscriptionToSelfArgument(Expression methodBody) { + if (methodBody instanceof Function.Lambda lambda) { + if (lambda.arguments().isEmpty()) { + throw new CompilerError( + "MethodDefinitions pass: expected at least one argument (self) in the method, but got" + + " none."); + } + var firstArg = lambda.arguments().head(); + if (firstArg instanceof DefinitionArgument.Specified selfArg + && selfArg.name() instanceof Name.Self) { + var selfType = new Name.SelfType(selfArg.identifiedLocation(), MetadataStorage.EMPTY); + var newSelfArg = selfArg.copyWithAscribedType(selfType); + return lambdaWithNewSelfArg(lambda, newSelfArg); + } else { + throw new CompilerError( + "MethodDefinitions pass: expected the first argument to be `self`, but got " + + firstArg); + } + } else { + throw new CompilerError("Unexpected body type " + methodBody + " in MethodDefinitions pass."); + } + } + + private static Function.Lambda lambdaWithNewSelfArg( + Function.Lambda lambda, DefinitionArgument newSelfArg) { + var args = CollectionConverters.asJava(lambda.arguments()); + assert !args.isEmpty(); + args.set(0, newSelfArg); + var newArgs = CollectionConverters.asScala(args).toList(); + return lambda.copyWithArguments(newArgs); + } + + // Generate static wrappers for + // 1. Types having at least one type constructor + // 2. All builtin types except for Nothing. Nothing's eigentype is Nothing and not Nothing.type, + // would lead to overriding conflicts. + // TODO: Remove the hardcoded type once Enso's annotations can define parameters. + private static boolean canGenerateStaticWrappers(Type tp) { + return tp.members().nonEmpty() || (tp.builtinType() && !"Nothing".equals(tp.name())); + } + + private Name resolveType(Name typePointer, BindingsMap availableSymbolsMap) { + if (typePointer instanceof Name.Qualified || typePointer instanceof Name.Literal) { + var items = + switch (typePointer) { + case Name.Qualified qualName -> qualName.parts().map(Name::name); + case Name.Literal lit -> list(lit.name()); + default -> throw new CompilerError("Impossible to reach"); + }; + + var resolvedItemsOpt = availableSymbolsMap.resolveQualifiedName(items); + if (resolvedItemsOpt.isLeft()) { + var err = resolvedItemsOpt.swap().toOption().get(); + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.ResolverError(err), + MetadataStorage.EMPTY); + } + var resolvedItems = resolvedItemsOpt.toOption().get(); + assert resolvedItems.size() == 1 : "Expected a single resolution"; + switch (resolvedItems.head()) { + case ResolvedConstructor ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedConstructor( + "a method definition target"), + MetadataStorage.EMPTY); + } + case ResolvedModule resMod -> { + MetadataInteropHelpers.updateMetadata(typePointer, this, new Resolution(resMod)); + return typePointer; + } + case ResolvedType resType -> { + MetadataInteropHelpers.updateMetadata(typePointer, this, new Resolution(resType)); + return typePointer; + } + case ResolvedPolyglotSymbol ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedPolyglot( + "a method definition target"), + MetadataStorage.EMPTY); + } + case ResolvedPolyglotField ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedPolyglot( + "a method definition target"), + MetadataStorage.EMPTY); + } + case ResolvedModuleMethod ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( + "a method definition target"), + MetadataStorage.EMPTY); + } + case ResolvedExtensionMethod ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( + "a static method definition target"), + MetadataStorage.EMPTY); + } + case ResolvedConversionMethod ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( + "a conversion method definition target"), + MetadataStorage.EMPTY); + } + default -> throw new IllegalStateException("Unexpected value: " + resolvedItems.head()); + } + } else if (typePointer instanceof org.enso.compiler.core.ir.expression.errors.Resolution) { + return typePointer; + } else { + throw new CompilerError("Unexpected kind of name for method reference"); + } + } + + private static List list(T item) { + return CollectionConverters.asScala(java.util.List.of(item)).toList(); + } + + @Override + public Expression runExpression(Expression ir, InlineContext inlineContext) { + return ir; + } + + private static boolean computeIsStatic(IR body) { + return Method.Explicit$.MODULE$.computeIsStatic(body); + } +} diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala index 13b99b7e480a..d9f75cba68eb 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Passes.scala @@ -40,7 +40,7 @@ class Passes(config: CompilerConfig) { val globalTypingPasses = new PassGroup( List( - MethodDefinitions, + MethodDefinitions.INSTANCE, SectionsToBinOp.INSTANCE, OperatorToFunction, LambdaShorthandToLambda, diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/data/BindingsMap.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/data/BindingsMap.scala index dbac0802ec34..9c8753aad043 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/data/BindingsMap.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/data/BindingsMap.scala @@ -875,7 +875,7 @@ object BindingsMap { case method: ir.module.scope.definition.Method.Explicit => method.methodReference.methodName.name == this.method.name && method.methodReference.typePointer .forall( - _.getMetadata(MethodDefinitions) + _.getMetadata(MethodDefinitions.INSTANCE) .contains(Resolution(ResolvedModule(module))) ) case _ => false diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/BindingAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/BindingAnalysis.scala index a49fab7d2a30..e58b744058ec 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/BindingAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/BindingAnalysis.scala @@ -41,7 +41,7 @@ case object BindingAnalysis extends IRPass { /** The passes that are invalidated by running this pass. */ override lazy val invalidatedPasses: Seq[IRPass] = - Seq(MethodDefinitions, Patterns) + Seq(MethodDefinitions.INSTANCE, Patterns) /** Executes the pass on the provided `ir`, and returns a possibly transformed * or annotated version of `ir`. diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/FullyQualifiedNames.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/FullyQualifiedNames.scala index 39543d6e4fd5..fb1bf1f0bcbe 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/FullyQualifiedNames.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/FullyQualifiedNames.scala @@ -181,7 +181,10 @@ case object FullyQualifiedNames extends IRPass { case asc: Type.Ascription => asc case method: definition.Method => val resolution = method.methodReference.typePointer.flatMap( - _.getMetadata(MethodDefinitions) + _.getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) ) method.mapExpressions( processExpression( diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/GlobalNames.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/GlobalNames.scala index 706709209c4d..e95ea3f79e8b 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/GlobalNames.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/GlobalNames.scala @@ -115,7 +115,10 @@ case object GlobalNames extends IRPass { case asc: Type.Ascription => asc case method: definition.Method => val resolution = method.methodReference.typePointer.flatMap( - _.getMetadata(MethodDefinitions) + _.getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) ) method.mapExpressions( processExpression(_, bindings, List(), freshNameSupply, resolution) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/MethodDefinitions.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/MethodDefinitions.scala deleted file mode 100644 index 7537cab6f2c3..000000000000 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/MethodDefinitions.scala +++ /dev/null @@ -1,281 +0,0 @@ -package org.enso.compiler.pass.resolve - -import org.enso.compiler.context.{InlineContext, ModuleContext} -import org.enso.compiler.core.Implicits.AsMetadata -import org.enso.compiler.core.ir.{ - DefinitionArgument, - Expression, - Function, - Module, - Name -} -import org.enso.compiler.core.ir.expression.errors -import org.enso.compiler.core.ir.module.scope.definition -import org.enso.compiler.core.ir.MetadataStorage.MetadataPair -import org.enso.compiler.data.BindingsMap -import org.enso.compiler.data.BindingsMap.{Resolution, ResolvedType, Type} -import org.enso.compiler.core.CompilerError -import org.enso.compiler.pass.IRPass -import org.enso.compiler.pass.analyse.BindingAnalysis -import org.enso.compiler.pass.desugar.{ - ComplexType, - FunctionBinding, - GenerateMethodBodies -} - -/** Resolves the correct `self` argument type for method definitions and stores - * the resolution in the method's metadata. - */ -case object MethodDefinitions extends IRPass { - - override type Metadata = BindingsMap.Resolution - - override type Config = IRPass.Configuration.Default - - override lazy val precursorPasses: Seq[IRPass] = - List(ComplexType, FunctionBinding, GenerateMethodBodies, BindingAnalysis) - - override lazy val invalidatedPasses: Seq[IRPass] = List() - - /** Executes the pass on the provided `ir`, and returns a possibly transformed - * or annotated version of `ir`. - * - * @param ir the Enso IR to process - * @param moduleContext a context object that contains the information needed - * to process a module - * @return `ir`, possibly having made transformations or annotations to that - * IR. - */ - override def runModule( - ir: Module, - moduleContext: ModuleContext - ): Module = { - val availableSymbolsMap = ir.unsafeGetMetadata( - BindingAnalysis, - "MethodDefinitionResolution is being run before BindingResolution" - ) - val newDefs = ir.bindings.map { - case method: definition.Method => - val methodRef = method.methodReference - val resolvedTypeRef = - methodRef.typePointer.map(resolveType(_, availableSymbolsMap)) - val resolvedMethodRef = methodRef.copy(typePointer = resolvedTypeRef) - - method match { - case method: definition.Method.Explicit => - val resolvedMethod = - method.copy( - methodReference = resolvedMethodRef, - isStatic = - definition.Method.Explicit.computeIsStatic(method.body) - ) - resolvedMethod - case method: definition.Method.Conversion => - val sourceTypeExpr = method.sourceTypeName - - val resolvedName: Name = sourceTypeExpr match { - case name: Name => resolveType(name, availableSymbolsMap) - case _ => - errors.Conversion( - sourceTypeExpr, - errors.Conversion.UnsupportedSourceType - ) - } - - val resolvedMethod = method.copy( - methodReference = resolvedMethodRef, - sourceTypeName = resolvedName - ) - resolvedMethod - - case _ => - throw new CompilerError( - "Unexpected method type in MethodDefinitions pass." - ) - } - case other => other - } - - val withStaticAliases = newDefs.flatMap { - case method: definition.Method.Explicit if !method.isStatic => - method.methodReference.typePointer.flatMap( - _.getMetadata(this) - ) match { - case Some(Resolution(ResolvedType(_, tp))) - if canGenerateStaticWrappers(tp) => - org.enso.common.Asserts - .assertInJvm(method.body.isInstanceOf[Function.Lambda]) - val dup = method.duplicate() - // This is the self argument that will receive the `SelfType.type` value upon dispatch, it is added to avoid modifying the dispatch mechanism. - val syntheticModuleSelfArg = DefinitionArgument.Specified( - Name.Self(identifiedLocation = null, synthetic = true), - None, - None, - suspended = false, - identifiedLocation = null - ) - - // The actual `self` argument that is referenced inside of method body is the second one in the lambda. - // This is the argument that will hold the actual instance of the object we are calling on, e.g. `My_Type.method instance`. - // We add a type check to it to ensure only `instance` of `My_Type` can be passed to it. - val static = dup.copy( - body = new Function.Lambda( - // This is the synthetic Self argument that gets the static module - List(syntheticModuleSelfArg), - // Here we add the type ascription ensuring that the 'proper' self argument only accepts _instances_ of the type (or triggers conversions) - addTypeAscriptionToSelfArgument(dup.body), - identifiedLocation = null - ), - isStaticWrapperForInstanceMethod = true, - isStatic = true - ) - List(method, static) - case _ => - List(method) - } - - case other => List(other) - } - - ir.copy(bindings = withStaticAliases) - } - - private def addTypeAscriptionToSelfArgument( - methodBody: Expression - ): Expression = methodBody match { - case lambda: Function.Lambda => - val newArguments = lambda.arguments match { - case (selfArg @ DefinitionArgument.Specified( - Name.Self(_, _, _), - _, - _, - _, - _, - _ - )) :: rest => - val selfType = - Name.SelfType(identifiedLocation = selfArg.identifiedLocation) - selfArg.copy(ascribedType = Some(selfType)) :: rest - case other :: _ => - throw new CompilerError( - s"MethodDefinitions pass: expected the first argument to be `self`, but got $other" - ) - case Nil => - throw new CompilerError( - s"MethodDefinitions pass: expected at least one argument (self) in the method, but got none." - ) - } - lambda.copy(arguments = newArguments) - case other => - throw new CompilerError( - s"Unexpected body type $other in MethodDefinitions pass." - ) - } - - // Generate static wrappers for - // 1. Types having at least one type constructor - // 2. All builtin types except for Nothing. Nothing's eigentype is Nothing and not Nothing.type, - // would lead to overriding conflicts. - // TODO: Remove the hardcoded type once Enso's annotations can define parameters. - private def canGenerateStaticWrappers(tp: Type): Boolean = - tp.members.nonEmpty || (tp.builtinType && (tp.name != "Nothing")) - - private def resolveType( - typePointer: Name, - availableSymbolsMap: BindingsMap - ): Name = { - typePointer match { - case _: Name.Qualified | _: Name.Literal => - val items = typePointer match { - case qualName: Name.Qualified => qualName.parts.map(_.name) - case literal: Name.Literal => List(literal.name) - case _ => - throw new CompilerError("Impossible to reach.") - } - availableSymbolsMap.resolveQualifiedName(items) match { - case Left(err) => - errors.Resolution( - typePointer, - errors.Resolution.ResolverError(err) - ) - case Right(resolvedItems) => - org.enso.common.Asserts.assertInJvm( - resolvedItems.size == 1, - "Expected a single resolution" - ) - resolvedItems.head match { - case _: BindingsMap.ResolvedConstructor => - errors.Resolution( - typePointer, - errors.Resolution.UnexpectedConstructor( - "a method definition target" - ) - ) - case value: BindingsMap.ResolvedModule => - typePointer.updateMetadata( - new MetadataPair(this, BindingsMap.Resolution(value)) - ) - case value: BindingsMap.ResolvedType => - typePointer.updateMetadata( - new MetadataPair(this, BindingsMap.Resolution(value)) - ) - case _: BindingsMap.ResolvedPolyglotSymbol => - errors.Resolution( - typePointer, - errors.Resolution.UnexpectedPolyglot( - "a method definition target" - ) - ) - case _: BindingsMap.ResolvedPolyglotField => - errors.Resolution( - typePointer, - errors.Resolution.UnexpectedPolyglot( - "a method definition target" - ) - ) - case _: BindingsMap.ResolvedModuleMethod => - errors.Resolution( - typePointer, - errors.Resolution.UnexpectedMethod( - "a method definition target" - ) - ) - case _: BindingsMap.ResolvedExtensionMethod => - errors.Resolution( - typePointer, - errors.Resolution.UnexpectedMethod( - "a static method definition target" - ) - ) - case _: BindingsMap.ResolvedConversionMethod => - errors.Resolution( - typePointer, - errors.Resolution.UnexpectedMethod( - "a conversion method definition target" - ) - ) - } - } - case tp: errors.Resolution => tp - case _ => - throw new CompilerError( - "Unexpected kind of name for method reference" - ) - } - } - - /** Executes the pass on the provided `ir`, and returns a possibly transformed - * or annotated version of `ir` in an inline context. - * - * @param ir the Enso IR to process - * @param inlineContext a context object that contains the information needed - * for inline evaluation - * @return `ir`, possibly having made transformations or annotations to that - * IR. - */ - override def runExpression( - ir: Expression, - inlineContext: InlineContext - ): Expression = ir - -} diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/Patterns.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/Patterns.scala index e2914b67881a..840c93d69beb 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/Patterns.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/Patterns.scala @@ -69,7 +69,10 @@ object Patterns extends IRPass { case method: definition.Method.Explicit => val resolution = method.methodReference.typePointer .flatMap( - _.getMetadata(MethodDefinitions) + _.getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) ) .map(_.target) val newBody = doExpression(method.body, bindings, resolution) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/TypeNames.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/TypeNames.scala index 0b675f973b82..04aaa046d574 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/TypeNames.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/TypeNames.scala @@ -96,7 +96,10 @@ case object TypeNames extends IRPass { def fromMethodReference(m: Name.MethodReference): SelfTypeInfo = m.typePointer match { case Some(p) => - p.getMetadata(MethodDefinitions) match { + p.getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) match { case Some(resolution) => resolution.target match { case typ: BindingsMap.ResolvedType => diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/Implicits.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/Implicits.scala index 702c835d7619..827fe4540820 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/Implicits.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/Implicits.scala @@ -3,6 +3,8 @@ package org.enso.compiler.core import org.enso.compiler.core.ir.MetadataStorage.MetadataPair import org.enso.compiler.core.ir.{Diagnostic, ProcessingPass} +import scala.annotation.unused + object Implicits { /** This class adds an extension method to control how the pass data element @@ -91,6 +93,19 @@ object Implicits { ir.passData.get(pass).asInstanceOf[Option[pass.Metadata]] } + /** Getting metadata from passes that are implemented in Java and thus have no + * overriden type alias for the metadata types. + * @return + */ + def getMetadata[MetaType <: ProcessingPass.Metadata]( + pass: ProcessingPass, + @unused + expectedMetaType: Class[MetaType] + ): Option[MetaType] = { + val meta = ir.passData.get(pass) + meta.asInstanceOf[Option[MetaType]] + } + /** Unsafely gets the metadata for the specified pass, if it exists. * * @param pass the pass to get metadata for diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/DefinitionArgument.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/DefinitionArgument.scala index fd820cd62a72..765dd45d52ff 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/DefinitionArgument.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/DefinitionArgument.scala @@ -141,6 +141,12 @@ object DefinitionArgument { } else this } + def copyWithAscribedType( + ascribedType: Expression + ): Specified = { + copy(ascribedType = Some(ascribedType)) + } + override def withName(ir: Name): DefinitionArgument = copy(name = ir) /** @inheritdoc */ diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Function.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Function.scala index 021e0c1b57cb..826c2fc2faba 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Function.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Function.scala @@ -135,6 +135,12 @@ object Function { } else this } + def copyWithArguments( + arguments: List[DefinitionArgument] + ): Lambda = { + copy(arguments = arguments) + } + /** @inheritdoc */ override def duplicate( keepLocations: Boolean = true, diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Name.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Name.scala index c4be29a77458..6509ae2179ee 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Name.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Name.scala @@ -93,6 +93,12 @@ object Name { } else this } + def copyWithTypePointer( + typePointer: Option[Name] + ) = { + copy(typePointer = typePointer) + } + /** @inheritdoc */ override def duplicate( keepLocations: Boolean = true, diff --git a/engine/runtime-suggestions/src/main/scala/org/enso/compiler/suggestions/SuggestionBuilder.scala b/engine/runtime-suggestions/src/main/scala/org/enso/compiler/suggestions/SuggestionBuilder.scala index 06c71db33068..a9b4758eab8b 100644 --- a/engine/runtime-suggestions/src/main/scala/org/enso/compiler/suggestions/SuggestionBuilder.scala +++ b/engine/runtime-suggestions/src/main/scala/org/enso/compiler/suggestions/SuggestionBuilder.scala @@ -131,7 +131,10 @@ final class SuggestionBuilder[A: IndexedSource]( val (selfTypeOpt, isStatic) = typePtr match { case Some(typePtr) => val selfType = typePtr - .getMetadata(MethodDefinitions) + .getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) .map(_.target.qualifiedName) selfType -> m.isStatic case None => @@ -167,7 +170,10 @@ final class SuggestionBuilder[A: IndexedSource]( ) if !conversionMeth.isPrivate => val selfType = typePtr.flatMap { typePointer => typePointer - .getMetadata(MethodDefinitions) + .getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) .map(_.target.qualifiedName) } val conversion = buildConversion( diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala b/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala index d071f6e07c94..93054d770922 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala @@ -724,7 +724,10 @@ class IrToTruffle( Some(scopeAssociatedType) case Some(tpePointer) => tpePointer - .getMetadata(MethodDefinitions) + .getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) .map { res => res.target match { case binding @ BindingsMap.ResolvedType(_, _) => @@ -945,38 +948,40 @@ class IrToTruffle( } private def getTypeResolution(expr: IR): Option[Type] = - expr.getMetadata(MethodDefinitions).map { res => - res.target match { - case binding @ BindingsMap.ResolvedType(_, _) => - asType(binding) - case BindingsMap.ResolvedModule(module) => - asAssociatedType(module.unsafeAsModule()) - case BindingsMap.ResolvedConstructor(_, _) => - throw new CompilerError( - "Impossible here, should be caught by MethodDefinitions pass." - ) - case BindingsMap.ResolvedPolyglotSymbol(_, _) => - throw new CompilerError( - "Impossible polyglot symbol, should be caught by MethodDefinitions pass." - ) - case BindingsMap.ResolvedPolyglotField(_, _) => - throw new CompilerError( - "Impossible polyglot field, should be caught by MethodDefinitions pass." - ) - case _: BindingsMap.ResolvedModuleMethod => - throw new CompilerError( - "Impossible module method here, should be caught by MethodDefinitions pass." - ) - case _: BindingsMap.ResolvedExtensionMethod => - throw new CompilerError( - "Impossible static method here, should be caught by MethodDefinitions pass." - ) - case _: BindingsMap.ResolvedConversionMethod => - throw new CompilerError( - "Impossible conversion method here, should be caught by MethodDefinitions pass." - ) + expr + .getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) + .map { res => + res.target match { + case binding @ BindingsMap.ResolvedType(_, _) => + asType(binding) + case BindingsMap.ResolvedModule(module) => + asAssociatedType(module.unsafeAsModule()) + case BindingsMap.ResolvedConstructor(_, _) => + throw new CompilerError( + "Impossible here, should be caught by MethodDefinitions pass." + ) + case BindingsMap.ResolvedPolyglotSymbol(_, _) => + throw new CompilerError( + "Impossible polyglot symbol, should be caught by MethodDefinitions pass." + ) + case BindingsMap.ResolvedPolyglotField(_, _) => + throw new CompilerError( + "Impossible polyglot field, should be caught by MethodDefinitions pass." + ) + case _: BindingsMap.ResolvedModuleMethod => + throw new CompilerError( + "Impossible module method here, should be caught by MethodDefinitions pass." + ) + case _: BindingsMap.ResolvedExtensionMethod => + throw new CompilerError( + "Impossible static method here, should be caught by MethodDefinitions pass." + ) + case _: BindingsMap.ResolvedConversionMethod => + throw new CompilerError( + "Impossible conversion method here, should be caught by MethodDefinitions pass." + ) + } } - } private def getTailStatus( expression: Expression From 0bb6b5e6d84f42322455557e08c33808211153d8 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 16:45:31 +0100 Subject: [PATCH 40/63] FIx compilation error in a test --- .../compiler/test/pass/analyse/PrivateModifierTest.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/PrivateModifierTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/PrivateModifierTest.scala index f864f7f2ed06..6ee32f8c2772 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/PrivateModifierTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/PrivateModifierTest.scala @@ -31,9 +31,10 @@ class PrivateModifierTest extends CompilerTest { implicit class AnalyseModule(ir: Module) { def analyse: Module = { - val miniPass = PrivateConstructorAnalysis.INSTANCE.createForModuleCompilation( - buildModuleContext(passConfiguration = Some(passConfig)) - ) + val miniPass = + PrivateConstructorAnalysis.INSTANCE.createForModuleCompilation( + buildModuleContext(passConfiguration = Some(passConfig)) + ) miniPass.transformModule(ir) } } From 866ffd9f6a2bfbae529b28d41864828675c7e0e9 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 16:55:57 +0100 Subject: [PATCH 41/63] Fix NPE --- .../java/org/enso/compiler/pass/resolve/MethodDefinitions.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java index 37f8e334649a..ef6ea259c97f 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java @@ -78,8 +78,7 @@ public Module runModule(Module ir, ModuleContext moduleContext) { Option resolvedTypeRef = methodRef .typePointer() - .map(tp -> resolveType(tp, availableSymbolsMap)) - .orElse(null); + .map(tp -> resolveType(tp, availableSymbolsMap)); var resolvedMethodRef = methodRef.copyWithTypePointer(resolvedTypeRef); return switch (method) { From 1166f024a87fc645c7c13f0189e6cdd6c462665a Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 16:56:14 +0100 Subject: [PATCH 42/63] FIx compilation error in a test --- .../org/enso/compiler/test/PassesTest.scala | 2 +- .../pass/resolve/MethodDefinitionsTest.scala | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala index d141d3dcb702..4f117401b4d7 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/PassesTest.scala @@ -60,7 +60,7 @@ class PassesTest extends CompilerTest { GenerateMethodBodies, BindingAnalysis, ModuleNameConflicts, - MethodDefinitions, + MethodDefinitions.INSTANCE, SectionsToBinOp.INSTANCE, OperatorToFunction, LambdaShorthandToLambda, diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala index aefe1db7f663..2db7ad29c79e 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala @@ -24,7 +24,7 @@ class MethodDefinitionsTest extends CompilerTest { val passes = new Passes(defaultConfig) val precursorPasses: PassGroup = - passes.getPrecursors(MethodDefinitions).get + passes.getPrecursors(MethodDefinitions.INSTANCE).get val passConfiguration: PassConfiguration = PassConfiguration() @@ -43,7 +43,7 @@ class MethodDefinitionsTest extends CompilerTest { * @return [[ir]], with tail call analysis metadata attached */ def analyse(implicit context: ModuleContext): Module = { - MethodDefinitions.runModule(ir, context) + MethodDefinitions.INSTANCE.runModule(ir, context) } } @@ -78,7 +78,7 @@ class MethodDefinitionsTest extends CompilerTest { .methodReference .typePointer .get - .getMetadata(MethodDefinitions) shouldEqual Some( + .getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedType( ctx.moduleReference(), @@ -96,7 +96,7 @@ class MethodDefinitionsTest extends CompilerTest { .methodReference .typePointer .get - .getMetadata(MethodDefinitions) shouldEqual Some( + .getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedModule(ctx.moduleReference()) ) @@ -112,7 +112,8 @@ class MethodDefinitionsTest extends CompilerTest { .bindings(6) .asInstanceOf[definition.Method.Conversion] conv1.methodReference.typePointer.get.getMetadata( - MethodDefinitions + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] ) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedType( @@ -121,7 +122,7 @@ class MethodDefinitionsTest extends CompilerTest { ) ) ) - conv1.sourceTypeName.getMetadata(MethodDefinitions) shouldEqual Some( + conv1.sourceTypeName.getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedType( ctx.moduleReference(), @@ -134,7 +135,8 @@ class MethodDefinitionsTest extends CompilerTest { .bindings(7) .asInstanceOf[definition.Method.Conversion] conv2.methodReference.typePointer.get.getMetadata( - MethodDefinitions + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] ) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedType( @@ -149,7 +151,7 @@ class MethodDefinitionsTest extends CompilerTest { .bindings(8) .asInstanceOf[definition.Method.Conversion] conv3.methodReference.typePointer.get shouldBe an[errors.Resolution] - conv3.sourceTypeName.getMetadata(MethodDefinitions) shouldEqual Some( + conv3.sourceTypeName.getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedType( ctx.moduleReference(), From 8d9db8fe01b70b2992103c446cac31bc266c10ff Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 18:09:40 +0100 Subject: [PATCH 43/63] MetadataStorage.EMPTY is not a global reference --- .../analyse/AmbiguousImportsAnalysis.java | 2 +- .../pass/analyse/ImportSymbolAnalysis.java | 4 ++-- .../pass/analyse/PrivateModuleAnalysis.java | 6 ++--- .../pass/resolve/MethodDefinitions.java | 24 +++++++++---------- .../compiler/core/ir/MetadataStorage.java | 1 - 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java index 8eb987d39b4f..2e13c4872a88 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java @@ -287,7 +287,7 @@ private ImportExport createErrorForAmbiguousImport( ambiguousSymbol, ambiguousSymbolPath ), - MetadataStorage.EMPTY + new MetadataStorage() ); } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java index 78a895e4fa5e..0654936c6df2 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -196,7 +196,7 @@ private static ImportExport createErrorForUnresolvedSymbol( convMethod.conversionMethod().sourceTpName()); default -> throw new IllegalStateException("Unexpected value: " + importTarget); }; - return new ImportExport(imp, errorReason, MetadataStorage.EMPTY); + return new ImportExport(imp, errorReason, new MetadataStorage()); } private static ImportExport createImportFromMethodError( @@ -204,7 +204,7 @@ private static ImportExport createImportFromMethodError( return new ImportExport( imp, new ImportExport.IllegalImportFromMethod(moduleName, methodName), - MetadataStorage.EMPTY); + new MetadataStorage()); } } } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java index c90a61d8a2e6..ed7ac9885295 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java @@ -118,7 +118,7 @@ public Module transformModule(Module moduleIr) { ImportExport.apply( resolvedImp.importDef(), new ImportExport.ImportPrivateModule(importedModuleName), - MetadataStorage.EMPTY)); + new MetadataStorage())); } return null; }); @@ -131,7 +131,7 @@ public Module transformModule(Module moduleIr) { ImportExport.apply( moduleIr.exports().apply(0), new ImportExport.ExportSymbolsFromPrivateModule(moduleName), - MetadataStorage.EMPTY)); + new MetadataStorage())); } // Ensure that private modules are not exported @@ -147,7 +147,7 @@ public Module transformModule(Module moduleIr) { ImportExport.apply( associatedExportIR.get(), new ImportExport.ExportPrivateModule(expModuleRef.getName().toString()), - MetadataStorage.EMPTY)); + new MetadataStorage())); } return null; }); diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java index ef6ea259c97f..80123a7eb797 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java @@ -105,7 +105,7 @@ public Module runModule(Module ir, ModuleContext moduleContext) { default -> new Conversion( sourceTypeExpr, UnsupportedSourceType$.MODULE$, - MetadataStorage.EMPTY); + new MetadataStorage()); }; var resolvedMethod = conversionMethod.copy( @@ -166,12 +166,12 @@ && canGenerateStaticWrappers(resType.tp())) { // added to avoid modifying the dispatch mechanism. var syntheticModuleSelfArg = new DefinitionArgument.Specified( - new Name.Self(null, true, MetadataStorage.EMPTY), + new Name.Self(null, true, new MetadataStorage()), Option.empty(), Option.empty(), false, null, - MetadataStorage.EMPTY); + new MetadataStorage()); var newBody = new Function.Lambda( // This is the synthetic Self argument that gets the static module @@ -181,7 +181,7 @@ && canGenerateStaticWrappers(resType.tp())) { addTypeAscriptionToSelfArgument(dup.body()), null, true, - MetadataStorage.EMPTY); + new MetadataStorage()); // The actual `self` argument that is referenced inside of method body is the second one in // the lambda. // This is the argument that will hold the actual instance of the object we are calling on, @@ -213,7 +213,7 @@ private static Expression addTypeAscriptionToSelfArgument(Expression methodBody) var firstArg = lambda.arguments().head(); if (firstArg instanceof DefinitionArgument.Specified selfArg && selfArg.name() instanceof Name.Self) { - var selfType = new Name.SelfType(selfArg.identifiedLocation(), MetadataStorage.EMPTY); + var selfType = new Name.SelfType(selfArg.identifiedLocation(), new MetadataStorage()); var newSelfArg = selfArg.copyWithAscribedType(selfType); return lambdaWithNewSelfArg(lambda, newSelfArg); } else { @@ -259,7 +259,7 @@ private Name resolveType(Name typePointer, BindingsMap availableSymbolsMap) { return new org.enso.compiler.core.ir.expression.errors.Resolution( typePointer, new org.enso.compiler.core.ir.expression.errors.Resolution.ResolverError(err), - MetadataStorage.EMPTY); + new MetadataStorage()); } var resolvedItems = resolvedItemsOpt.toOption().get(); assert resolvedItems.size() == 1 : "Expected a single resolution"; @@ -269,7 +269,7 @@ private Name resolveType(Name typePointer, BindingsMap availableSymbolsMap) { typePointer, new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedConstructor( "a method definition target"), - MetadataStorage.EMPTY); + new MetadataStorage()); } case ResolvedModule resMod -> { MetadataInteropHelpers.updateMetadata(typePointer, this, new Resolution(resMod)); @@ -284,35 +284,35 @@ private Name resolveType(Name typePointer, BindingsMap availableSymbolsMap) { typePointer, new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedPolyglot( "a method definition target"), - MetadataStorage.EMPTY); + new MetadataStorage()); } case ResolvedPolyglotField ignored -> { return new org.enso.compiler.core.ir.expression.errors.Resolution( typePointer, new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedPolyglot( "a method definition target"), - MetadataStorage.EMPTY); + new MetadataStorage()); } case ResolvedModuleMethod ignored -> { return new org.enso.compiler.core.ir.expression.errors.Resolution( typePointer, new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( "a method definition target"), - MetadataStorage.EMPTY); + new MetadataStorage()); } case ResolvedExtensionMethod ignored -> { return new org.enso.compiler.core.ir.expression.errors.Resolution( typePointer, new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( "a static method definition target"), - MetadataStorage.EMPTY); + new MetadataStorage()); } case ResolvedConversionMethod ignored -> { return new org.enso.compiler.core.ir.expression.errors.Resolution( typePointer, new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( "a conversion method definition target"), - MetadataStorage.EMPTY); + new MetadataStorage()); } default -> throw new IllegalStateException("Unexpected value: " + resolvedItems.head()); } diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java index c41476f14bcc..4d80ba523ebe 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java @@ -18,7 +18,6 @@ @Persistable(id = 398) public final class MetadataStorage { private Map metadata; - public static final MetadataStorage EMPTY = new MetadataStorage(); /** Constructs empty, ready to be populated metadata. */ public MetadataStorage() { From 9136a8b88b35cb042cf6b4d93e6af13ec8d73823 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 18:10:17 +0100 Subject: [PATCH 44/63] FIx compilation error in a test --- .../pass/resolve/MethodDefinitionsTest.scala | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala index 2db7ad29c79e..cf1694380bbf 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala @@ -78,7 +78,10 @@ class MethodDefinitionsTest extends CompilerTest { .methodReference .typePointer .get - .getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) shouldEqual Some( + .getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedType( ctx.moduleReference(), @@ -96,7 +99,10 @@ class MethodDefinitionsTest extends CompilerTest { .methodReference .typePointer .get - .getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) shouldEqual Some( + .getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedModule(ctx.moduleReference()) ) @@ -122,7 +128,10 @@ class MethodDefinitionsTest extends CompilerTest { ) ) ) - conv1.sourceTypeName.getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) shouldEqual Some( + conv1.sourceTypeName.getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedType( ctx.moduleReference(), @@ -151,7 +160,10 @@ class MethodDefinitionsTest extends CompilerTest { .bindings(8) .asInstanceOf[definition.Method.Conversion] conv3.methodReference.typePointer.get shouldBe an[errors.Resolution] - conv3.sourceTypeName.getMetadata(MethodDefinitions.INSTANCE, classOf[BindingsMap.Resolution]) shouldEqual Some( + conv3.sourceTypeName.getMetadata( + MethodDefinitions.INSTANCE, + classOf[BindingsMap.Resolution] + ) shouldEqual Some( BindingsMap.Resolution( BindingsMap.ResolvedType( ctx.moduleReference(), From a3321fee972bcb2b9f3292286a97708988fc07d9 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Fri, 22 Nov 2024 18:10:37 +0100 Subject: [PATCH 45/63] MethodDefinitions.lambdaWithNewSelfArg copies arguments --- .../org/enso/compiler/pass/resolve/MethodDefinitions.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java index 80123a7eb797..931c35a66876 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java @@ -76,9 +76,7 @@ public Module runModule(Module ir, ModuleContext moduleContext) { if (def instanceof Method method) { var methodRef = method.methodReference(); Option resolvedTypeRef = - methodRef - .typePointer() - .map(tp -> resolveType(tp, availableSymbolsMap)); + methodRef.typePointer().map(tp -> resolveType(tp, availableSymbolsMap)); var resolvedMethodRef = methodRef.copyWithTypePointer(resolvedTypeRef); return switch (method) { @@ -228,7 +226,7 @@ private static Expression addTypeAscriptionToSelfArgument(Expression methodBody) private static Function.Lambda lambdaWithNewSelfArg( Function.Lambda lambda, DefinitionArgument newSelfArg) { - var args = CollectionConverters.asJava(lambda.arguments()); + var args = new ArrayList<>(CollectionConverters.asJava(lambda.arguments())); assert !args.isEmpty(); args.set(0, newSelfArg); var newArgs = CollectionConverters.asScala(args).toList(); From a47798f5008b5da18e6e14e2a7a3608aae26baf8 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 25 Nov 2024 12:04:02 +0100 Subject: [PATCH 46/63] Fix copy of static method in MethodDefinitions --- .../compiler/pass/resolve/MethodDefinitions.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java index 931c35a66876..f7a0318b58eb 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java @@ -187,15 +187,15 @@ && canGenerateStaticWrappers(resType.tp())) { // We add a type check to it to ensure only `instance` of `My_Type` can be passed to it. var staticMethod = dup.copy( - method.methodReference(), + dup.methodReference(), newBody, true, - method.isPrivate(), + dup.isPrivate(), true, - method.location(), - method.passData(), - method.diagnostics(), - method.id()); + dup.location(), + dup.passData(), + dup.diagnostics(), + dup.id()); return staticMethod; } return null; From 6c77612b4cdf8d9821869c950a09652cadc7a6f0 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 25 Nov 2024 15:16:21 +0100 Subject: [PATCH 47/63] Ensure consistency between ModuleContext and IR in uncachedParseModule --- .../src/main/scala/org/enso/compiler/Compiler.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala index e7d0db8505c1..8df72d828b26 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/Compiler.scala @@ -606,6 +606,7 @@ class Compiler( expr else injectSyntheticModuleExports(expr, module.getDirectModulesRefs) + context.updateModule(module, _.ir(exprWithModuleExports)) val discoveredModule = recognizeBindings(exprWithModuleExports, moduleContext) if (context.wasLoadedFromCache(module)) { From a1d2270a259b4425380bb5856ba2a8e19e726da4 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 25 Nov 2024 15:17:06 +0100 Subject: [PATCH 48/63] Better error message in IrToTruffle.generateReExportBindings --- .../main/scala/org/enso/interpreter/runtime/IrToTruffle.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala b/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala index 93054d770922..1ad5b986a45f 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala @@ -1142,7 +1142,8 @@ class IrToTruffle( ) org.enso.common.Asserts.assertInJvm( fun != null, - s"exported symbol (static method) `${staticMethod.name}` needs to be registered first in the module " + s"exported symbol (static method) `${staticMethod.name}` on type '${eigenTp.getName}' " + + s"needs to be registered first in the module '${actualModule.getName.toString}'." ) scopeBuilder.registerMethod( scopeAssociatedType, From b0ce6c4234bea340d02276eccba167ba42c082ba Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 26 Nov 2024 13:27:11 +0100 Subject: [PATCH 49/63] Fix No_Such_Method error. Problem was in MetadataStorage.get(MethodDefinitions.INSTANCE) that returned null if MethodDefinitions is not final. --- .../org/enso/compiler/pass/resolve/MethodDefinitions.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java index f7a0318b58eb..ec8880b435d9 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java @@ -44,7 +44,9 @@ * *

Metadata type is {@link BindingsMap.Resolution} */ -public class MethodDefinitions implements IRPass { +public final class MethodDefinitions implements IRPass { + private MethodDefinitions() {} + public static final MethodDefinitions INSTANCE = new MethodDefinitions(); @Override From fb3e29d87b1328ca8bb78ef73c76421cc1e6db8f Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 26 Nov 2024 14:19:36 +0100 Subject: [PATCH 50/63] Fix invalid IR in GlobalNamesTest --- .../scala/org/enso/compiler/test/CompilerTest.scala | 11 +++++++++++ .../compiler/test/pass/resolve/GlobalNamesTest.scala | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala index cff592f1941d..a81def60f616 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/CompilerTest.scala @@ -73,6 +73,17 @@ trait CompilerRunner { newIr }) } + + def runPasses( + passManager: PassManager, + passGroup: PassGroup, + moduleContext: ModuleContext + ): Module = { + val runtimeMod = runtime.Module.fromCompilerModule(moduleContext.module) + ModuleTestUtils.unsafeSetIr(runtimeMod, ir) + val newIr = passManager.runPassesOnModule(ir, moduleContext, passGroup) + newIr + } } /** Wrapper for the implicit method. Callable from Java. diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/GlobalNamesTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/GlobalNamesTest.scala index f16c70bea1ee..c79fd0998d74 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/GlobalNamesTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/GlobalNamesTest.scala @@ -85,11 +85,11 @@ class GlobalNamesTest extends CompilerTest { | |""".stripMargin val parsed = EnsoParser.compile(code) - val moduleMapped = passManager.runPassesOnModule(parsed, ctx, group1) + val moduleMapped = parsed.runPasses(passManager, group1, ctx) ModuleTestUtils.unsafeSetIr(both._2, moduleMapped) new ExportsResolution(null).run(List(both._2.asCompilerModule())) - val allPrecursors = passManager.runPassesOnModule(moduleMapped, ctx, group2) + val allPrecursors = moduleMapped.runPasses(passManager, group2, ctx) val ir = allPrecursors.analyse val bodyExprs = ir From 808ae22a897b7791e01bb445ba369012a5d14e7c Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 27 Nov 2024 15:17:56 +0100 Subject: [PATCH 51/63] Convert MethodDefinitions to mini pass --- .../enso/compiler/MetadataInteropHelpers.java | 6 +- .../pass/resolve/MethodDefinitions.java | 516 +++++++++--------- .../pass/analyse/BindingAnalysis.scala | 4 +- 3 files changed, 277 insertions(+), 249 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/MetadataInteropHelpers.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/MetadataInteropHelpers.java index 74d2dbe808a7..4b7f270df95d 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/MetadataInteropHelpers.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/MetadataInteropHelpers.java @@ -3,7 +3,7 @@ import org.enso.compiler.core.IR; import org.enso.compiler.core.ir.MetadataStorage.MetadataPair; import org.enso.compiler.core.ir.ProcessingPass; -import org.enso.compiler.pass.IRPass; +import org.enso.compiler.pass.IRProcessingPass; import scala.Option; /** @@ -12,7 +12,7 @@ *

This encapsulates the friction of interop between Scala and Java types. */ public final class MetadataInteropHelpers { - public static T getMetadataOrNull(IR ir, IRPass pass, Class expectedType) { + public static T getMetadataOrNull(IR ir, IRProcessingPass pass, Class expectedType) { Option option = ir.passData().get(pass); if (option.isDefined()) { try { @@ -31,7 +31,7 @@ public static T getMetadataOrNull(IR ir, IRPass pass, Class expectedType) } } - public static T getMetadata(IR ir, IRPass pass, Class expectedType) { + public static T getMetadata(IR ir, IRProcessingPass pass, Class expectedType) { T metadataOrNull = getMetadataOrNull(ir, pass, expectedType); if (metadataOrNull == null) { throw new IllegalStateException("Missing expected " + pass + " metadata for " + ir + "."); diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java index ec8880b435d9..3264faf4610f 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/resolve/MethodDefinitions.java @@ -27,8 +27,9 @@ import org.enso.compiler.data.BindingsMap.ResolvedPolyglotSymbol; import org.enso.compiler.data.BindingsMap.ResolvedType; import org.enso.compiler.data.BindingsMap.Type; -import org.enso.compiler.pass.IRPass; import org.enso.compiler.pass.IRProcessingPass; +import org.enso.compiler.pass.MiniIRPass; +import org.enso.compiler.pass.MiniPassFactory; import org.enso.compiler.pass.analyse.BindingAnalysis$; import org.enso.compiler.pass.desugar.ComplexType$; import org.enso.compiler.pass.desugar.FunctionBinding$; @@ -44,11 +45,23 @@ * *

Metadata type is {@link BindingsMap.Resolution} */ -public final class MethodDefinitions implements IRPass { +public final class MethodDefinitions implements MiniPassFactory { private MethodDefinitions() {} public static final MethodDefinitions INSTANCE = new MethodDefinitions(); + @Override + public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { + var bindingsMap = moduleContext.bindingsAnalysis(); + assert bindingsMap != null; + return new Mini(bindingsMap); + } + + @Override + public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { + return null; + } + @Override public Seq precursorPasses() { java.util.List passes = @@ -67,272 +80,287 @@ public Seq invalidatedPasses() { return (scala.collection.immutable.List) obj; } - @Override - public Module runModule(Module ir, ModuleContext moduleContext) { - BindingsMap availableSymbolsMap = - MetadataInteropHelpers.getMetadata(ir, BindingAnalysis$.MODULE$, BindingsMap.class); - var newDefs = - ir.bindings() - .map( - def -> { - if (def instanceof Method method) { - var methodRef = method.methodReference(); - Option resolvedTypeRef = - methodRef.typePointer().map(tp -> resolveType(tp, availableSymbolsMap)); - var resolvedMethodRef = methodRef.copyWithTypePointer(resolvedTypeRef); + private static List list(T item) { + return CollectionConverters.asScala(java.util.List.of(item)).toList(); + } - return switch (method) { - case Method.Explicit explicitMethod -> { - var isStatic = computeIsStatic(explicitMethod.body()); - var resolvedMethod = - explicitMethod.copy( - resolvedMethodRef, - explicitMethod.body(), - isStatic, - explicitMethod.isPrivate(), - explicitMethod.isStaticWrapperForInstanceMethod(), - explicitMethod.location(), - explicitMethod.passData(), - explicitMethod.diagnostics(), - explicitMethod.id()); - yield resolvedMethod; - } - case Method.Conversion conversionMethod -> { - var sourceTypeExpr = conversionMethod.sourceTypeName(); - Name resolvedName = - switch (sourceTypeExpr) { - case Name name -> resolveType(name, availableSymbolsMap); - default -> new Conversion( - sourceTypeExpr, - UnsupportedSourceType$.MODULE$, - new MetadataStorage()); - }; - var resolvedMethod = - conversionMethod.copy( - resolvedMethodRef, - resolvedName, - conversionMethod.body(), - conversionMethod.location(), - conversionMethod.passData(), - conversionMethod.diagnostics(), - conversionMethod.id()); - yield resolvedMethod; - } - default -> throw new CompilerError( - "Unexpected method type in MethodDefinitions pass."); - }; - } else { - return def; - } - }); + private static boolean computeIsStatic(IR body) { + return Method.Explicit$.MODULE$.computeIsStatic(body); + } - java.util.List withStaticAliases = new ArrayList<>(); - for (var def : CollectionConverters.asJava(newDefs)) { - withStaticAliases.add(def); - if (def instanceof Method.Explicit method && !method.isStatic()) { - var staticAlias = generateStaticAliasMethod(method); - if (staticAlias != null) { - withStaticAliases.add(staticAlias); - } - } - } + private static final class Mini extends MiniIRPass { + private final BindingsMap bindingsMap; - return ir.copyWithBindings(CollectionConverters.asScala(withStaticAliases).toList()); - } + private Mini(BindingsMap bindingsMap) { + this.bindingsMap = bindingsMap; + } - /** - * Returns null if there is no suitable static alias method that can be generated for the given - * {@code method}. - * - * @param method Non-static method from which a static alias method is generated. - * @return Static alias method for the given {@code method} or null. - */ - private Method.Explicit generateStaticAliasMethod(Method.Explicit method) { - assert !method.isStatic(); - var typePointer = method.methodReference().typePointer(); - if (typePointer.isEmpty()) { - return null; + @Override + public Expression transformExpression(Expression expr) { + throw new IllegalStateException("unreachable - prepare returns null."); } - var resolution = - MetadataInteropHelpers.getMetadataOrNull(typePointer.get(), this, Resolution.class); - if (resolution == null) { + + @Override + public MiniIRPass prepare(IR parent, Expression child) { + // Supports only transformModule return null; } - if (resolution.target() instanceof ResolvedType resType - && canGenerateStaticWrappers(resType.tp())) { - assert method.body() instanceof Function.Lambda; - var dup = method.duplicate(true, true, true, false); - // This is the self argument that will receive the `SelfType.type` value upon dispatch, it is - // added to avoid modifying the dispatch mechanism. - var syntheticModuleSelfArg = - new DefinitionArgument.Specified( - new Name.Self(null, true, new MetadataStorage()), - Option.empty(), - Option.empty(), - false, - null, - new MetadataStorage()); - var newBody = - new Function.Lambda( - // This is the synthetic Self argument that gets the static module - list(syntheticModuleSelfArg), - // Here we add the type ascription ensuring that the 'proper' self argument only - // accepts _instances_ of the type (or triggers conversions) - addTypeAscriptionToSelfArgument(dup.body()), - null, - true, - new MetadataStorage()); - // The actual `self` argument that is referenced inside of method body is the second one in - // the lambda. - // This is the argument that will hold the actual instance of the object we are calling on, - // e.g. `My_Type.method instance`. - // We add a type check to it to ensure only `instance` of `My_Type` can be passed to it. - var staticMethod = - dup.copy( - dup.methodReference(), - newBody, - true, - dup.isPrivate(), - true, - dup.location(), - dup.passData(), - dup.diagnostics(), - dup.id()); - return staticMethod; + + @Override + public Module transformModule(Module moduleIr) { + var newDefs = + moduleIr + .bindings() + .map( + def -> { + if (def instanceof Method method) { + var methodRef = method.methodReference(); + Option resolvedTypeRef = + methodRef.typePointer().map(tp -> resolveType(tp, bindingsMap)); + var resolvedMethodRef = methodRef.copyWithTypePointer(resolvedTypeRef); + + return switch (method) { + case Method.Explicit explicitMethod -> { + var isStatic = computeIsStatic(explicitMethod.body()); + var resolvedMethod = + explicitMethod.copy( + resolvedMethodRef, + explicitMethod.body(), + isStatic, + explicitMethod.isPrivate(), + explicitMethod.isStaticWrapperForInstanceMethod(), + explicitMethod.location(), + explicitMethod.passData(), + explicitMethod.diagnostics(), + explicitMethod.id()); + yield resolvedMethod; + } + case Method.Conversion conversionMethod -> { + var sourceTypeExpr = conversionMethod.sourceTypeName(); + Name resolvedName = + switch (sourceTypeExpr) { + case Name name -> resolveType(name, bindingsMap); + default -> new Conversion( + sourceTypeExpr, + UnsupportedSourceType$.MODULE$, + new MetadataStorage()); + }; + var resolvedMethod = + conversionMethod.copy( + resolvedMethodRef, + resolvedName, + conversionMethod.body(), + conversionMethod.location(), + conversionMethod.passData(), + conversionMethod.diagnostics(), + conversionMethod.id()); + yield resolvedMethod; + } + default -> throw new CompilerError( + "Unexpected method type in MethodDefinitions pass."); + }; + } else { + return def; + } + }); + + java.util.List withStaticAliases = new ArrayList<>(); + for (var def : CollectionConverters.asJava(newDefs)) { + withStaticAliases.add(def); + if (def instanceof Method.Explicit method && !method.isStatic()) { + var staticAlias = generateStaticAliasMethod(method); + if (staticAlias != null) { + withStaticAliases.add(staticAlias); + } + } + } + + return moduleIr.copyWithBindings(CollectionConverters.asScala(withStaticAliases).toList()); } - return null; - } - private static Expression addTypeAscriptionToSelfArgument(Expression methodBody) { - if (methodBody instanceof Function.Lambda lambda) { - if (lambda.arguments().isEmpty()) { - throw new CompilerError( - "MethodDefinitions pass: expected at least one argument (self) in the method, but got" - + " none."); + /** + * Returns null if there is no suitable static alias method that can be generated for the given + * {@code method}. + * + * @param method Non-static method from which a static alias method is generated. + * @return Static alias method for the given {@code method} or null. + */ + private Method.Explicit generateStaticAliasMethod(Method.Explicit method) { + assert !method.isStatic(); + var typePointer = method.methodReference().typePointer(); + if (typePointer.isEmpty()) { + return null; + } + var resolution = + MetadataInteropHelpers.getMetadataOrNull(typePointer.get(), INSTANCE, Resolution.class); + if (resolution == null) { + return null; } - var firstArg = lambda.arguments().head(); - if (firstArg instanceof DefinitionArgument.Specified selfArg - && selfArg.name() instanceof Name.Self) { - var selfType = new Name.SelfType(selfArg.identifiedLocation(), new MetadataStorage()); - var newSelfArg = selfArg.copyWithAscribedType(selfType); - return lambdaWithNewSelfArg(lambda, newSelfArg); + if (resolution.target() instanceof ResolvedType resType + && canGenerateStaticWrappers(resType.tp())) { + assert method.body() instanceof Function.Lambda; + var dup = method.duplicate(true, true, true, false); + // This is the self argument that will receive the `SelfType.type` value upon dispatch, it + // is + // added to avoid modifying the dispatch mechanism. + var syntheticModuleSelfArg = + new DefinitionArgument.Specified( + new Name.Self(null, true, new MetadataStorage()), + Option.empty(), + Option.empty(), + false, + null, + new MetadataStorage()); + var newBody = + new Function.Lambda( + // This is the synthetic Self argument that gets the static module + list(syntheticModuleSelfArg), + // Here we add the type ascription ensuring that the 'proper' self argument only + // accepts _instances_ of the type (or triggers conversions) + addTypeAscriptionToSelfArgument(dup.body()), + null, + true, + new MetadataStorage()); + // The actual `self` argument that is referenced inside of method body is the second one in + // the lambda. + // This is the argument that will hold the actual instance of the object we are calling on, + // e.g. `My_Type.method instance`. + // We add a type check to it to ensure only `instance` of `My_Type` can be passed to it. + var staticMethod = + dup.copy( + dup.methodReference(), + newBody, + true, + dup.isPrivate(), + true, + dup.location(), + dup.passData(), + dup.diagnostics(), + dup.id()); + return staticMethod; + } + return null; + } + + private static Expression addTypeAscriptionToSelfArgument(Expression methodBody) { + if (methodBody instanceof Function.Lambda lambda) { + if (lambda.arguments().isEmpty()) { + throw new CompilerError( + "MethodDefinitions pass: expected at least one argument (self) in the method, but got" + + " none."); + } + var firstArg = lambda.arguments().head(); + if (firstArg instanceof DefinitionArgument.Specified selfArg + && selfArg.name() instanceof Name.Self) { + var selfType = new Name.SelfType(selfArg.identifiedLocation(), new MetadataStorage()); + var newSelfArg = selfArg.copyWithAscribedType(selfType); + return lambdaWithNewSelfArg(lambda, newSelfArg); + } else { + throw new CompilerError( + "MethodDefinitions pass: expected the first argument to be `self`, but got " + + firstArg); + } } else { throw new CompilerError( - "MethodDefinitions pass: expected the first argument to be `self`, but got " - + firstArg); + "Unexpected body type " + methodBody + " in MethodDefinitions pass."); } - } else { - throw new CompilerError("Unexpected body type " + methodBody + " in MethodDefinitions pass."); } - } - private static Function.Lambda lambdaWithNewSelfArg( - Function.Lambda lambda, DefinitionArgument newSelfArg) { - var args = new ArrayList<>(CollectionConverters.asJava(lambda.arguments())); - assert !args.isEmpty(); - args.set(0, newSelfArg); - var newArgs = CollectionConverters.asScala(args).toList(); - return lambda.copyWithArguments(newArgs); - } + private static Function.Lambda lambdaWithNewSelfArg( + Function.Lambda lambda, DefinitionArgument newSelfArg) { + var args = new ArrayList<>(CollectionConverters.asJava(lambda.arguments())); + assert !args.isEmpty(); + args.set(0, newSelfArg); + var newArgs = CollectionConverters.asScala(args).toList(); + return lambda.copyWithArguments(newArgs); + } - // Generate static wrappers for - // 1. Types having at least one type constructor - // 2. All builtin types except for Nothing. Nothing's eigentype is Nothing and not Nothing.type, - // would lead to overriding conflicts. - // TODO: Remove the hardcoded type once Enso's annotations can define parameters. - private static boolean canGenerateStaticWrappers(Type tp) { - return tp.members().nonEmpty() || (tp.builtinType() && !"Nothing".equals(tp.name())); - } + // Generate static wrappers for + // 1. Types having at least one type constructor + // 2. All builtin types except for Nothing. Nothing's eigentype is Nothing and not Nothing.type, + // would lead to overriding conflicts. + // TODO: Remove the hardcoded type once Enso's annotations can define parameters. + private static boolean canGenerateStaticWrappers(Type tp) { + return tp.members().nonEmpty() || (tp.builtinType() && !"Nothing".equals(tp.name())); + } - private Name resolveType(Name typePointer, BindingsMap availableSymbolsMap) { - if (typePointer instanceof Name.Qualified || typePointer instanceof Name.Literal) { - var items = - switch (typePointer) { - case Name.Qualified qualName -> qualName.parts().map(Name::name); - case Name.Literal lit -> list(lit.name()); - default -> throw new CompilerError("Impossible to reach"); - }; + private Name resolveType(Name typePointer, BindingsMap availableSymbolsMap) { + if (typePointer instanceof Name.Qualified || typePointer instanceof Name.Literal) { + var items = + switch (typePointer) { + case Name.Qualified qualName -> qualName.parts().map(Name::name); + case Name.Literal lit -> list(lit.name()); + default -> throw new CompilerError("Impossible to reach"); + }; - var resolvedItemsOpt = availableSymbolsMap.resolveQualifiedName(items); - if (resolvedItemsOpt.isLeft()) { - var err = resolvedItemsOpt.swap().toOption().get(); - return new org.enso.compiler.core.ir.expression.errors.Resolution( - typePointer, - new org.enso.compiler.core.ir.expression.errors.Resolution.ResolverError(err), - new MetadataStorage()); - } - var resolvedItems = resolvedItemsOpt.toOption().get(); - assert resolvedItems.size() == 1 : "Expected a single resolution"; - switch (resolvedItems.head()) { - case ResolvedConstructor ignored -> { - return new org.enso.compiler.core.ir.expression.errors.Resolution( - typePointer, - new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedConstructor( - "a method definition target"), - new MetadataStorage()); - } - case ResolvedModule resMod -> { - MetadataInteropHelpers.updateMetadata(typePointer, this, new Resolution(resMod)); - return typePointer; - } - case ResolvedType resType -> { - MetadataInteropHelpers.updateMetadata(typePointer, this, new Resolution(resType)); - return typePointer; - } - case ResolvedPolyglotSymbol ignored -> { - return new org.enso.compiler.core.ir.expression.errors.Resolution( - typePointer, - new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedPolyglot( - "a method definition target"), - new MetadataStorage()); - } - case ResolvedPolyglotField ignored -> { + var resolvedItemsOpt = availableSymbolsMap.resolveQualifiedName(items); + if (resolvedItemsOpt.isLeft()) { + var err = resolvedItemsOpt.swap().toOption().get(); return new org.enso.compiler.core.ir.expression.errors.Resolution( typePointer, - new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedPolyglot( - "a method definition target"), + new org.enso.compiler.core.ir.expression.errors.Resolution.ResolverError(err), new MetadataStorage()); } - case ResolvedModuleMethod ignored -> { - return new org.enso.compiler.core.ir.expression.errors.Resolution( - typePointer, - new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( - "a method definition target"), - new MetadataStorage()); + var resolvedItems = resolvedItemsOpt.toOption().get(); + assert resolvedItems.size() == 1 : "Expected a single resolution"; + switch (resolvedItems.head()) { + case ResolvedConstructor ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedConstructor( + "a method definition target"), + new MetadataStorage()); + } + case ResolvedModule resMod -> { + MetadataInteropHelpers.updateMetadata(typePointer, INSTANCE, new Resolution(resMod)); + return typePointer; + } + case ResolvedType resType -> { + MetadataInteropHelpers.updateMetadata(typePointer, INSTANCE, new Resolution(resType)); + return typePointer; + } + case ResolvedPolyglotSymbol ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedPolyglot( + "a method definition target"), + new MetadataStorage()); + } + case ResolvedPolyglotField ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedPolyglot( + "a method definition target"), + new MetadataStorage()); + } + case ResolvedModuleMethod ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( + "a method definition target"), + new MetadataStorage()); + } + case ResolvedExtensionMethod ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( + "a static method definition target"), + new MetadataStorage()); + } + case ResolvedConversionMethod ignored -> { + return new org.enso.compiler.core.ir.expression.errors.Resolution( + typePointer, + new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( + "a conversion method definition target"), + new MetadataStorage()); + } + default -> throw new IllegalStateException("Unexpected value: " + resolvedItems.head()); } - case ResolvedExtensionMethod ignored -> { - return new org.enso.compiler.core.ir.expression.errors.Resolution( - typePointer, - new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( - "a static method definition target"), - new MetadataStorage()); - } - case ResolvedConversionMethod ignored -> { - return new org.enso.compiler.core.ir.expression.errors.Resolution( - typePointer, - new org.enso.compiler.core.ir.expression.errors.Resolution.UnexpectedMethod( - "a conversion method definition target"), - new MetadataStorage()); - } - default -> throw new IllegalStateException("Unexpected value: " + resolvedItems.head()); + } else if (typePointer instanceof org.enso.compiler.core.ir.expression.errors.Resolution) { + return typePointer; + } else { + throw new CompilerError("Unexpected kind of name for method reference"); } - } else if (typePointer instanceof org.enso.compiler.core.ir.expression.errors.Resolution) { - return typePointer; - } else { - throw new CompilerError("Unexpected kind of name for method reference"); } } - - private static List list(T item) { - return CollectionConverters.asScala(java.util.List.of(item)).toList(); - } - - @Override - public Expression runExpression(Expression ir, InlineContext inlineContext) { - return ir; - } - - private static boolean computeIsStatic(IR body) { - return Method.Explicit$.MODULE$.computeIsStatic(body); - } } diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/BindingAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/BindingAnalysis.scala index e58b744058ec..913c9a5b00ca 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/BindingAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/BindingAnalysis.scala @@ -13,7 +13,7 @@ import org.enso.compiler.data.BindingsMap.{ ExtensionMethod, ModuleMethod } -import org.enso.compiler.pass.IRPass +import org.enso.compiler.pass.{IRPass, IRProcessingPass} import org.enso.compiler.pass.desugar.{ ComplexType, FunctionBinding, @@ -40,7 +40,7 @@ case object BindingAnalysis extends IRPass { Seq(ComplexType, FunctionBinding, GenerateMethodBodies) /** The passes that are invalidated by running this pass. */ - override lazy val invalidatedPasses: Seq[IRPass] = + override lazy val invalidatedPasses: Seq[IRProcessingPass] = Seq(MethodDefinitions.INSTANCE, Patterns) /** Executes the pass on the provided `ir`, and returns a possibly transformed From 8d681469bec48ae7d9bce025d9bddf89dd46c654 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 27 Nov 2024 16:14:39 +0100 Subject: [PATCH 52/63] FIx compilation of MethodDefinitionsTest --- .../test/pass/resolve/MethodDefinitionsTest.scala | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala index cf1694380bbf..b6f19462701e 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/MethodDefinitionsTest.scala @@ -9,7 +9,12 @@ import org.enso.compiler.core.ir.expression.errors import org.enso.compiler.data.BindingsMap import org.enso.compiler.data.BindingsMap.Type import org.enso.compiler.pass.resolve.MethodDefinitions -import org.enso.compiler.pass.{PassConfiguration, PassGroup, PassManager} +import org.enso.compiler.pass.{ + MiniIRPass, + PassConfiguration, + PassGroup, + PassManager +} import org.enso.compiler.test.CompilerTest class MethodDefinitionsTest extends CompilerTest { @@ -43,7 +48,9 @@ class MethodDefinitionsTest extends CompilerTest { * @return [[ir]], with tail call analysis metadata attached */ def analyse(implicit context: ModuleContext): Module = { - MethodDefinitions.INSTANCE.runModule(ir, context) + val miniPass = + MethodDefinitions.INSTANCE.createForModuleCompilation(context) + MiniIRPass.compile(classOf[Module], ir, miniPass) } } From fe7e50a21cbfc68892bc03f898af399c099c8b11 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 27 Nov 2024 18:16:10 +0100 Subject: [PATCH 53/63] Remove TODO comment --- .../src/main/scala/org/enso/compiler/pass/PassManager.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 9de06659266f..ce99a32f8945 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -101,7 +101,6 @@ class PassManager( } /** Executes all passes on the [[Expression]]. - * TODO: Remove this method? * * @param ir the expression to execute the compiler passes on * @param inlineContext the inline context in which the passes are executed From 417843dc850d49075f89970f4284749e15118e5d Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 11:14:05 +0100 Subject: [PATCH 54/63] Move ScalaConversions to scala-libs-wrapper --- build.sbt | 2 +- engine/common/src/main/java/module-info.java | 1 - engine/runtime-compiler/src/main/java/module-info.java | 1 + .../enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java | 2 +- .../org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java | 2 +- .../enso/interpreter/instrument/job/ExecuteExpressionJob.java | 2 +- .../java/org/enso/compiler/test/SerializationManagerTest.java | 2 +- .../test/java/org/enso/interpreter/runtime/ModuleTest.java | 2 +- .../java/org/enso/interpreter/test/meta/EnsoProjectTest.java | 2 +- .../org/enso/interpreter/runtime/TruffleCompilerContext.java | 4 ++-- .../org/enso/interpreter/runtime/scope/TopLevelScope.java | 2 +- lib/java/scala-libs-wrapper/src/main/java/module-info.java | 2 ++ .../main/java/org/enso/scala/wrapper}/ScalaConversions.java | 2 +- 13 files changed, 14 insertions(+), 12 deletions(-) rename {engine/common/src/main/java/org/enso/common => lib/java/scala-libs-wrapper/src/main/java/org/enso/scala/wrapper}/ScalaConversions.java (98%) diff --git a/build.sbt b/build.sbt index 1e69e54eb36b..f84ed0f06623 100644 --- a/build.sbt +++ b/build.sbt @@ -2160,7 +2160,6 @@ lazy val `engine-common` = project .enablePlugins(JPMSPlugin) .settings( frgaalJavaCompilerSetting, - scalaModuleDependencySetting, Test / fork := true, commands += WithDebugCommand.withDebug, Test / envVars ++= distributionEnvironmentOverrides, @@ -3253,6 +3252,7 @@ lazy val `runtime-compiler` = (`pkg` / Compile / exportedModule).value, (`runtime-parser` / Compile / exportedModule).value, (`syntax-rust-definition` / Compile / exportedModule).value, + (`scala-libs-wrapper` / Compile / exportedModule).value, (`persistance` / Compile / exportedModule).value, (`editions` / Compile / exportedModule).value ), diff --git a/engine/common/src/main/java/module-info.java b/engine/common/src/main/java/module-info.java index d6404d8e6de2..ab242d316c5e 100644 --- a/engine/common/src/main/java/module-info.java +++ b/engine/common/src/main/java/module-info.java @@ -1,5 +1,4 @@ module org.enso.engine.common { - requires scala.library; requires org.graalvm.polyglot; requires org.enso.logging.utils; requires org.enso.logging.config; diff --git a/engine/runtime-compiler/src/main/java/module-info.java b/engine/runtime-compiler/src/main/java/module-info.java index 17cd6c98e308..e081f014144a 100644 --- a/engine/runtime-compiler/src/main/java/module-info.java +++ b/engine/runtime-compiler/src/main/java/module-info.java @@ -8,6 +8,7 @@ requires org.enso.runtime.parser; requires static org.enso.persistance; requires org.enso.syntax; + requires org.enso.scala.wrapper; requires org.openide.util.lookup.RELEASE180; requires org.slf4j; diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java index 2e13c4872a88..591d13f8b776 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/AmbiguousImportsAnalysis.java @@ -4,7 +4,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import org.enso.common.ScalaConversions; +import org.enso.scala.wrapper.ScalaConversions; import org.enso.compiler.context.InlineContext; import org.enso.compiler.context.ModuleContext; import org.enso.compiler.core.CompilerError; diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java index 0654936c6df2..49eda9b3ddad 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -2,7 +2,6 @@ import java.util.ArrayList; import java.util.List; -import org.enso.common.ScalaConversions; import org.enso.compiler.context.InlineContext; import org.enso.compiler.context.ModuleContext; import org.enso.compiler.core.IR; @@ -17,6 +16,7 @@ import org.enso.compiler.pass.MiniIRPass; import org.enso.compiler.pass.MiniPassFactory; import org.enso.compiler.pass.desugar.GenerateMethodBodies$; +import org.enso.scala.wrapper.ScalaConversions; import scala.collection.immutable.Seq; import scala.jdk.javaapi.CollectionConverters; diff --git a/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java b/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java index 859ddc19a59c..220c25cf53d1 100644 --- a/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java +++ b/engine/runtime-instrument-common/src/main/java/org/enso/interpreter/instrument/job/ExecuteExpressionJob.java @@ -1,10 +1,10 @@ package org.enso.interpreter.instrument.job; import java.util.UUID; -import org.enso.common.ScalaConversions; import org.enso.interpreter.instrument.OneshotExpression; import org.enso.interpreter.instrument.execution.Executable; import org.enso.interpreter.instrument.execution.RuntimeContext; +import org.enso.scala.wrapper.ScalaConversions; /** The job that schedules the execution of the expression. */ public class ExecuteExpressionJob extends Job implements UniqueJob { diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java index 65d058a20ec9..bb41f3d6a129 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/SerializationManagerTest.java @@ -11,7 +11,6 @@ import org.apache.commons.io.FileUtils; import org.enso.common.LanguageInfo; import org.enso.common.MethodNames; -import org.enso.common.ScalaConversions; import org.enso.editions.LibraryName; import org.enso.interpreter.runtime.EnsoContext; import org.enso.interpreter.runtime.util.TruffleFileSystem; @@ -19,6 +18,7 @@ import org.enso.pkg.Package; import org.enso.pkg.PackageManager; import org.enso.polyglot.Suggestion; +import org.enso.scala.wrapper.ScalaConversions; import org.junit.After; import org.junit.Assert; import org.junit.Before; diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ModuleTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ModuleTest.java index 53b2f73dd1ef..c37652ab4795 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ModuleTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/runtime/ModuleTest.java @@ -1,6 +1,6 @@ package org.enso.interpreter.runtime; -import static org.enso.common.ScalaConversions.nil; +import static org.enso.scala.wrapper.ScalaConversions.nil; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/meta/EnsoProjectTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/meta/EnsoProjectTest.java index 017bd4d7956a..30a915f5e0ab 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/meta/EnsoProjectTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/meta/EnsoProjectTest.java @@ -9,9 +9,9 @@ import java.util.Set; import org.enso.common.LanguageInfo; import org.enso.common.RuntimeOptions; -import org.enso.common.ScalaConversions; import org.enso.pkg.QualifiedName; import org.enso.polyglot.PolyglotContext; +import org.enso.scala.wrapper.ScalaConversions; import org.enso.test.utils.ContextUtils; import org.enso.test.utils.ProjectUtils; import org.enso.test.utils.SourceModule; diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java index 8af87d0696db..f0c7d5069f39 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/TruffleCompilerContext.java @@ -1,7 +1,7 @@ package org.enso.interpreter.runtime; -import static org.enso.common.ScalaConversions.cons; -import static org.enso.common.ScalaConversions.nil; +import static org.enso.scala.wrapper.ScalaConversions.cons; +import static org.enso.scala.wrapper.ScalaConversions.nil; import com.oracle.truffle.api.TruffleFile; import com.oracle.truffle.api.TruffleLogger; diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java index b04dfa7f160d..79452f8043f8 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/scope/TopLevelScope.java @@ -15,7 +15,6 @@ import java.util.Optional; import java.util.concurrent.ExecutionException; import org.enso.common.MethodNames; -import org.enso.common.ScalaConversions; import org.enso.compiler.PackageRepository; import org.enso.editions.LibraryName; import org.enso.interpreter.runtime.EnsoContext; @@ -26,6 +25,7 @@ import org.enso.interpreter.runtime.type.Types; import org.enso.pkg.Package; import org.enso.pkg.QualifiedName; +import org.enso.scala.wrapper.ScalaConversions; /** Represents the top scope of Enso execution, containing all the importable modules. */ @ExportLibrary(InteropLibrary.class) diff --git a/lib/java/scala-libs-wrapper/src/main/java/module-info.java b/lib/java/scala-libs-wrapper/src/main/java/module-info.java index 8d56d1d2d699..fecf3f974905 100644 --- a/lib/java/scala-libs-wrapper/src/main/java/module-info.java +++ b/lib/java/scala-libs-wrapper/src/main/java/module-info.java @@ -4,6 +4,8 @@ requires org.jline; requires org.slf4j; + exports org.enso.scala.wrapper; + // "org.typelevel" % ("cats-core_" + scalaVer) % "2.10.0", exports cats; exports cats.arrow; diff --git a/engine/common/src/main/java/org/enso/common/ScalaConversions.java b/lib/java/scala-libs-wrapper/src/main/java/org/enso/scala/wrapper/ScalaConversions.java similarity index 98% rename from engine/common/src/main/java/org/enso/common/ScalaConversions.java rename to lib/java/scala-libs-wrapper/src/main/java/org/enso/scala/wrapper/ScalaConversions.java index bf19595cec1f..a2020178888f 100644 --- a/engine/common/src/main/java/org/enso/common/ScalaConversions.java +++ b/lib/java/scala-libs-wrapper/src/main/java/org/enso/scala/wrapper/ScalaConversions.java @@ -1,4 +1,4 @@ -package org.enso.common; +package org.enso.scala.wrapper; import java.util.List; import java.util.Optional; From 273bea8f37c724ffa9ae424af0b2314707af4ac4 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 16:42:43 +0100 Subject: [PATCH 55/63] Move check for null minipasses to the factory method when combining --- .../main/java/org/enso/compiler/pass/ChainedMiniPass.java | 6 ++++++ .../src/main/java/org/enso/compiler/pass/MiniIRPass.java | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java index 78d7cb215023..8794a1a606c2 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java @@ -15,6 +15,12 @@ private ChainedMiniPass(MiniIRPass firstPass, MiniIRPass secondPass) { } static MiniIRPass chain(MiniIRPass firstPass, MiniIRPass secondPass) { + if (firstPass == null) { + return secondPass; + } + if (secondPass == null) { + return firstPass; + } return new ChainedMiniPass(firstPass, secondPass); } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java index 46ad0a580700..1ee06dd4217b 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/MiniIRPass.java @@ -110,12 +110,6 @@ public String toString() { * if both provided passes are {@code null}. */ public static MiniIRPass combine(MiniIRPass first, MiniIRPass second) { - if (first == null) { - return second; - } - if (second == null) { - return first; - } return ChainedMiniPass.chain(first, second); } From bd10bf6d84df4e1442495b8d5cd81bfe0b0c6069 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 16:45:28 +0100 Subject: [PATCH 56/63] ChainedMiniPass can disable itself from processing subtree --- .../src/main/java/org/enso/compiler/pass/ChainedMiniPass.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java index 8794a1a606c2..3e4b7b24b43a 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java @@ -31,7 +31,7 @@ public MiniIRPass prepare(IR parent, Expression current) { if (firstPrepared == firstPass && secondPrepared == secondPass) { return this; } else { - return new ChainedMiniPass(firstPrepared, secondPrepared); + return chain(firstPass, secondPass); } } From b3f750eb52704a386b26400f54cc8d685ff3bc60 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 16:46:37 +0100 Subject: [PATCH 57/63] Rename variables --- .../main/java/org/enso/compiler/pass/ChainedMiniPass.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java index 3e4b7b24b43a..ae90df372be1 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java @@ -44,9 +44,9 @@ public Expression transformExpression(Expression ir) { @Override public Module transformModule(Module moduleIr) { - var first = firstPass == null ? moduleIr : firstPass.transformModule(moduleIr); - var second = secondPass == null ? first : secondPass.transformModule(first); - return second; + var firstIr = firstPass == null ? moduleIr : firstPass.transformModule(moduleIr); + var secondIr = secondPass == null ? firstIr : secondPass.transformModule(firstIr); + return secondIr; } @Override From cb5f8c1b6c194b1c8c5b255646d6d044e7389745 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 16:47:43 +0100 Subject: [PATCH 58/63] ImportSymbolAnalysis is final --- .../org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java index 49eda9b3ddad..ce7fc568b71e 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.java @@ -26,7 +26,7 @@ * the IR import with {@link org.enso.compiler.core.ir.expression.errors.ImportExport}. Reports only * the first unresolved symbol. */ -public class ImportSymbolAnalysis implements MiniPassFactory { +public final class ImportSymbolAnalysis implements MiniPassFactory { public static final ImportSymbolAnalysis INSTANCE = new ImportSymbolAnalysis(); private ImportSymbolAnalysis() {} From 85ab82f3ce9891445b4cd4ccddbbdff7361abf51 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 16:48:43 +0100 Subject: [PATCH 59/63] Typo in docs --- .../src/main/scala/org/enso/compiler/pass/PassManager.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index ce99a32f8945..9c643deeb200 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -248,7 +248,7 @@ class PassManager( } /** Validates consistency between the IR accessible via `moduleContext` and `ir`. - * There is no way how to enforce this consistency statically. + * There is no way to enforce this consistency statically. * Should be called only iff assertions are enabled. * @return true if they are consistent, otherwise throws [[AssertionError]]. */ From 3e1ade9fff64052528ddc436213675b14c458bc2 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 16:54:17 +0100 Subject: [PATCH 60/63] Make PassManager.passes field private --- .../src/main/scala/org/enso/compiler/pass/PassManager.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 9c643deeb200..11c6ed886686 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -22,7 +22,7 @@ import scala.collection.mutable.ListBuffer */ //noinspection DuplicatedCode class PassManager( - protected val passes: List[PassGroup], + private val passes: List[PassGroup], passConfiguration: PassConfiguration ) { private val logger = LoggerFactory.getLogger(classOf[PassManager]) From 4380b08eba7ade42bae7403f528decb76e3d767b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 17:14:53 +0100 Subject: [PATCH 61/63] Fix: ChainedMiniPass can disable itself from processing subtree --- .../src/main/java/org/enso/compiler/pass/ChainedMiniPass.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java index ae90df372be1..3b8888f00ab5 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/ChainedMiniPass.java @@ -31,7 +31,7 @@ public MiniIRPass prepare(IR parent, Expression current) { if (firstPrepared == firstPass && secondPrepared == secondPass) { return this; } else { - return chain(firstPass, secondPass); + return chain(firstPrepared, secondPrepared); } } From 023d1aa4165a805b7f5273209601f98435b9d4cc Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 17:21:01 +0100 Subject: [PATCH 62/63] Add mini pass traverser test --- .../test/pass/MiniPassTraverserTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MiniPassTraverserTest.java b/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MiniPassTraverserTest.java index de50a8eb68cf..f05e6b71e68c 100644 --- a/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MiniPassTraverserTest.java +++ b/engine/runtime-compiler/src/test/java/org/enso/compiler/test/pass/MiniPassTraverserTest.java @@ -105,4 +105,24 @@ public void chainedMiniPass_StopsTraversingWhenPrepareReturnsNull() { assertThat( "e2 should still be transformed by miniPass1", e2.isTransformedBy(miniPass1), is(true)); } + + @Test + public void chainedMiniPass_StopsTraversingWhenPrepareFromBothPassesReturnNull() { + var e1 = new MockExpression(false); + var e2 = new MockExpression(true); + var e3 = new MockExpression(true); + e1.addChild(e2); + e2.addChild(e3); + // Both mini passes process just e1. + var miniPass1 = MockMiniPass.builder().stopExpr(e2).build(); + var miniPass2 = MockMiniPass.builder().stopExpr(e2).build(); + var chainedPass = MiniIRPass.combine(miniPass1, miniPass2); + MiniIRPass.compile(MockExpression.class, e1, chainedPass); + assertThat("e3 should not be prepared by any pass", e3.isPreparedByAny(), is(false)); + assertThat("e3 should not be transformed by any pass", e3.isTransformedByAny(), is(false)); + assertThat("e2 should not be prepared by any pass", e2.isPreparedByAny(), is(false)); + assertThat("e2 should not be transformed by any pass", e2.isTransformedByAny(), is(false)); + assertThat("e1 should be processed by both passes", e1.isTransformedBy(miniPass1), is(true)); + assertThat("e1 should be processed by both passes", e1.isTransformedBy(miniPass2), is(true)); + } } From f605f1638de3b163283677be7e65b397612bf57f Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 28 Nov 2024 18:05:58 +0100 Subject: [PATCH 63/63] PassManager.passes field needs to be protected (package private) --- .../src/main/scala/org/enso/compiler/pass/PassManager.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 11c6ed886686..9c643deeb200 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -22,7 +22,7 @@ import scala.collection.mutable.ListBuffer */ //noinspection DuplicatedCode class PassManager( - private val passes: List[PassGroup], + protected val passes: List[PassGroup], passConfiguration: PassConfiguration ) { private val logger = LoggerFactory.getLogger(classOf[PassManager])