Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More IR mini passes #11501

Merged
merged 67 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
ad6504d
Move ScalaConversions from runtime to engine-common
Akirathan Nov 6, 2024
f811f2e
Convert ImportSymbolAnalysis to mini pass
Akirathan Nov 6, 2024
410bd55
fmt
Akirathan Nov 6, 2024
fd3814a
createForInlineCompilation cannot return null
Akirathan Nov 8, 2024
76af72e
ChainedMiniPass delegates to transformModule
Akirathan Nov 8, 2024
207946f
ImportSymbolAnalysis copies unchanged imports
Akirathan Nov 8, 2024
2e4106b
Fix compilation of a test
Akirathan Nov 8, 2024
eb20aaf
Combining mini passes can result in null
Akirathan Nov 8, 2024
91119ee
Merge branch 'develop' into wip/akirathan/11326-more-mini-passes
Akirathan Nov 14, 2024
8d3c4b3
Add consistency validation to PassManager
Akirathan Nov 15, 2024
cab74dc
Add assert that bindingsMap != null
Akirathan Nov 15, 2024
1935b9e
Consistency validation has better error message.
Akirathan Nov 19, 2024
546f17c
CompilerTestSetup explicitly changes IR in ModuleContext.
Akirathan Nov 19, 2024
289a255
runtime-integration-tests accesses PassManager field via reflection
Akirathan Nov 19, 2024
494975d
Add MiniPassTraverserTest
Akirathan Nov 20, 2024
deadad5
MiniPassTraverser supports end of the traversal
Akirathan Nov 20, 2024
abc32ad
Fix MockMiniPass - stopExpression is not even prepared
Akirathan Nov 20, 2024
806406c
ImportSymbolAnalysis skips processing
Akirathan Nov 20, 2024
eaf10e8
Merge branch 'develop' into wip/akirathan/11326-more-mini-passes
Akirathan Nov 20, 2024
edcd10b
Migrate AmbiguousImportsAnalysis to mini pass and to Java
Akirathan Nov 20, 2024
5493586
Add some tests for chained passes
Akirathan Nov 20, 2024
84a02d9
ChainedMiniPass handles null passes
Akirathan Nov 21, 2024
e0eba27
Fix compilation error in PasesTest
Akirathan Nov 21, 2024
7a047bb
Merge branch 'develop' into wip/akirathan/11326-more-mini-passes
Akirathan Nov 21, 2024
a204ed2
Fix NPE
Akirathan Nov 21, 2024
5a74cc1
Fix errors in polyglot imports
Akirathan Nov 21, 2024
92a0725
Remove unnecessary isntancdeof
Akirathan Nov 21, 2024
62cecbb
MiniPassTest does not call PassManager.runModule directly.
Akirathan Nov 21, 2024
e27a59f
Move tests from runtime-compiler to runtime-integration-tests
Akirathan Nov 21, 2024
2b5c76c
MockMiniPass uses builder pattern
Akirathan Nov 21, 2024
3033462
Remove EventRecorder, MockMiniPass is not IRProcessingPass
Akirathan Nov 21, 2024
0a4ac1f
Move MiniPassTraverserTest back into runtime-compiler/Test
Akirathan Nov 21, 2024
6d2f6c6
PassManager.runInline also chains mini passes.
Akirathan Nov 21, 2024
239029a
MiniPassFactory.createForInlineCompilation can return null.
Akirathan Nov 21, 2024
f2687db
Fix IR inconsistency in TypeInferenceTest
Akirathan Nov 22, 2024
6ead57e
Remove unused method from PassManager
Akirathan Nov 22, 2024
2bcdede
ImportSymbolAnalysis does not support inline compilation.
Akirathan Nov 22, 2024
04181f9
Convert PrivateModuleAnalysis to mini pass
Akirathan Nov 22, 2024
7032b4a
Fix spurious errors
Akirathan Nov 22, 2024
303c9fe
Convert PrivateConstructorAnalysis to mini pass
Akirathan Nov 22, 2024
557f356
FIx compilation error in a test
Akirathan Nov 22, 2024
4e6837f
Convert MethodDefinitions to Java
Akirathan Nov 22, 2024
0bb6b5e
FIx compilation error in a test
Akirathan Nov 22, 2024
866ffd9
Fix NPE
Akirathan Nov 22, 2024
1166f02
FIx compilation error in a test
Akirathan Nov 22, 2024
8d9db8f
MetadataStorage.EMPTY is not a global reference
Akirathan Nov 22, 2024
9136a8b
FIx compilation error in a test
Akirathan Nov 22, 2024
a3321fe
MethodDefinitions.lambdaWithNewSelfArg copies arguments
Akirathan Nov 22, 2024
a47798f
Fix copy of static method in MethodDefinitions
Akirathan Nov 25, 2024
6c77612
Ensure consistency between ModuleContext and IR in uncachedParseModule
Akirathan Nov 25, 2024
a1d2270
Better error message in IrToTruffle.generateReExportBindings
Akirathan Nov 25, 2024
b0ce6c4
Fix No_Such_Method error.
Akirathan Nov 26, 2024
e0f3d31
Merge branch 'develop' into wip/akirathan/11326-more-mini-passes
Akirathan Nov 26, 2024
fb3e29d
Fix invalid IR in GlobalNamesTest
Akirathan Nov 26, 2024
808ae22
Convert MethodDefinitions to mini pass
Akirathan Nov 27, 2024
8d68146
FIx compilation of MethodDefinitionsTest
Akirathan Nov 27, 2024
fe7e50a
Remove TODO comment
Akirathan Nov 27, 2024
417843d
Move ScalaConversions to scala-libs-wrapper
Akirathan Nov 28, 2024
273bea8
Move check for null minipasses to the factory method when combining
Akirathan Nov 28, 2024
bd10bf6
ChainedMiniPass can disable itself from processing subtree
Akirathan Nov 28, 2024
b3f750e
Rename variables
Akirathan Nov 28, 2024
cb5f8c1
ImportSymbolAnalysis is final
Akirathan Nov 28, 2024
85ab82f
Typo in docs
Akirathan Nov 28, 2024
3e1ade9
Make PassManager.passes field private
Akirathan Nov 28, 2024
4380b08
Fix: ChainedMiniPass can disable itself from processing subtree
Akirathan Nov 28, 2024
023d1aa
Add mini pass traverser test
Akirathan Nov 28, 2024
f605f16
PassManager.passes field needs to be protected
Akirathan Nov 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -2121,6 +2121,7 @@ lazy val `engine-common` = project
.enablePlugins(JPMSPlugin)
.settings(
frgaalJavaCompilerSetting,
scalaModuleDependencySetting,
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
Test / fork := true,
commands += WithDebugCommand.withDebug,
Test / envVars ++= distributionEnvironmentOverrides,
Expand Down Expand Up @@ -2908,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(
Expand Down Expand Up @@ -3191,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,
Expand Down Expand Up @@ -3230,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
)
)
Expand Down
1 change: 1 addition & 0 deletions engine/common/src/main/java/module-info.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.enso.interpreter.util;
package org.enso.common;

import java.util.List;
import java.util.Optional;
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -15,9 +15,8 @@ private ChainedMiniPass(MiniIRPass firstPass, MiniIRPass secondPass) {
}

static MiniIRPass chain(MiniIRPass firstPass, MiniIRPass secondPass) {
if (firstPass == null) {
return secondPass;
}
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
assert firstPass != null;
assert secondPass != null;
return new ChainedMiniPass(firstPass, secondPass);
}

Expand All @@ -39,13 +38,20 @@ 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);
}

@Override
public String toString() {
return Objects.toString(firstPass) + ":" + Objects.toString(secondPass);
return "{" + firstPass + " + " + secondPass + "}";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -103,10 +105,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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<IR> enqueueSubExpressions(
Collection<MiniPassTraverser> queue, IR ir, MiniIRPass miniPass) {
Expand All @@ -81,7 +81,9 @@ private static List<IR> 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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
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;
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 {
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
public static final ImportSymbolAnalysis INSTANCE = new ImportSymbolAnalysis();

private ImportSymbolAnalysis() {}

@Override
public Seq<? extends IRProcessingPass> precursorPasses() {
List<IRProcessingPass> passes =
List.of(BindingAnalysis$.MODULE$, GenerateMethodBodies$.MODULE$);
return ScalaConversions.seq(passes);
}

@Override
public Seq<? extends IRProcessingPass> invalidatedPasses() {
return ScalaConversions.seq(List.of());
}

@Override
public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) {
var bindingsMap = moduleContext.bindingsAnalysis();
assert bindingsMap != null;
return new Mini(bindingsMap);
}

@Override
public MiniIRPass createForInlineCompilation(InlineContext inlineContext) {
var bindingsMap = inlineContext.bindingsAnalysis();
return new Mini(bindingsMap);
}

private static final class Mini extends MiniIRPass {
private final BindingsMap bindingsMap;

private Mini(BindingsMap bindingsMap) {
this.bindingsMap = bindingsMap;
}

@Override
public Module transformModule(Module moduleIr) {
var newImports = new ArrayList<Import>();
for (var imp : CollectionConverters.asJava(moduleIr.imports())) {
if (imp instanceof Import.Module modImp) {
var encounteredErrors = analyseSymbolsFromImport(modImp);
if (encounteredErrors != null) {
newImports.addAll(encounteredErrors);
continue;
}
}
newImports.add(imp);
}
return moduleIr.copy(
CollectionConverters.asScala(newImports).toList(),
moduleIr.exports(),
moduleIr.bindings(),
moduleIr.isPrivate(),
moduleIr.location(),
moduleIr.passData(),
moduleIr.diagnostics(),
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) {
throw new IllegalStateException("Should not be called - prepare returns null");
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
}

/** Returns list of encountered errors, or null. */
private List<Import> 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()) {
scala.collection.immutable.List<Import> errs =
unresolvedSymbols.map(
unresolvedSym ->
createErrorForUnresolvedSymbol(imp, importedTargets.head(), unresolvedSym));
return CollectionConverters.asJava(errs);
}
}

// 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<Import>();
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 encounteredErrors;
}
}
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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private PrivateModuleAnalysis() {}
@Override
public Seq<IRProcessingPass> precursorPasses() {
List<IRProcessingPass> passes =
List.of(BindingAnalysis$.MODULE$, ImportSymbolAnalysis$.MODULE$);
List.of(BindingAnalysis$.MODULE$, ImportSymbolAnalysis.INSTANCE);
return CollectionConverters.asScala(passes).toList();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Passes(config: CompilerConfig) {
SectionsToBinOp.INSTANCE,
OperatorToFunction,
LambdaShorthandToLambda,
ImportSymbolAnalysis,
ImportSymbolAnalysis.INSTANCE,
AmbiguousImportsAnalysis
) ++ (if (config.privateCheckEnabled) {
List(
Expand Down
Loading
Loading