From 6abf764076610c69a5753371fb5084a907eafcff Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 19 Oct 2024 18:59:23 +0200 Subject: [PATCH 01/38] Recognize if-like Tree.MultiSegmentApp as IfThenElse IR --- .../java/org/enso/compiler/core/TreeToIr.java | 12 ++ .../core/ir/expression/IfThenElse.scala | 152 ++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java index 90dd69ff72d3..4ba0190a7932 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java @@ -25,6 +25,7 @@ import org.enso.compiler.core.ir.expression.Case; import org.enso.compiler.core.ir.expression.Comment; import org.enso.compiler.core.ir.expression.Foreign; +import org.enso.compiler.core.ir.expression.IfThenElse; import org.enso.compiler.core.ir.expression.Operator; import org.enso.compiler.core.ir.expression.Section; import org.enso.compiler.core.ir.expression.errors.Syntax; @@ -956,6 +957,17 @@ yield translateSyntaxError(l.getExpression().getExpression(), sep = "_"; } var fullName = fnName.toString(); + if ("if_then_else".equals(fullName) && args.size() == 3) { + var ifArg = args.apply(2); + var trueArg = args.apply(1); + var falseArg = args.apply(0); + yield new IfThenElse(ifArg.value(), trueArg.value(), falseArg.value(), getIdentifiedLocation(tree), meta()); + } + if ("if_then".equals(fullName) && args.size() == 2) { + var ifArg = args.apply(1); + var trueArg = args.apply(0); + yield new IfThenElse(ifArg.value(), trueArg.value(), null, getIdentifiedLocation(tree), meta()); + } if (fullName.equals(FREEZE_MACRO_IDENTIFIER)) { yield translateExpression(app.getSegments().get(0).getBody(), false); } else if (fullName.equals(SKIP_MACRO_IDENTIFIER)) { diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala new file mode 100644 index 000000000000..5acc160b7de4 --- /dev/null +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala @@ -0,0 +1,152 @@ +package org.enso.compiler.core.ir +package expression + +import org.enso.compiler.core.Implicits.{ShowPassData, ToStringHelper} +import org.enso.compiler.core.{IR, Identifier} +import org.enso.compiler.core.IR.{indentLevel, mkIndent} + +import java.util.UUID + +/** The Enso case expression. */ +case class IfThenElse( + cond: Expression, + trueBranch: Expression, + private val falseBranchOrNull: Expression, + override val identifiedLocation: IdentifiedLocation, + override val passData: MetadataStorage = new MetadataStorage() +) extends Expression + with IRKind.Primitive + with LazyDiagnosticStorage + with LazyId { + + cond.getClass() + trueBranch.getClass() + + def falseBranch(): Option[Expression] = Option(falseBranchOrNull) + + /** Creates a copy of `this`. + * + * @param cond the expression whose value is being matched on + * @param branches the branches of the case expression + * @param isNested if true, the flag indicates that the expr represents a desugared nested case + * @param location the source location that the node corresponds to + * @param passData the pass metadata associated with this node + * @param diagnostics compiler diagnostics for this node + * @param id the identifier for the new node + * @return a copy of `this`, updated with the specified values + */ + def copy( + cond: Expression = cond, + trueBranch: Expression = trueBranch, + falseBranchOrNull: Expression = falseBranchOrNull, + location: Option[IdentifiedLocation] = location, + passData: MetadataStorage = passData, + diagnostics: DiagnosticStorage = diagnostics, + id: UUID @Identifier = id + ): IfThenElse = { + if ( + cond != this.cond + || trueBranch != this.trueBranch + || falseBranchOrNull != this.falseBranchOrNull + || location != this.location + || passData != this.passData + || diagnostics != this.diagnostics + || id != this.id + ) { + val res = new IfThenElse( + cond, + trueBranch, + falseBranchOrNull, + location.orNull, + passData + ) + res.diagnostics = diagnostics + res.id = id + res + } else this + } + + /** @inheritdoc */ + override def mapExpressions( + fn: java.util.function.Function[Expression, Expression] + ): IfThenElse = { + copy( + cond = fn(cond), + trueBranch = fn(trueBranch), + falseBranchOrNull = + if (falseBranchOrNull ne null) fn(falseBranchOrNull) else null + ) + } + + /** @inheritdoc */ + override def setLocation(location: Option[IdentifiedLocation]): IfThenElse = + copy(location = location) + + /** @inheritdoc */ + override def duplicate( + keepLocations: Boolean = true, + keepMetadata: Boolean = true, + keepDiagnostics: Boolean = true, + keepIdentifiers: Boolean = false + ): IfThenElse = { + copy( + cond = cond.duplicate( + keepLocations, + keepMetadata, + keepDiagnostics, + keepIdentifiers + ), + trueBranch = trueBranch.duplicate( + keepLocations, + keepMetadata, + keepDiagnostics, + keepIdentifiers + ), + falseBranchOrNull = + if (falseBranchOrNull ne null) + falseBranchOrNull.duplicate( + keepLocations, + keepMetadata, + keepDiagnostics, + keepIdentifiers + ) + else null, + location = if (keepLocations) location else None, + passData = + if (keepMetadata) passData.duplicate else new MetadataStorage(), + diagnostics = if (keepDiagnostics) diagnosticsCopy else null, + id = if (keepIdentifiers) id else null + ) + } + + /** String representation. */ + override def toString: String = + s""" + |IfThenElse( + |cond = $cond, + |trueBranch = $trueBranch, + |falseBranch = ${falseBranch()}, + |location = $location, + |passData = ${this.showPassData}, + |diagnostics = $diagnostics, + |id = $id + |) + |""".toSingleLine + + /** @inheritdoc */ + override def children: List[IR] = + List(cond, trueBranch) ++ falseBranch().toList + + /** @inheritdoc */ + override def showCode(indent: Int): String = { + val newIndent = indent + indentLevel + val headerStr = + s"if ${cond.showCode(indent)} then\n${trueBranch.showCode(newIndent)}" + val elseStr = + if (falseBranchOrNull ne null) + s"${mkIndent(indent)}else\n${falseBranchOrNull.showCode(newIndent)}" + else "" + s"$headerStr\n$elseStr" + } + +} From f5ca8e0ce54563ef1963834bce3c0ff5b43aae28 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sun, 20 Oct 2024 07:06:06 +0200 Subject: [PATCH 02/38] Converting IfThenElse IR to case statement --- .../pass/desugar/IfThenElseToCase.java | 101 ++++++++++++++++++ .../main/scala/org/enso/compiler/Passes.scala | 1 + .../enso/compiler/test/IfThenElseTest.java | 63 +++++++++++ .../org/enso/compiler/core/ir/Literal.scala | 87 +++++++++++++++ .../node/expression/literal/LiteralNode.java | 10 ++ .../interpreter/runtime/IrToTruffle.scala | 9 ++ 6 files changed, 271 insertions(+) create mode 100644 engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java create mode 100644 engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java new file mode 100644 index 000000000000..c0c4b4eeeb57 --- /dev/null +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java @@ -0,0 +1,101 @@ +package org.enso.compiler.pass.desugar; + +import java.util.List; +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.Literal; +import org.enso.compiler.core.ir.MetadataStorage; +import org.enso.compiler.core.ir.Name; +import org.enso.compiler.core.ir.Pattern; +import org.enso.compiler.core.ir.expression.Case; +import org.enso.compiler.core.ir.expression.IfThenElse; +import org.enso.compiler.pass.IRProcessingPass; +import org.enso.compiler.pass.MiniIRPass; +import org.enso.compiler.pass.MiniPassFactory; +import org.enso.compiler.pass.analyse.DemandAnalysis$; +import scala.Option; +import scala.collection.immutable.Seq; +import scala.jdk.javaapi.CollectionConverters; + +public final class IfThenElseToCase implements MiniPassFactory { + + public static final IfThenElseToCase INSTANCE = new IfThenElseToCase(); + + private IfThenElseToCase() {} + + @Override + public Seq precursorPasses() { + List passes = List.of(GenerateMethodBodies$.MODULE$); + return CollectionConverters.asScala(passes).toList(); + } + + @Override + public Seq invalidatedPasses() { + List passes = List.of(DemandAnalysis$.MODULE$); + return CollectionConverters.asScala(passes).toList(); + } + + @Override + public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { + var ctx = InlineContext.fromModuleContext(moduleContext); + return new Mini(ctx); + } + + @Override + public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { + return new Mini(inlineContext); + } + + private static final class Mini extends MiniIRPass { + + private final InlineContext ctx; + + private Mini(InlineContext ctx) { + this.ctx = ctx; + } + + @Override + public Expression transformExpression(Expression ir) { + return switch (ir) { + case IfThenElse expr -> { + var condArg = expr.cond(); + var trueAt = expr.trueBranch().identifiedLocation(); + var falseAt = orNull(expr.falseBranch().map(b -> b.identifiedLocation())); + var truePattern = + new Pattern.Literal(new Literal.Bool(true, null, meta()), trueAt, meta()); + var trueBranch = new Case.Branch(truePattern, expr.trueBranch(), true, trueAt, meta()); + var branches = Mini.nil(); + if (expr.falseBranch().nonEmpty()) { + var falsePattern = new Pattern.Name(new Name.Blank(null, meta()), falseAt, meta()); + var falseBranch = + new Case.Branch(falsePattern, orNull(expr.falseBranch()), true, falseAt, meta()); + branches = join(falseBranch, nil()); + } + branches = join(trueBranch, branches); + var caseExpr = new Case.Expr(condArg, branches, false, expr.identifiedLocation(), meta()); + yield caseExpr; + } + default -> ir; + }; + } + + @SuppressWarnings("unchecked") + private static final scala.collection.immutable.List nil() { + return (scala.collection.immutable.List) scala.collection.immutable.Nil$.MODULE$; + } + + private static final scala.collection.immutable.List join( + T head, scala.collection.immutable.List tail) { + return scala.collection.immutable.$colon$colon$.MODULE$.apply(head, tail); + } + + private static T orNull(Option opt) { + return opt.isEmpty() ? null : opt.get(); + } + + private static MetadataStorage meta() { + return new MetadataStorage(); + } + } +} 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..5cdb145b2d74 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,6 +44,7 @@ class Passes(config: CompilerConfig) { SectionsToBinOp.INSTANCE, OperatorToFunction, LambdaShorthandToLambda, + IfThenElseToCase.INSTANCE, ImportSymbolAnalysis, AmbiguousImportsAnalysis ) ++ (if (config.privateCheckEnabled) { diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java new file mode 100644 index 000000000000..5a18a1926d8b --- /dev/null +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java @@ -0,0 +1,63 @@ +package org.enso.compiler.test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import java.io.ByteArrayOutputStream; +import java.nio.file.Paths; +import java.util.logging.Level; +import org.enso.common.MethodNames; +import org.enso.common.RuntimeOptions; +import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.io.IOAccess; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +public class IfThenElseTest { + private static Context ctx; + private static final ByteArrayOutputStream MESSAGES = new ByteArrayOutputStream(); + + @BeforeClass + public static void initEnsoContext() { + ctx = + Context.newBuilder() + .allowExperimentalOptions(true) + .allowIO(IOAccess.ALL) + .option( + RuntimeOptions.LANGUAGE_HOME_OVERRIDE, + Paths.get("../../distribution/component").toFile().getAbsolutePath()) + .option(RuntimeOptions.STRICT_ERRORS, "true") + .option(RuntimeOptions.LOG_LEVEL, Level.WARNING.getName()) + .logHandler(System.err) + .out(MESSAGES) + .err(MESSAGES) + .allowAllAccess(true) + .build(); + assertNotNull("Enso language is supported", ctx.getEngine().getLanguages().get("enso")); + } + + @After + public void cleanMessages() { + MESSAGES.reset(); + } + + @AfterClass + public static void closeEnsoContext() { + ctx.close(); + ctx = null; + } + + @Test + public void simpleIfThenElse() throws Exception { + var module = ctx.eval("enso", """ + check x = if x then "Yes" else "No" + """); + + var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + + assertEquals("Yes", check.execute(true).asString()); + assertEquals("No", check.execute(false).asString()); + } +} diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala index 52d41f2ca739..2d6f651036cd 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala @@ -250,4 +250,91 @@ object Literal { /** @inheritdoc */ override def showCode(indent: Int): String = s""""$text"""" } + + /** A boolean Enso literal. + * + * @param bool the value of the literal + * @param identifiedLocation the source location that the node corresponds to + * @param passData the pass metadata associated with this node + */ + sealed case class Bool( + bool: Boolean, + override val identifiedLocation: IdentifiedLocation, + override val passData: MetadataStorage = new MetadataStorage() + ) extends Literal + with LazyDiagnosticStorage + with LazyId { + + /** Creates a copy of `this`. + * + * @param bool the boolean value of the literal + * @param location the source location that the node corresponds to + * @param passData the pass metadata associated with this node + * @param diagnostics compiler diagnostics for this node + * @param id the identifier for the new node + * @return a copy of `this`, updated with the specified values + */ + def copy( + bool: Boolean = bool, + location: Option[IdentifiedLocation] = location, + passData: MetadataStorage = passData, + diagnostics: DiagnosticStorage = diagnostics, + id: UUID @Identifier = id + ): Bool = { + if ( + bool != this.bool + || location != this.location + || passData != this.passData + || diagnostics != this.diagnostics + || id != this.id + ) { + val res = Bool(bool, location.orNull, passData) + res.diagnostics = diagnostics + res.id = id + res + } else this + } + + /** @inheritdoc */ + override def duplicate( + keepLocations: Boolean = true, + keepMetadata: Boolean = true, + keepDiagnostics: Boolean = true, + keepIdentifiers: Boolean = false + ): Bool = + copy( + location = if (keepLocations) location else None, + passData = + if (keepMetadata) passData.duplicate else new MetadataStorage(), + diagnostics = if (keepDiagnostics) diagnosticsCopy else null, + id = if (keepIdentifiers) id else null + ) + + /** @inheritdoc */ + override def setLocation(location: Option[IdentifiedLocation]): Bool = + copy(location = location) + + /** @inheritdoc */ + override def mapExpressions( + fn: java.util.function.Function[Expression, Expression] + ): Bool = this + + /** String representation. */ + override def toString: String = + s""" + |Literal.Bool( + |bool = $bool, + |location = $location, + |passData = ${this.showPassData}, + |diagnostics = $diagnostics, + |id = $id + |) + |""".toSingleLine + + /** @inheritdoc */ + override def children: List[IR] = List() + + /** @inheritdoc */ + override def showCode(indent: Int): String = s"$bool" + } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/literal/LiteralNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/literal/LiteralNode.java index 69a3861a6a0d..397589357d73 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/literal/LiteralNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/literal/LiteralNode.java @@ -51,6 +51,16 @@ public static LiteralNode build(double value) { return new LiteralNode(value); } + /** + * Creates an instance of the literal node. + * + * @param value boolean value for the node to represent + * @return a node representing the literal given by {@code value} + */ + public static LiteralNode build(boolean value) { + return new LiteralNode(value); + } + /** * Creates an instance of the literal node. * 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 f2d24d34da3d..df2bacad090b 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 @@ -1731,6 +1731,14 @@ class IrToTruffle( branch.terminalBranch ) ) + case bool: Literal.Bool => + Right( + BooleanBranchNode.build( + bool.bool, + branchCodeNode.getCallTarget, + branch.terminalBranch + ) + ) } case typePattern: Pattern.Type => typePattern.tpe.getMetadata(Patterns) match { @@ -2095,6 +2103,7 @@ class IrToTruffle( setLocation(node, lit.location) case lit: Literal.Text => setLocation(LiteralNode.build(lit.text), lit.location) + case bool: Literal.Bool => LiteralNode.build(bool.bool) } private def fileLocationFromSection(loc: IdentifiedLocation) = { From f9987086543ef54dc569b0d478b46c81b3940aff Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sun, 20 Oct 2024 07:18:09 +0200 Subject: [PATCH 03/38] Support if_then without else --- .../compiler/pass/desugar/IfThenElseToCase.java | 14 ++++++-------- .../org/enso/compiler/test/IfThenElseTest.java | 13 +++++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java index c0c4b4eeeb57..70c24376c201 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java @@ -3,6 +3,7 @@ import java.util.List; import org.enso.compiler.context.InlineContext; import org.enso.compiler.context.ModuleContext; +import org.enso.compiler.core.ir.Empty; import org.enso.compiler.core.ir.Expression; import org.enso.compiler.core.ir.Literal; import org.enso.compiler.core.ir.MetadataStorage; @@ -65,14 +66,11 @@ public Expression transformExpression(Expression ir) { var truePattern = new Pattern.Literal(new Literal.Bool(true, null, meta()), trueAt, meta()); var trueBranch = new Case.Branch(truePattern, expr.trueBranch(), true, trueAt, meta()); - var branches = Mini.nil(); - if (expr.falseBranch().nonEmpty()) { - var falsePattern = new Pattern.Name(new Name.Blank(null, meta()), falseAt, meta()); - var falseBranch = - new Case.Branch(falsePattern, orNull(expr.falseBranch()), true, falseAt, meta()); - branches = join(falseBranch, nil()); - } - branches = join(trueBranch, branches); + var falseCode = + expr.falseBranch().nonEmpty() ? expr.falseBranch().get() : new Empty(null, meta()); + var falsePattern = new Pattern.Name(new Name.Blank(null, meta()), falseAt, meta()); + var falseBranch = new Case.Branch(falsePattern, falseCode, true, falseAt, meta()); + var branches = join(trueBranch, join(falseBranch, nil())); var caseExpr = new Case.Expr(condArg, branches, false, expr.identifiedLocation(), meta()); yield caseExpr; } diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java index 5a18a1926d8b..216e87ef6580 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import java.io.ByteArrayOutputStream; import java.nio.file.Paths; @@ -60,4 +61,16 @@ public void simpleIfThenElse() throws Exception { assertEquals("Yes", check.execute(true).asString()); assertEquals("No", check.execute(false).asString()); } + + @Test + public void simpleIfThen() throws Exception { + var module = ctx.eval("enso", """ + check x = if x then "Yes" + """); + + var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + + assertEquals("Yes", check.execute(true).asString()); + assertTrue("Expect Nothing", check.execute(false).isNull()); + } } From fba42c5a475360bb2c17c49d25215afc610e03df Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sun, 20 Oct 2024 08:07:18 +0200 Subject: [PATCH 04/38] Register Literal.Bool and Empty for persistance --- .../org/enso/compiler/core/ir/IrPersistance.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java index 4dd42faf9462..9da1205046c0 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java @@ -39,12 +39,14 @@ @Persistable(clazz = Name.Self.class, id = 705) @Persistable(clazz = Literal.Number.class, id = 706) @Persistable(clazz = Literal.Text.class, id = 707) -@Persistable(clazz = CallArgument.Specified.class, id = 708) -@Persistable(clazz = Definition.Type.class, id = 709) -@Persistable(clazz = Definition.Data.class, id = 710) -@Persistable(clazz = Name.Blank.class, id = 711) -@Persistable(clazz = Name.GenericAnnotation.class, id = 712) -@Persistable(clazz = Name.SelfType.class, id = 713) +@Persistable(clazz = Literal.Bool.class, id = 708) +@Persistable(clazz = CallArgument.Specified.class, id = 711) +@Persistable(clazz = Definition.Type.class, id = 712) +@Persistable(clazz = Definition.Data.class, id = 713) +@Persistable(clazz = Name.Blank.class, id = 714) +@Persistable(clazz = Empty.class, id = 715) +@Persistable(clazz = Name.GenericAnnotation.class, id = 716) +@Persistable(clazz = Name.SelfType.class, id = 717) @Persistable(clazz = Expression.Block.class, id = 751) @Persistable(clazz = Expression.Binding.class, id = 752) @Persistable(clazz = Application.Prefix.class, id = 753) From 7c7c5b38000cc2f5eec1708bab14cee0da130c2f Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sun, 20 Oct 2024 08:12:46 +0200 Subject: [PATCH 05/38] Note in changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79a4ac58429d..c89d7bf12c5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,12 @@ [11235]: https://github.com/enso-org/enso/pull/11235 [11255]: https://github.com/enso-org/enso/pull/11255 +#### Enso Language & Runtime + +- [Unify internal handling of if/then/else and case statement][11365] + +[11365]: https://github.com/enso-org/enso/pull/11365 + # Enso 2024.4 #### Enso IDE From 94b07b47487a2e3b940496d2260f9d98e0f306a7 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sun, 20 Oct 2024 08:59:50 +0200 Subject: [PATCH 06/38] Handle missing else branch without NPE --- .../src/main/java/org/enso/compiler/core/TreeToIr.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java index 4ba0190a7932..809734a7af1a 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java @@ -961,6 +961,9 @@ yield translateSyntaxError(l.getExpression().getExpression(), var ifArg = args.apply(2); var trueArg = args.apply(1); var falseArg = args.apply(0); + if (falseArg == null) { + yield translateSyntaxError(app, new Syntax.UnsupportedSyntax("Missing else branch")); + } yield new IfThenElse(ifArg.value(), trueArg.value(), falseArg.value(), getIdentifiedLocation(tree), meta()); } if ("if_then".equals(fullName) && args.size() == 2) { From f6f0938c4e95333604ad6e0427c8d41ad3757022 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 24 Oct 2024 10:47:56 +0200 Subject: [PATCH 07/38] Testing behavior of a variable being defined in branch --- .../enso/compiler/test/IfThenElseTest.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java index 216e87ef6580..823ffb0d5bc4 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java @@ -73,4 +73,37 @@ public void simpleIfThen() throws Exception { assertEquals("Yes", check.execute(true).asString()); assertTrue("Expect Nothing", check.execute(false).isNull()); } + + @Test + public void variableDefinedInThen() throws Exception { + var module = + ctx.eval( + "enso", """ + check x = if x then + xt = x.to_text + "Good:"+xt + """); + + var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + + assertEquals("Good:True", check.execute(true).asString()); + assertTrue("Expect Nothing", check.execute(false).isNull()); + } + + @Test + public void variableDefinedInElse() throws Exception { + var module = + ctx.eval( + "enso", + """ + check x = if x then "OKeyish:"+x.to_text else + xt = x.to_text + "Bad:"+xt + """); + + var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + + assertEquals("OKeyish:True", check.execute(true).asString()); + assertEquals("Bad:False", check.execute(false).asString()); + } } From 3a2906b02650d3c44d3115d36ebc7f7ce647641d Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 24 Oct 2024 10:54:12 +0200 Subject: [PATCH 08/38] Variable cannot be use after the if branch is over --- .../enso/compiler/test/IfThenElseTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java index 823ffb0d5bc4..c2ba001e24e6 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java @@ -1,8 +1,10 @@ package org.enso.compiler.test; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayOutputStream; import java.nio.file.Paths; @@ -10,7 +12,10 @@ import org.enso.common.MethodNames; import org.enso.common.RuntimeOptions; import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.PolyglotException; import org.graalvm.polyglot.io.IOAccess; +import org.hamcrest.Matchers; +import org.hamcrest.core.AllOf; import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -106,4 +111,30 @@ public void variableDefinedInElse() throws Exception { assertEquals("OKeyish:True", check.execute(true).asString()); assertEquals("Bad:False", check.execute(false).asString()); } + + @Test + public void variableUsedAfterTheBranch() throws Exception { + try { + var module = + ctx.eval( + "enso", + """ + check x = + res = if x then "OKeyish:"+x.to_text else + xt = x.to_text + "Bad:"+xt + + xt + """); + + var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + fail("Expecting error, but got: " + check); + } catch (PolyglotException ex) { + assertThat( + ex.getMessage(), + AllOf.allOf( + Matchers.containsString("The name `xt` could not be found"), + Matchers.containsString("6:5: error"))); + } + } } From ffab9384640a1f3a89621e77d07002aa93596980 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 25 Oct 2024 10:41:56 +0200 Subject: [PATCH 09/38] Removing changes related to IfThenElseToCase IR conversions --- .../pass/desugar/IfThenElseToCase.java | 99 ------- .../main/scala/org/enso/compiler/Passes.scala | 1 - .../enso/compiler/test/IfThenElseTest.java | 245 +++++++++++++++--- .../enso/compiler/core/ir/IrPersistance.java | 14 +- .../org/enso/compiler/core/ir/Literal.scala | 87 ------- .../interpreter/runtime/IrToTruffle.scala | 9 - 6 files changed, 208 insertions(+), 247 deletions(-) delete mode 100644 engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java deleted file mode 100644 index 70c24376c201..000000000000 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/desugar/IfThenElseToCase.java +++ /dev/null @@ -1,99 +0,0 @@ -package org.enso.compiler.pass.desugar; - -import java.util.List; -import org.enso.compiler.context.InlineContext; -import org.enso.compiler.context.ModuleContext; -import org.enso.compiler.core.ir.Empty; -import org.enso.compiler.core.ir.Expression; -import org.enso.compiler.core.ir.Literal; -import org.enso.compiler.core.ir.MetadataStorage; -import org.enso.compiler.core.ir.Name; -import org.enso.compiler.core.ir.Pattern; -import org.enso.compiler.core.ir.expression.Case; -import org.enso.compiler.core.ir.expression.IfThenElse; -import org.enso.compiler.pass.IRProcessingPass; -import org.enso.compiler.pass.MiniIRPass; -import org.enso.compiler.pass.MiniPassFactory; -import org.enso.compiler.pass.analyse.DemandAnalysis$; -import scala.Option; -import scala.collection.immutable.Seq; -import scala.jdk.javaapi.CollectionConverters; - -public final class IfThenElseToCase implements MiniPassFactory { - - public static final IfThenElseToCase INSTANCE = new IfThenElseToCase(); - - private IfThenElseToCase() {} - - @Override - public Seq precursorPasses() { - List passes = List.of(GenerateMethodBodies$.MODULE$); - return CollectionConverters.asScala(passes).toList(); - } - - @Override - public Seq invalidatedPasses() { - List passes = List.of(DemandAnalysis$.MODULE$); - return CollectionConverters.asScala(passes).toList(); - } - - @Override - public MiniIRPass createForModuleCompilation(ModuleContext moduleContext) { - var ctx = InlineContext.fromModuleContext(moduleContext); - return new Mini(ctx); - } - - @Override - public MiniIRPass createForInlineCompilation(InlineContext inlineContext) { - return new Mini(inlineContext); - } - - private static final class Mini extends MiniIRPass { - - private final InlineContext ctx; - - private Mini(InlineContext ctx) { - this.ctx = ctx; - } - - @Override - public Expression transformExpression(Expression ir) { - return switch (ir) { - case IfThenElse expr -> { - var condArg = expr.cond(); - var trueAt = expr.trueBranch().identifiedLocation(); - var falseAt = orNull(expr.falseBranch().map(b -> b.identifiedLocation())); - var truePattern = - new Pattern.Literal(new Literal.Bool(true, null, meta()), trueAt, meta()); - var trueBranch = new Case.Branch(truePattern, expr.trueBranch(), true, trueAt, meta()); - var falseCode = - expr.falseBranch().nonEmpty() ? expr.falseBranch().get() : new Empty(null, meta()); - var falsePattern = new Pattern.Name(new Name.Blank(null, meta()), falseAt, meta()); - var falseBranch = new Case.Branch(falsePattern, falseCode, true, falseAt, meta()); - var branches = join(trueBranch, join(falseBranch, nil())); - var caseExpr = new Case.Expr(condArg, branches, false, expr.identifiedLocation(), meta()); - yield caseExpr; - } - default -> ir; - }; - } - - @SuppressWarnings("unchecked") - private static final scala.collection.immutable.List nil() { - return (scala.collection.immutable.List) scala.collection.immutable.Nil$.MODULE$; - } - - private static final scala.collection.immutable.List join( - T head, scala.collection.immutable.List tail) { - return scala.collection.immutable.$colon$colon$.MODULE$.apply(head, tail); - } - - private static T orNull(Option opt) { - return opt.isEmpty() ? null : opt.get(); - } - - private static MetadataStorage meta() { - return new MetadataStorage(); - } - } -} 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 5cdb145b2d74..acdf7361e1e6 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,6 @@ class Passes(config: CompilerConfig) { SectionsToBinOp.INSTANCE, OperatorToFunction, LambdaShorthandToLambda, - IfThenElseToCase.INSTANCE, ImportSymbolAnalysis, AmbiguousImportsAnalysis ) ++ (if (config.privateCheckEnabled) { diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java index c2ba001e24e6..0a8896377183 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java @@ -6,19 +6,21 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.interop.TruffleObject; +import com.oracle.truffle.api.library.ExportLibrary; +import com.oracle.truffle.api.library.ExportMessage; import java.io.ByteArrayOutputStream; -import java.nio.file.Paths; -import java.util.logging.Level; -import org.enso.common.MethodNames; -import org.enso.common.RuntimeOptions; +import org.enso.test.utils.ContextUtils; import org.graalvm.polyglot.Context; import org.graalvm.polyglot.PolyglotException; -import org.graalvm.polyglot.io.IOAccess; +import org.graalvm.polyglot.Value; import org.hamcrest.Matchers; import org.hamcrest.core.AllOf; import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; public class IfThenElseTest { @@ -27,20 +29,7 @@ public class IfThenElseTest { @BeforeClass public static void initEnsoContext() { - ctx = - Context.newBuilder() - .allowExperimentalOptions(true) - .allowIO(IOAccess.ALL) - .option( - RuntimeOptions.LANGUAGE_HOME_OVERRIDE, - Paths.get("../../distribution/component").toFile().getAbsolutePath()) - .option(RuntimeOptions.STRICT_ERRORS, "true") - .option(RuntimeOptions.LOG_LEVEL, Level.WARNING.getName()) - .logHandler(System.err) - .out(MESSAGES) - .err(MESSAGES) - .allowAllAccess(true) - .build(); + ctx = ContextUtils.defaultContextBuilder().out(MESSAGES).err(MESSAGES).build(); assertNotNull("Enso language is supported", ctx.getEngine().getLanguages().get("enso")); } @@ -57,11 +46,11 @@ public static void closeEnsoContext() { @Test public void simpleIfThenElse() throws Exception { - var module = ctx.eval("enso", """ + var code = """ check x = if x then "Yes" else "No" - """); + """; - var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); assertEquals("Yes", check.execute(true).asString()); assertEquals("No", check.execute(false).asString()); @@ -69,11 +58,11 @@ public void simpleIfThenElse() throws Exception { @Test public void simpleIfThen() throws Exception { - var module = ctx.eval("enso", """ + var code = """ check x = if x then "Yes" - """); + """; - var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); assertEquals("Yes", check.execute(true).asString()); assertTrue("Expect Nothing", check.execute(false).isNull()); @@ -81,15 +70,13 @@ public void simpleIfThen() throws Exception { @Test public void variableDefinedInThen() throws Exception { - var module = - ctx.eval( - "enso", """ + var code = """ check x = if x then xt = x.to_text "Good:"+xt - """); + """; - var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); assertEquals("Good:True", check.execute(true).asString()); assertTrue("Expect Nothing", check.execute(false).isNull()); @@ -97,16 +84,13 @@ public void variableDefinedInThen() throws Exception { @Test public void variableDefinedInElse() throws Exception { - var module = - ctx.eval( - "enso", - """ + var code = + """ check x = if x then "OKeyish:"+x.to_text else xt = x.to_text "Bad:"+xt - """); - - var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + """; + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); assertEquals("OKeyish:True", check.execute(true).asString()); assertEquals("Bad:False", check.execute(false).asString()); @@ -115,19 +99,17 @@ public void variableDefinedInElse() throws Exception { @Test public void variableUsedAfterTheBranch() throws Exception { try { - var module = - ctx.eval( - "enso", - """ + var code = + """ check x = res = if x then "OKeyish:"+x.to_text else xt = x.to_text "Bad:"+xt xt - """); + """; - var check = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "check"); + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); fail("Expecting error, but got: " + check); } catch (PolyglotException ex) { assertThat( @@ -137,4 +119,181 @@ public void variableUsedAfterTheBranch() throws Exception { Matchers.containsString("6:5: error"))); } } + + @Test + public void conditionMustBeBoolean() throws Exception { + var code = """ + check x = if x then "Yes" else "No" + """; + + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + + try { + var res = check.execute("Yes").asString(); + fail("Expecting error, not: " + res); + } catch (PolyglotException ex) { + assertThat(ex.getMessage(), Matchers.containsString(".Error")); + } + try { + var res = check.execute((Object) null).asString(); + fail("Expecting error, not: " + res); + } catch (PolyglotException ex) { + assertThat(ex.getMessage(), Matchers.containsString(".Error")); + } + } + + @Test + public void javaScriptBooleanIsSupported() throws Exception { + var code = + """ + foreign js toBool txt = ''' + if (txt == "Ano") return true; + if (txt == "Ne") return false; + throw "What do you mean by: " + txt; + + check x = if toBool x then "Yes" else "No" + """; + + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + + assertEquals("Yes", check.execute("Ano").asString()); + assertEquals("No", check.execute("Ne").asString()); + } + + @Ignore + @Test + public void truffleObjectConvertibleToBooleanIsSupported() throws Exception { + var code = + """ + from Standard.Base import all + + check x = if x then "Yes" else "No" + """; + + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + + var t = new BoolObject(true); + var f = new BoolObject(false); + + assertEquals("Yes", check.execute(t).asString()); + assertEquals("No", check.execute(f).asString()); + } + + @ExportLibrary(InteropLibrary.class) + static final class BoolObject implements TruffleObject { + private final boolean value; + + public BoolObject(boolean value) { + this.value = value; + } + + @ExportMessage + boolean isBoolean() { + return true; + } + + @ExportMessage + boolean asBoolean() { + return value; + } + } + + @Test + public void warningsAndIfThenElse() throws Exception { + var code = + """ + from Standard.Base import all + + check x = if x then "Yes" else "No" + """; + + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + + var warnCode = """ + from Standard.Base import all + + warn w v = Warning.attach w v + """; + var warn = ContextUtils.getMethodFromModule(ctx, warnCode, "warn"); + + var t = warn.execute("Maybe", true); + var f = warn.execute("Maybe not", false); + + var yes = check.execute(t); + var no = check.execute(f); + + assertEquals("Yes", yes.asString()); + assertWarning("Maybe", yes); + assertEquals("No", no.asString()); + assertWarning("Maybe not", no); + } + + @Test + public void warningsInThenOrElse() throws Exception { + var code = """ + from Standard.Base import all + + check x y n = if x then y else n + """; + + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + + var warnCode = """ + from Standard.Base import all + + warn w v = Warning.attach w v + """; + var warn = ContextUtils.getMethodFromModule(ctx, warnCode, "warn"); + + var y = warn.execute("Good", "Yes"); + var n = warn.execute("Bad", "No"); + + var yes = check.execute(true, y, n); + var no = check.execute(false, y, n); + + assertEquals("Yes", yes.asString()); + assertWarning("Good", yes); + assertEquals("No", no.asString()); + assertWarning("Bad", no); + } + + @Test + public void warningsInCondAndThenOrElse() throws Exception { + var code = """ + from Standard.Base import all + + check x y n = if x then y else n + """; + + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + + var warnCode = """ + from Standard.Base import all + + warn w v = Warning.attach w v + """; + var warn = ContextUtils.getMethodFromModule(ctx, warnCode, "warn"); + + var y = warn.execute("Good", "Yes"); + var n = warn.execute("Bad", "No"); + var t = warn.execute("Maybe", true); + var f = warn.execute("Maybe not", false); + + var yes = check.execute(t, y, n); + var no = check.execute(f, y, n); + + assertEquals("Yes", yes.asString()); + assertWarning("GoodMaybe", yes); + assertEquals("No", no.asString()); + assertWarning("BadMaybe not", no); + } + + private static void assertWarning(String txt, Value v) { + assertTrue("Value " + v + " should be an exceptional", v.isException()); + try { + throw v.throwException(); + } catch (PolyglotException ex) { + assertEquals(txt, ex.getMessage()); + } + } } diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java index 9da1205046c0..4dd42faf9462 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java @@ -39,14 +39,12 @@ @Persistable(clazz = Name.Self.class, id = 705) @Persistable(clazz = Literal.Number.class, id = 706) @Persistable(clazz = Literal.Text.class, id = 707) -@Persistable(clazz = Literal.Bool.class, id = 708) -@Persistable(clazz = CallArgument.Specified.class, id = 711) -@Persistable(clazz = Definition.Type.class, id = 712) -@Persistable(clazz = Definition.Data.class, id = 713) -@Persistable(clazz = Name.Blank.class, id = 714) -@Persistable(clazz = Empty.class, id = 715) -@Persistable(clazz = Name.GenericAnnotation.class, id = 716) -@Persistable(clazz = Name.SelfType.class, id = 717) +@Persistable(clazz = CallArgument.Specified.class, id = 708) +@Persistable(clazz = Definition.Type.class, id = 709) +@Persistable(clazz = Definition.Data.class, id = 710) +@Persistable(clazz = Name.Blank.class, id = 711) +@Persistable(clazz = Name.GenericAnnotation.class, id = 712) +@Persistable(clazz = Name.SelfType.class, id = 713) @Persistable(clazz = Expression.Block.class, id = 751) @Persistable(clazz = Expression.Binding.class, id = 752) @Persistable(clazz = Application.Prefix.class, id = 753) diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala index 2d6f651036cd..52d41f2ca739 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Literal.scala @@ -250,91 +250,4 @@ object Literal { /** @inheritdoc */ override def showCode(indent: Int): String = s""""$text"""" } - - /** A boolean Enso literal. - * - * @param bool the value of the literal - * @param identifiedLocation the source location that the node corresponds to - * @param passData the pass metadata associated with this node - */ - sealed case class Bool( - bool: Boolean, - override val identifiedLocation: IdentifiedLocation, - override val passData: MetadataStorage = new MetadataStorage() - ) extends Literal - with LazyDiagnosticStorage - with LazyId { - - /** Creates a copy of `this`. - * - * @param bool the boolean value of the literal - * @param location the source location that the node corresponds to - * @param passData the pass metadata associated with this node - * @param diagnostics compiler diagnostics for this node - * @param id the identifier for the new node - * @return a copy of `this`, updated with the specified values - */ - def copy( - bool: Boolean = bool, - location: Option[IdentifiedLocation] = location, - passData: MetadataStorage = passData, - diagnostics: DiagnosticStorage = diagnostics, - id: UUID @Identifier = id - ): Bool = { - if ( - bool != this.bool - || location != this.location - || passData != this.passData - || diagnostics != this.diagnostics - || id != this.id - ) { - val res = Bool(bool, location.orNull, passData) - res.diagnostics = diagnostics - res.id = id - res - } else this - } - - /** @inheritdoc */ - override def duplicate( - keepLocations: Boolean = true, - keepMetadata: Boolean = true, - keepDiagnostics: Boolean = true, - keepIdentifiers: Boolean = false - ): Bool = - copy( - location = if (keepLocations) location else None, - passData = - if (keepMetadata) passData.duplicate else new MetadataStorage(), - diagnostics = if (keepDiagnostics) diagnosticsCopy else null, - id = if (keepIdentifiers) id else null - ) - - /** @inheritdoc */ - override def setLocation(location: Option[IdentifiedLocation]): Bool = - copy(location = location) - - /** @inheritdoc */ - override def mapExpressions( - fn: java.util.function.Function[Expression, Expression] - ): Bool = this - - /** String representation. */ - override def toString: String = - s""" - |Literal.Bool( - |bool = $bool, - |location = $location, - |passData = ${this.showPassData}, - |diagnostics = $diagnostics, - |id = $id - |) - |""".toSingleLine - - /** @inheritdoc */ - override def children: List[IR] = List() - - /** @inheritdoc */ - override def showCode(indent: Int): String = s"$bool" - } } 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 df2bacad090b..f2d24d34da3d 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 @@ -1731,14 +1731,6 @@ class IrToTruffle( branch.terminalBranch ) ) - case bool: Literal.Bool => - Right( - BooleanBranchNode.build( - bool.bool, - branchCodeNode.getCallTarget, - branch.terminalBranch - ) - ) } case typePattern: Pattern.Type => typePattern.tpe.getMetadata(Patterns) match { @@ -2103,7 +2095,6 @@ class IrToTruffle( setLocation(node, lit.location) case lit: Literal.Text => setLocation(LiteralNode.build(lit.text), lit.location) - case bool: Literal.Bool => LiteralNode.build(bool.bool) } private def fileLocationFromSection(loc: IdentifiedLocation) = { From 382a0dff776f2d42263626500f21fefe69f5fff8 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 25 Oct 2024 11:32:17 +0200 Subject: [PATCH 10/38] Processing IfThenElse IR via controlflow.IfThenElseNode --- .../pass/analyse/DataflowAnalysis.scala | 21 +++++++++ .../pass/analyse/DemandAnalysis.scala | 22 +++++++++ .../core/ir/expression/IfThenElse.scala | 12 +++-- .../node/controlflow/IfThenElseNode.java | 47 +++++++++++++++++++ ...IfThenNode.java => IfThenBuiltinNode.java} | 6 +-- ...seNode.java => IfThenElseBuiltinNode.java} | 2 +- .../interpreter/runtime/IrToTruffle.scala | 19 +++++++- 7 files changed, 121 insertions(+), 8 deletions(-) create mode 100644 engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java rename engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/{IfThenNode.java => IfThenBuiltinNode.java} (90%) rename engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/{IfThenElseNode.java => IfThenElseBuiltinNode.java} (96%) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DataflowAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DataflowAnalysis.scala index 2147865f8c94..5aea4de90a78 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DataflowAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DataflowAnalysis.scala @@ -12,6 +12,7 @@ import org.enso.compiler.core.ir.expression.{ Comment, Error, Foreign, + IfThenElse, Operator } import org.enso.compiler.core.ir.{ @@ -244,6 +245,7 @@ case object DataflowAnalysis extends IRPass { case app: Application => analyseApplication(app, info) case typ: Type => analyseType(typ, info) case name: Name => analyseName(name, info) + case ife: IfThenElse => analyseIfThenElse(ife, info) case cse: Case => analyseCase(cse, info) case literal: Literal => literal.updateMetadata(new MetadataPair(this, info)) @@ -580,6 +582,25 @@ case object DataflowAnalysis extends IRPass { } } + /** Performs dependency analysis on a if then else expression. + * + * The value of a if expression is dependent on both its condition and the + * definitions of its branches. The computation of the branches also depends + * on the condition. + * + * @param ife the expression + * @param info the dependency information for the module + * @return `ife`, with attached dependency information + */ + private def analyseIfThenElse( + ife: IfThenElse, + info: DependencyInfo + ): IfThenElse = { + // TBD + info.getClass() + ife + } + /** Performs dependency analysis on a case expression. * * The value of a case expression is dependent on both its scrutinee and the diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DemandAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DemandAnalysis.scala index e59033695589..e7a45d54ea06 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DemandAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DemandAnalysis.scala @@ -20,6 +20,7 @@ import org.enso.compiler.core.ir.expression.{ Comment, Error, Foreign, + IfThenElse, Operator } import org.enso.compiler.core.CompilerError @@ -114,6 +115,8 @@ case object DemandAnalysis extends IRPass { analyseApplication(app, isInsideCallArgument) case typ: Type => analyseType(typ, isInsideCallArgument) + case ite: IfThenElse => + analyseIfThenElse(ite, isInsideCallArgument) case cse: Case => analyseCase(cse, isInsideCallArgument) case block @ Expression.Block(expressions, retVal, _, _, _) => @@ -309,6 +312,25 @@ case object DemandAnalysis extends IRPass { ): Type = typ.mapExpressions(x => analyseExpression(x, isInsideCallArgument)) + /** Performs demand analysis on an if/then/else expression. + * + * @param ife the expression + * @param isInsideCallArgument whether expression occurs inside a + * function call argument + * @return `cse`, transformed by the demand analysis process + */ + private def analyseIfThenElse( + ife: IfThenElse, + isInsideCallArgument: Boolean + ): IfThenElse = { + ife.copy( + cond = analyseExpression(ife.cond, isInsideCallArgument), + trueBranch = analyseExpression(ife.trueBranch, false), + falseBranchOrNull = + ife.falseBranch.map(analyseExpression(_, false)).orNull + ) + } + /** Performs demand analysis on a case expression. * * @param cse the case expression to perform demand analysis on diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala index 5acc160b7de4..04b489b83019 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala @@ -19,10 +19,16 @@ case class IfThenElse( with LazyDiagnosticStorage with LazyId { + // checks for non-null cond.getClass() trueBranch.getClass() - def falseBranch(): Option[Expression] = Option(falseBranchOrNull) + /** Represents the "false branch" is an option to allow Scala + * like manipulation with `None` or `Some`. + * + * @return option derived from value of `falseBranchOrNull` + */ + def falseBranch: Option[Expression] = Option(falseBranchOrNull) /** Creates a copy of `this`. * @@ -125,7 +131,7 @@ case class IfThenElse( |IfThenElse( |cond = $cond, |trueBranch = $trueBranch, - |falseBranch = ${falseBranch()}, + |falseBranch = ${falseBranch}, |location = $location, |passData = ${this.showPassData}, |diagnostics = $diagnostics, @@ -135,7 +141,7 @@ case class IfThenElse( /** @inheritdoc */ override def children: List[IR] = - List(cond, trueBranch) ++ falseBranch().toList + List(cond, trueBranch) ++ falseBranch.toList /** @inheritdoc */ override def showCode(indent: Int): String = { diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java new file mode 100644 index 000000000000..72938223af41 --- /dev/null +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java @@ -0,0 +1,47 @@ +package org.enso.interpreter.node.controlflow; + +import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.nodes.Node.Child; +import com.oracle.truffle.api.nodes.NodeInfo; +import java.util.Objects; +import org.enso.interpreter.node.ExpressionNode; +import org.enso.interpreter.runtime.EnsoContext; + +@NodeInfo( + shortName = "if_then_else", + description = "The runtime representation of a if then else expression.") +public final class IfThenElseNode extends ExpressionNode { + private @Child ExpressionNode cond; + private @Child ExpressionNode trueBranch; + private @Child ExpressionNode falseBranch; + + private IfThenElseNode( + ExpressionNode cond, ExpressionNode trueBranch, ExpressionNode falseBranch) { + this.cond = Objects.requireNonNull(cond); + this.trueBranch = Objects.requireNonNull(trueBranch); + this.falseBranch = falseBranch; + } + + public static IfThenElseNode build( + ExpressionNode cond, ExpressionNode trueBranch, ExpressionNode falseBranch) { + return new IfThenElseNode(cond, trueBranch, falseBranch); + } + + public Object executeGeneric(VirtualFrame frame) { + var condResult = cond.executeGeneric(frame); + var ctx = EnsoContext.get(this); + if (condResult instanceof Boolean trueOrFalse) { + if (trueOrFalse) { + return trueBranch.executeGeneric(frame); + } else { + if (falseBranch == null) { + return ctx.getBuiltins().nothing(); + } else { + return falseBranch.executeGeneric(frame); + } + } + } else { + throw ctx.raiseAssertionPanic(this, "Expecting boolean", null); + } + } +} diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenBuiltinNode.java similarity index 90% rename from engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenNode.java rename to engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenBuiltinNode.java index 9c040d49129f..568aa5833721 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenBuiltinNode.java @@ -16,12 +16,12 @@ name = "if_then", description = "Performs the standard if-then control flow operation.", inlineable = true) -public abstract class IfThenNode extends Node { +public abstract class IfThenBuiltinNode extends Node { private @Child ThunkExecutorNode leftThunkExecutorNode = ThunkExecutorNode.build(); private final CountingConditionProfile condProfile = CountingConditionProfile.create(); - static IfThenNode build() { - return IfThenNodeGen.create(); + static IfThenBuiltinNode build() { + return IfThenBuiltinNodeGen.create(); } abstract Object execute(VirtualFrame frame, State state, boolean self, @Suspend Object if_true); diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenElseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenElseBuiltinNode.java similarity index 96% rename from engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenElseNode.java rename to engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenElseBuiltinNode.java index 289df4dd2bcc..83def6a8f550 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenElseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/bool/IfThenElseBuiltinNode.java @@ -14,7 +14,7 @@ name = "if_then_else", description = "Performs the standard if-then-else control flow operation.", inlineable = true) -public final class IfThenElseNode extends Node { +public final class IfThenElseBuiltinNode extends Node { private @Child ThunkExecutorNode leftThunkExecutorNode = ThunkExecutorNode.build(); private @Child ThunkExecutorNode rightThunkExecutorNode = ThunkExecutorNode.build(); private final CountingConditionProfile condProfile = CountingConditionProfile.create(); 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 f2d24d34da3d..0f158c830d54 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 @@ -40,6 +40,7 @@ import org.enso.compiler.core.ir.expression.{ Comment, Error, Foreign, + IfThenElse, Operator, Section } @@ -78,6 +79,7 @@ import org.enso.interpreter.node.callable.{ InvokeCallableNode, SequenceLiteralNode } +import org.enso.interpreter.node.controlflow.IfThenElseNode import org.enso.interpreter.node.controlflow.caseexpr._ import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode import org.enso.interpreter.node.expression.builtin.BuiltinRootNode @@ -1305,6 +1307,8 @@ class IrToTruffle( case name: Name => processName(name) case function: Function => processFunction(function, binding) case binding: Expression.Binding => processBinding(binding) + case ife: IfThenElse => + processIfThenElse(ife, subjectToInstrumentation) case caseExpr: Case => processCase(caseExpr, subjectToInstrumentation) case asc: Tpe.Ascription => @@ -1426,12 +1430,25 @@ class IrToTruffle( ) } + /** Code generation for if then else expression. + */ + private def processIfThenElse( + ife: IfThenElse, + subjectToInstrumentation: Boolean + ): RuntimeExpression = { + val condNode = this.run(ife.cond, subjectToInstrumentation) + val trueNode = this.run(ife.trueBranch, subjectToInstrumentation) + val falseNode = + ife.falseBranch.map(this.run(_, subjectToInstrumentation)).orNull + IfThenElseNode.build(condNode, trueNode, falseNode) + } + /** Performs code generation for an Enso case expression. * * @param caseExpr the case expression to generate code for * @return the truffle nodes corresponding to `caseExpr` */ - def processCase( + private def processCase( caseExpr: Case, subjectToInstrumentation: Boolean ): RuntimeExpression = From 6b6be17ab3938c5edd10670f1415296ce4d228a9 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 25 Oct 2024 15:32:46 +0200 Subject: [PATCH 11/38] IfThenElse can access local variables --- .../compiler/pass/analyse/AliasAnalysis.scala | 26 +++++++++++++++++-- .../pass/analyse/DataflowAnalysis.scala | 10 +++++-- .../pass/analyse/FramePointerAnalysis.scala | 6 ++++- .../node/controlflow/IfThenElseNode.java | 11 +++++++- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala index a5d465d5eeef..72ac20f84ede 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala @@ -10,6 +10,7 @@ import org.enso.compiler.core.ir.expression.{ Case, Comment, Error, + IfThenElse, Operator, Section } @@ -386,7 +387,8 @@ case object AliasAnalysis extends IRPass { graph, parentScope ) - case cse: Case => analyseCase(cse, graph, parentScope) + case ife: IfThenElse => analyseIfThenElse(ife, graph, parentScope) + case cse: Case => analyseCase(cse, graph, parentScope) case block: Expression.Block => val currentScope = if (!block.suspended) parentScope else parentScope.addChild() @@ -796,6 +798,26 @@ case object AliasAnalysis extends IRPass { ) } + /** Performs alias analysis on a if then else expression. + * + * @param ir the expression to analyse + * @param graph the graph in which the analysis is taking place + * @param parentScope the scope in which the expression occurs + * @return `ir`, possibly with alias analysis information attached + */ + private def analyseIfThenElse( + ir: IfThenElse, + graph: Graph, + parentScope: Scope + ): IfThenElse = { + ir.copy( + cond = analyseExpression(ir.cond, graph, parentScope), + trueBranch = analyseExpression(ir.trueBranch, graph, parentScope), + falseBranchOrNull = + ir.falseBranch.map(analyseExpression(_, graph, parentScope)).orNull + ) + } + /** Performs alias analysis on a case expression. * * @param ir the case expression to analyse @@ -803,7 +825,7 @@ case object AliasAnalysis extends IRPass { * @param parentScope the scope in which the case expression occurs * @return `ir`, possibly with alias analysis information attached */ - def analyseCase( + private def analyseCase( ir: Case, graph: Graph, parentScope: Scope diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DataflowAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DataflowAnalysis.scala index 5aea4de90a78..ece93066cec8 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DataflowAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/DataflowAnalysis.scala @@ -596,9 +596,15 @@ case object DataflowAnalysis extends IRPass { ife: IfThenElse, info: DependencyInfo ): IfThenElse = { - // TBD - info.getClass() + val ifeDep = asStatic(ife) + val condDep = asStatic(ife.cond) + info.dependents.updateAt(condDep, Set(ifeDep)) + info.dependencies.updateAt(ifeDep, Set(condDep)) ife + .copy( + cond = analyseExpression(ife.cond, info) + ) + .updateMetadata(new MetadataPair(this, info)) } /** Performs dependency analysis on a case expression. diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala index 71a1e00b1b03..dc51c965b79e 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala @@ -4,7 +4,7 @@ import org.enso.compiler.pass.analyse.FramePointer import org.enso.compiler.context.{InlineContext, LocalScope, ModuleContext} import org.enso.compiler.core.ir.Name.GenericAnnotation import org.enso.compiler.core.{CompilerError, IR} -import org.enso.compiler.core.ir.expression.{Application, Case} +import org.enso.compiler.core.ir.expression.{Application, Case, IfThenElse} import org.enso.compiler.core.ir.{ CallArgument, DefinitionArgument, @@ -159,6 +159,10 @@ case object FramePointerAnalysis extends IRPass { processExpression(expr, graph) maybeAttachFramePointer(binding, graph) case app: Application => processApplication(app, graph) + case ife: IfThenElse => + processExpression(ife.cond, graph) + processExpression(ife.trueBranch, graph) + ife.falseBranch.map(processExpression(_, graph)) case caseExpr: Case.Expr => processExpression(caseExpr.scrutinee, graph) caseExpr.branches.foreach { branch => diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java index 72938223af41..44c153774109 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java @@ -1,11 +1,13 @@ package org.enso.interpreter.node.controlflow; +import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.Node.Child; import com.oracle.truffle.api.nodes.NodeInfo; import java.util.Objects; import org.enso.interpreter.node.ExpressionNode; import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.error.PanicException; @NodeInfo( shortName = "if_then_else", @@ -27,6 +29,7 @@ public static IfThenElseNode build( return new IfThenElseNode(cond, trueBranch, falseBranch); } + @Override public Object executeGeneric(VirtualFrame frame) { var condResult = cond.executeGeneric(frame); var ctx = EnsoContext.get(this); @@ -41,7 +44,13 @@ public Object executeGeneric(VirtualFrame frame) { } } } else { - throw ctx.raiseAssertionPanic(this, "Expecting boolean", null); + throw expectingBoolean(ctx, condResult); } } + + @CompilerDirectives.TruffleBoundary + private PanicException expectingBoolean(EnsoContext ctx, Object condResult) + throws PanicException { + throw ctx.raiseAssertionPanic(this, "Expecting boolean but got " + condResult, null); + } } From 2835bb474e747f30ed6036a2c2eb5064a53f04e5 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 25 Oct 2024 18:39:14 +0200 Subject: [PATCH 12/38] IfThenElse uses addChild(flattenToParent=true) to process, but isolate all the sub-branches --- .../pass/analyse/PassPersistance.java | 6 ++- .../enso/compiler/pass/analyse/TailCall.java | 20 ++++++++++ .../compiler/pass/analyse/AliasAnalysis.scala | 9 +++-- .../pass/analyse/FramePointerAnalysis.scala | 4 +- .../pass/analyse/alias/graph/Graph.scala | 13 ++++-- .../enso/compiler/test/IfThenElseTest.java | 40 ++++++++++++++++++- .../enso/compiler/core/ir/IrPersistance.java | 2 + .../core/ir/expression/IfThenElse.scala | 6 +-- 8 files changed, 86 insertions(+), 14 deletions(-) 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 c88bbc586ab6..829c0f6a530c 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 @@ -96,17 +96,18 @@ protected IgnoredBindings.State readObject(Input in) @org.openide.util.lookup.ServiceProvider(service = Persistance.class) public static final class PersistAliasAnalysisGraphScope extends Persistance { public PersistAliasAnalysisGraphScope() { - super(Graph.Scope.class, false, 1267); + super(Graph.Scope.class, false, 1269); } @Override @SuppressWarnings("unchecked") protected Graph.Scope readObject(Input in) throws IOException { + var flatten = in.readBoolean(); var childScopes = in.readInline(scala.collection.immutable.List.class); var occurrencesValues = (scala.collection.immutable.Set) in.readObject(); var occurrences = occurrencesValues.map(v -> Tuple2$.MODULE$.apply(v.id(), v)).toMap(null); var allDefinitions = in.readInline(scala.collection.immutable.List.class); - var parent = new Graph.Scope(childScopes, occurrences, allDefinitions); + var parent = new Graph.Scope(flatten, childScopes, occurrences, allDefinitions); var optionParent = Option.apply(parent); childScopes.forall( (object) -> { @@ -120,6 +121,7 @@ protected Graph.Scope readObject(Input in) throws IOException { @Override @SuppressWarnings("unchecked") protected void writeObject(Graph.Scope obj, Output out) throws IOException { + out.writeBoolean(obj.flattenToParent()); out.writeInline(scala.collection.immutable.List.class, obj.childScopes()); out.writeObject(obj.occurrences().values().toSet()); out.writeInline(scala.collection.immutable.List.class, obj.allDefinitions()); diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/TailCall.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/TailCall.java index b328f666abae..55a19f2996bb 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/TailCall.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/TailCall.java @@ -18,6 +18,7 @@ import org.enso.compiler.core.ir.expression.Application; import org.enso.compiler.core.ir.expression.Case; import org.enso.compiler.core.ir.expression.Comment; +import org.enso.compiler.core.ir.expression.IfThenElse; import org.enso.compiler.core.ir.module.scope.Definition; import org.enso.compiler.core.ir.module.scope.definition.*; import org.enso.compiler.pass.IRPass; @@ -206,6 +207,16 @@ public Expression transformExpression(Expression ir) { // Note [Call Argument Tail Position] p.arguments().foreach(a -> markAsTail(a)); } + case IfThenElse ite -> { + if (isInTailPos) { + markAsTail(ite); + // Note [Analysing Branches in Case Expressions] + markAsTail(ite.trueBranch()); + if (ite.falseBranchOrNull() != null) { + markAsTail(ite.falseBranchOrNull()); + } + } + } case Case.Expr e -> { if (isInTailPos) { markAsTail(ir); @@ -246,6 +257,15 @@ private void collectTailCandidatesExpression( Expression expression, java.util.Map tailCandidates) { switch (expression) { case Function function -> collectTailCandicateFunction(function, tailCandidates); + case IfThenElse ite -> { + if (isInTailPos) { + // Note [Analysing Branches in Case Expressions] + tailCandidates.put(ite.trueBranch(), true); + if (ite.falseBranchOrNull() != null) { + tailCandidates.put(ite.falseBranchOrNull(), true); + } + } + } case Case caseExpr -> collectTailCandidatesCase(caseExpr, tailCandidates); case Application app -> collectTailCandidatesApplication(app, tailCandidates); case Name name -> collectTailCandidatesName(name, tailCandidates); diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala index 72ac20f84ede..efc848971365 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala @@ -810,11 +810,14 @@ case object AliasAnalysis extends IRPass { graph: Graph, parentScope: Scope ): IfThenElse = { + val condScope = parentScope.addChild(flattenToParent = true) + val trueScope = parentScope.addChild(flattenToParent = true) + val falseScope = parentScope.addChild(flattenToParent = true) ir.copy( - cond = analyseExpression(ir.cond, graph, parentScope), - trueBranch = analyseExpression(ir.trueBranch, graph, parentScope), + cond = analyseExpression(ir.cond, graph, condScope), + trueBranch = analyseExpression(ir.trueBranch, graph, trueScope), falseBranchOrNull = - ir.falseBranch.map(analyseExpression(_, graph, parentScope)).orNull + ir.falseBranch.map(analyseExpression(_, graph, falseScope)).orNull ) } diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala index dc51c965b79e..f6ecbb64647c 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala @@ -376,8 +376,10 @@ case object FramePointerAnalysis extends IRPass { var currScope: Option[Graph.Scope] = Some(childScope) var scopeDistance = 0 while (currScope.isDefined && currScope.get != parentScope) { + if (!currScope.get.flattenToParent) { + scopeDistance += 1 + } currScope = currScope.get.parent - scopeDistance += 1 } scopeDistance } diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala index 21abf4d890ca..28d9615ce23a 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala @@ -359,6 +359,7 @@ object Graph { * Note that there may not be a link for all these definitions. */ sealed class Scope( + val flattenToParent: Boolean = false, var childScopes: List[Scope] = List(), var occurrences: Map[Id, GraphOccurrence] = HashMap(), var allDefinitions: List[GraphOccurrence.Def] = List() @@ -405,7 +406,12 @@ object Graph { childScopeCopies += scope.deepCopy(mapping) ) val newScope = - new Scope(childScopeCopies.toList, occurrences, allDefinitions) + new Scope( + this.flattenToParent, + childScopeCopies.toList, + occurrences, + allDefinitions + ) mapping.put(this, newScope) newScope } @@ -433,10 +439,11 @@ object Graph { /** Creates and returns a scope that is a child of this one. * + * @param is this scope "just virtual" and will be flatten to parent at the end? * @return a scope that is a child of `this` */ - def addChild(): Scope = { - val scope = new Scope() + def addChild(flattenToParent: Boolean = false): Scope = { + val scope = new Scope(flattenToParent) scope.parent = Some(this) childScopes ::= scope diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java index 0a8896377183..d3c2dea8d172 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java @@ -15,12 +15,12 @@ import org.graalvm.polyglot.Context; import org.graalvm.polyglot.PolyglotException; import org.graalvm.polyglot.Value; +import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.hamcrest.core.AllOf; import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; public class IfThenElseTest { @@ -96,6 +96,43 @@ public void variableDefinedInElse() throws Exception { assertEquals("Bad:False", check.execute(false).asString()); } + @Test + public void variableInMultipleIfBranches() throws Exception { + var code = + """ + check x = + if x then + xt = "Yes" + if x.not then + xt = "No" + "Hooo" + """; + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + + assertEquals("Hooo", check.execute(true).asString()); + assertEquals("Hooo", check.execute(false).asString()); + } + + @Test + public void variableNotVisibleAfterBranches() throws Exception { + var code = + """ + check x = + if x then + xt = "Yes" + if x.not then + xt = "No" + xt + """; + try { + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + fail("The code should not compile, but returned: " + check); + } catch (PolyglotException ex) { + MatcherAssert.assertThat( + ex.getMessage(), Matchers.containsString("name `xt` could not be found")); + } + } + @Test public void variableUsedAfterTheBranch() throws Exception { try { @@ -160,7 +197,6 @@ public void javaScriptBooleanIsSupported() throws Exception { assertEquals("No", check.execute("Ne").asString()); } - @Ignore @Test public void truffleObjectConvertibleToBooleanIsSupported() throws Exception { var code = diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java index 4dd42faf9462..ff17f5c9a409 100644 --- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java @@ -9,6 +9,7 @@ import org.enso.compiler.core.ir.expression.Application; import org.enso.compiler.core.ir.expression.Case; import org.enso.compiler.core.ir.expression.Foreign; +import org.enso.compiler.core.ir.expression.IfThenElse; import org.enso.compiler.core.ir.expression.Operator; import org.enso.compiler.core.ir.expression.warnings.Unused; import org.enso.compiler.core.ir.module.scope.Definition; @@ -50,6 +51,7 @@ @Persistable(clazz = Application.Prefix.class, id = 753) @Persistable(clazz = Application.Force.class, id = 754) @Persistable(clazz = Application.Sequence.class, id = 755) +@Persistable(clazz = IfThenElse.class, id = 760) @Persistable(clazz = Case.Expr.class, id = 761) @Persistable(clazz = Case.Branch.class, id = 762) @Persistable(clazz = Pattern.Constructor.class, id = 763) diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala index 04b489b83019..25b6a9453c5c 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/IfThenElse.scala @@ -9,9 +9,9 @@ import java.util.UUID /** The Enso case expression. */ case class IfThenElse( - cond: Expression, - trueBranch: Expression, - private val falseBranchOrNull: Expression, + val cond: Expression, + val trueBranch: Expression, + val falseBranchOrNull: Expression, override val identifiedLocation: IdentifiedLocation, override val passData: MetadataStorage = new MetadataStorage() ) extends Expression From 97c90b8308bd426b7339386cd8c6941d3d9e292e Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 25 Oct 2024 19:45:31 +0200 Subject: [PATCH 13/38] @Specialization methods don't have to be public --- .../node/controlflow/caseexpr/CaseNode.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java index 0967739a28d6..f0486e35c265 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java @@ -14,7 +14,9 @@ import org.enso.interpreter.node.ExpressionNode; import org.enso.interpreter.runtime.EnsoContext; import org.enso.interpreter.runtime.callable.function.Function; -import org.enso.interpreter.runtime.error.*; +import org.enso.interpreter.runtime.error.DataflowError; +import org.enso.interpreter.runtime.error.PanicException; +import org.enso.interpreter.runtime.error.PanicSentinel; import org.enso.interpreter.runtime.state.State; import org.enso.interpreter.runtime.type.TypesGen; import org.enso.interpreter.runtime.warning.AppendWarningNode; @@ -64,7 +66,7 @@ public static CaseNode build(ExpressionNode scrutinee, BranchNode[] cases, boole * @return the result of executing the case expression on {@code error} */ @Specialization - public Object doError(VirtualFrame frame, DataflowError error) { + final Object doError(VirtualFrame frame, DataflowError error) { return error; } @@ -76,13 +78,13 @@ public Object doError(VirtualFrame frame, DataflowError error) { * @return nothing */ @Specialization - public Object doPanicSentinel(VirtualFrame frame, PanicSentinel sentinel) { + final Object doPanicSentinel(VirtualFrame frame, PanicSentinel sentinel) { CompilerDirectives.transferToInterpreter(); throw sentinel; } @Specialization(guards = {"object != null", "warnings.hasWarnings(object)"}) - Object doWarning( + final Object doWarning( VirtualFrame frame, Object object, @Shared("warnsLib") @CachedLibrary(limit = "3") WarningsLibrary warnings, @@ -110,7 +112,7 @@ Object doWarning( "!warnings.hasWarnings(object)" }) @ExplodeLoop - public Object doMatch( + final Object doMatch( VirtualFrame frame, Object object, @Shared("warnsLib") @CachedLibrary(limit = "3") WarningsLibrary warnings) { From 3420d33bfa58bcb59e77eca769a3f81592e0a38c Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 25 Oct 2024 19:46:00 +0200 Subject: [PATCH 14/38] Using @Specialization to handle various IfThenElseNode situations --- .../node/controlflow/IfThenElseNode.java | 97 +++++++++++++------ 1 file changed, 70 insertions(+), 27 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java index 44c153774109..8b3d5bf3f95c 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java @@ -1,56 +1,99 @@ package org.enso.interpreter.node.controlflow; import com.oracle.truffle.api.CompilerDirectives; +import com.oracle.truffle.api.dsl.Cached; +import com.oracle.truffle.api.dsl.Cached.Shared; +import com.oracle.truffle.api.dsl.Fallback; +import com.oracle.truffle.api.dsl.NodeChild; +import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.interop.TruffleObject; +import com.oracle.truffle.api.interop.UnsupportedMessageException; +import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.Node.Child; import com.oracle.truffle.api.nodes.NodeInfo; +import com.oracle.truffle.api.profiles.InlinedCountingConditionProfile; import java.util.Objects; import org.enso.interpreter.node.ExpressionNode; import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.data.EnsoObject; +import org.enso.interpreter.runtime.error.DataflowError; import org.enso.interpreter.runtime.error.PanicException; +import org.enso.interpreter.runtime.error.PanicSentinel; @NodeInfo( shortName = "if_then_else", description = "The runtime representation of a if then else expression.") -public final class IfThenElseNode extends ExpressionNode { - private @Child ExpressionNode cond; +@NodeChild(value = "cond", type = ExpressionNode.class) +public abstract class IfThenElseNode extends ExpressionNode { private @Child ExpressionNode trueBranch; private @Child ExpressionNode falseBranch; - private IfThenElseNode( - ExpressionNode cond, ExpressionNode trueBranch, ExpressionNode falseBranch) { - this.cond = Objects.requireNonNull(cond); - this.trueBranch = Objects.requireNonNull(trueBranch); - this.falseBranch = falseBranch; + IfThenElseNode(ExpressionNode t, ExpressionNode fOrNull) { + this.trueBranch = Objects.requireNonNull(t); + this.falseBranch = fOrNull; } - public static IfThenElseNode build( - ExpressionNode cond, ExpressionNode trueBranch, ExpressionNode falseBranch) { - return new IfThenElseNode(cond, trueBranch, falseBranch); + public static IfThenElseNode build(ExpressionNode c, ExpressionNode t, ExpressionNode fOrNull) { + return IfThenElseNodeGen.create(t, fOrNull, c); } - @Override - public Object executeGeneric(VirtualFrame frame) { - var condResult = cond.executeGeneric(frame); - var ctx = EnsoContext.get(this); - if (condResult instanceof Boolean trueOrFalse) { - if (trueOrFalse) { - return trueBranch.executeGeneric(frame); + @Specialization + final Object doBoolean( + VirtualFrame frame, + boolean cond, + @Shared("profile") @Cached InlinedCountingConditionProfile profile) { + if (cond) { + profile.wasTrue(this); + return trueBranch.executeGeneric(frame); + } else { + profile.wasFalse(this); + if (falseBranch == null) { + var ctx = EnsoContext.get(this); + return ctx.getBuiltins().nothing(); } else { - if (falseBranch == null) { - return ctx.getBuiltins().nothing(); - } else { - return falseBranch.executeGeneric(frame); - } + return falseBranch.executeGeneric(frame); } - } else { - throw expectingBoolean(ctx, condResult); } } + static boolean notEnsoObject(Object o) { + return !(o instanceof EnsoObject); + } + + @Specialization( + limit = "3", + guards = {"notEnsoObject(foreignObj)", "iop.isBoolean(foreignObj)"}) + final Object doTruffleBoolean( + VirtualFrame frame, + TruffleObject foreignObj, + @CachedLibrary("foreignObj") InteropLibrary iop, + @Shared("profile") @Cached InlinedCountingConditionProfile profile) { + try { + var cond = iop.asBoolean(foreignObj); + return doBoolean(frame, cond, profile); + } catch (UnsupportedMessageException ex) { + var ctx = EnsoContext.get(this); + throw ctx.raiseAssertionPanic(this, "Expecting boolean", ex); + } + } + + @Specialization + final Object doError(VirtualFrame frame, DataflowError error) { + return error; + } + + @Specialization + final Object doPanicSentinel(VirtualFrame frame, PanicSentinel sentinel) { + CompilerDirectives.transferToInterpreter(); + throw sentinel; + } + + @Fallback @CompilerDirectives.TruffleBoundary - private PanicException expectingBoolean(EnsoContext ctx, Object condResult) - throws PanicException { - throw ctx.raiseAssertionPanic(this, "Expecting boolean but got " + condResult, null); + final PanicException doOther(Object cond) throws PanicException { + var ctx = EnsoContext.get(this); + throw ctx.raiseAssertionPanic(this, "Expecting boolean but got " + cond, null); } } From 063e7f0e5ab05a32e4069f62fec082c531f6a32a Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 25 Oct 2024 20:14:47 +0200 Subject: [PATCH 15/38] Handling Warnings WithCondition node --- .../node/controlflow/IfThenElseNode.java | 163 ++++++++++++------ .../node/controlflow/caseexpr/CaseNode.java | 23 +-- 2 files changed, 121 insertions(+), 65 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java index 8b3d5bf3f95c..683423fad930 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java @@ -4,13 +4,13 @@ import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.Fallback; -import com.oracle.truffle.api.dsl.NodeChild; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.TruffleObject; import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.api.library.CachedLibrary; +import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.nodes.Node.Child; import com.oracle.truffle.api.nodes.NodeInfo; import com.oracle.truffle.api.profiles.InlinedCountingConditionProfile; @@ -21,79 +21,134 @@ import org.enso.interpreter.runtime.error.DataflowError; import org.enso.interpreter.runtime.error.PanicException; import org.enso.interpreter.runtime.error.PanicSentinel; +import org.enso.interpreter.runtime.warning.AppendWarningNode; +import org.enso.interpreter.runtime.warning.WarningsLibrary; @NodeInfo( shortName = "if_then_else", description = "The runtime representation of a if then else expression.") -@NodeChild(value = "cond", type = ExpressionNode.class) -public abstract class IfThenElseNode extends ExpressionNode { +public final class IfThenElseNode extends ExpressionNode { + private @Child ExpressionNode cond; private @Child ExpressionNode trueBranch; private @Child ExpressionNode falseBranch; + private @Child WithCondition with; - IfThenElseNode(ExpressionNode t, ExpressionNode fOrNull) { - this.trueBranch = Objects.requireNonNull(t); - this.falseBranch = fOrNull; + IfThenElseNode(ExpressionNode cond, ExpressionNode trueBranch, ExpressionNode falseBranch) { + this.cond = Objects.requireNonNull(cond); + this.trueBranch = trueBranch; + this.falseBranch = falseBranch; + this.with = IfThenElseNodeFactory.WithConditionNodeGen.create(); } public static IfThenElseNode build(ExpressionNode c, ExpressionNode t, ExpressionNode fOrNull) { - return IfThenElseNodeGen.create(t, fOrNull, c); + return new IfThenElseNode(c, t, fOrNull); } - @Specialization - final Object doBoolean( - VirtualFrame frame, - boolean cond, - @Shared("profile") @Cached InlinedCountingConditionProfile profile) { - if (cond) { - profile.wasTrue(this); - return trueBranch.executeGeneric(frame); - } else { - profile.wasFalse(this); - if (falseBranch == null) { - var ctx = EnsoContext.get(this); - return ctx.getBuiltins().nothing(); + @Override + public Object executeGeneric(VirtualFrame frame) { + var condValue = cond.executeGeneric(frame); + var result = with.executeIf(frame, condValue, trueBranch, falseBranch); + return result; + } + + abstract static class WithCondition extends Node { + WithCondition() {} + + abstract Object executeIf( + VirtualFrame frame, Object cond, ExpressionNode trueBranch, ExpressionNode falseBranch); + + @Specialization + final Object doBoolean( + VirtualFrame frame, + boolean cond, + ExpressionNode trueBranch, + ExpressionNode falseBranch, + @Shared("profile") @Cached InlinedCountingConditionProfile profile) { + if (cond) { + profile.wasTrue(this); + return trueBranch.executeGeneric(frame); } else { - return falseBranch.executeGeneric(frame); + profile.wasFalse(this); + if (falseBranch == null) { + var ctx = EnsoContext.get(this); + return ctx.getBuiltins().nothing(); + } else { + return falseBranch.executeGeneric(frame); + } } } - } - static boolean notEnsoObject(Object o) { - return !(o instanceof EnsoObject); - } + static boolean notEnsoObject(Object o) { + return !(o instanceof EnsoObject); + } - @Specialization( - limit = "3", - guards = {"notEnsoObject(foreignObj)", "iop.isBoolean(foreignObj)"}) - final Object doTruffleBoolean( - VirtualFrame frame, - TruffleObject foreignObj, - @CachedLibrary("foreignObj") InteropLibrary iop, - @Shared("profile") @Cached InlinedCountingConditionProfile profile) { - try { - var cond = iop.asBoolean(foreignObj); - return doBoolean(frame, cond, profile); - } catch (UnsupportedMessageException ex) { - var ctx = EnsoContext.get(this); - throw ctx.raiseAssertionPanic(this, "Expecting boolean", ex); + @Specialization( + limit = "3", + guards = {"notEnsoObject(foreignObj)", "iop.isBoolean(foreignObj)"}) + final Object doTruffleBoolean( + VirtualFrame frame, + TruffleObject foreignObj, + ExpressionNode trueBranch, + ExpressionNode falseBranch, + @CachedLibrary("foreignObj") InteropLibrary iop, + @Shared("profile") @Cached InlinedCountingConditionProfile profile) { + try { + var cond = iop.asBoolean(foreignObj); + return doBoolean(frame, cond, trueBranch, falseBranch, profile); + } catch (UnsupportedMessageException ex) { + var ctx = EnsoContext.get(this); + throw ctx.raiseAssertionPanic(this, "Expecting boolean", ex); + } } - } - @Specialization - final Object doError(VirtualFrame frame, DataflowError error) { - return error; - } + @Specialization( + limit = "3", + guards = {"cond != null", "warnings.hasWarnings(cond)"}) + final Object doWarning( + VirtualFrame frame, + Object cond, + ExpressionNode trueBranch, + ExpressionNode falseBranch, + @CachedLibrary(value = "cond") WarningsLibrary warnings, + @Cached AppendWarningNode appendWarningNode, + @Cached WithCondition delegate) { + try { + var ws = warnings.getWarnings(cond, false); + var without = warnings.removeWarnings(cond); - @Specialization - final Object doPanicSentinel(VirtualFrame frame, PanicSentinel sentinel) { - CompilerDirectives.transferToInterpreter(); - throw sentinel; - } + var result = delegate.executeIf(frame, without, trueBranch, falseBranch); + return appendWarningNode.executeAppend(null, result, ws); + } catch (UnsupportedMessageException e) { + var ctx = EnsoContext.get(this); + throw ctx.raiseAssertionPanic(this, null, e); + } + } - @Fallback - @CompilerDirectives.TruffleBoundary - final PanicException doOther(Object cond) throws PanicException { - var ctx = EnsoContext.get(this); - throw ctx.raiseAssertionPanic(this, "Expecting boolean but got " + cond, null); + @Specialization + final Object doError( + VirtualFrame frame, + DataflowError cond, + ExpressionNode trueBranch, + ExpressionNode falseBranch) { + return cond; + } + + @Specialization + final Object doPanicSentinel( + VirtualFrame frame, + PanicSentinel cond, + ExpressionNode trueBranch, + ExpressionNode falseBranch) { + CompilerDirectives.transferToInterpreter(); + throw cond; + } + + @Fallback + @CompilerDirectives.TruffleBoundary + final PanicException doOther(Object cond, ExpressionNode trueBranch, ExpressionNode falseBranch) + throws PanicException { + var ctx = EnsoContext.get(this); + throw ctx.raiseAssertionPanic(this, "Expecting boolean but got " + cond, null); + } } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java index f0486e35c265..cbbb8eeef712 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java @@ -1,5 +1,16 @@ package org.enso.interpreter.node.controlflow.caseexpr; +import org.enso.interpreter.node.ExpressionNode; +import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.callable.function.Function; +import org.enso.interpreter.runtime.error.DataflowError; +import org.enso.interpreter.runtime.error.PanicException; +import org.enso.interpreter.runtime.error.PanicSentinel; +import org.enso.interpreter.runtime.state.State; +import org.enso.interpreter.runtime.type.TypesGen; +import org.enso.interpreter.runtime.warning.AppendWarningNode; +import org.enso.interpreter.runtime.warning.WarningsLibrary; + import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Cached.Shared; @@ -11,16 +22,6 @@ import com.oracle.truffle.api.nodes.ExplodeLoop; import com.oracle.truffle.api.nodes.NodeInfo; import com.oracle.truffle.api.profiles.CountingConditionProfile; -import org.enso.interpreter.node.ExpressionNode; -import org.enso.interpreter.runtime.EnsoContext; -import org.enso.interpreter.runtime.callable.function.Function; -import org.enso.interpreter.runtime.error.DataflowError; -import org.enso.interpreter.runtime.error.PanicException; -import org.enso.interpreter.runtime.error.PanicSentinel; -import org.enso.interpreter.runtime.state.State; -import org.enso.interpreter.runtime.type.TypesGen; -import org.enso.interpreter.runtime.warning.AppendWarningNode; -import org.enso.interpreter.runtime.warning.WarningsLibrary; /** * A node representing a pattern match on an arbitrary runtime value. @@ -61,7 +62,7 @@ public static CaseNode build(ExpressionNode scrutinee, BranchNode[] cases, boole * *

It is important that this is the first specialization. * - * @param frame the stack frame in which to execute + * @param frame the stack frame in which to executeq * @param error the error being matched against * @return the result of executing the case expression on {@code error} */ From 4f4fa1db651a386c70222ce7b7811dd2648911f6 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 25 Oct 2024 20:29:34 +0200 Subject: [PATCH 16/38] javafmtAll --- .../node/controlflow/caseexpr/CaseNode.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java index cbbb8eeef712..97d48d3024cf 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java @@ -1,16 +1,5 @@ package org.enso.interpreter.node.controlflow.caseexpr; -import org.enso.interpreter.node.ExpressionNode; -import org.enso.interpreter.runtime.EnsoContext; -import org.enso.interpreter.runtime.callable.function.Function; -import org.enso.interpreter.runtime.error.DataflowError; -import org.enso.interpreter.runtime.error.PanicException; -import org.enso.interpreter.runtime.error.PanicSentinel; -import org.enso.interpreter.runtime.state.State; -import org.enso.interpreter.runtime.type.TypesGen; -import org.enso.interpreter.runtime.warning.AppendWarningNode; -import org.enso.interpreter.runtime.warning.WarningsLibrary; - import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Cached.Shared; @@ -22,6 +11,16 @@ import com.oracle.truffle.api.nodes.ExplodeLoop; import com.oracle.truffle.api.nodes.NodeInfo; import com.oracle.truffle.api.profiles.CountingConditionProfile; +import org.enso.interpreter.node.ExpressionNode; +import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.callable.function.Function; +import org.enso.interpreter.runtime.error.DataflowError; +import org.enso.interpreter.runtime.error.PanicException; +import org.enso.interpreter.runtime.error.PanicSentinel; +import org.enso.interpreter.runtime.state.State; +import org.enso.interpreter.runtime.type.TypesGen; +import org.enso.interpreter.runtime.warning.AppendWarningNode; +import org.enso.interpreter.runtime.warning.WarningsLibrary; /** * A node representing a pattern match on an arbitrary runtime value. From c2c6b248e4de34f378de2042b415f35c4f0cf6dd Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 26 Oct 2024 07:14:27 +0200 Subject: [PATCH 17/38] Test showing that definition of a variable in if branch overrides global --- .../enso/compiler/test/IfThenElseTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java index d3c2dea8d172..ecbead554312 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java @@ -332,4 +332,26 @@ private static void assertWarning(String txt, Value v) { assertEquals(txt, ex.getMessage()); } } + + @Test + public void dontOverrideVariablesFromOuterScope() throws Exception { + var code = + """ + type Hello + World msg good + + join self init = + if self.good then "Ciao" else + x = init + x + self.msg + + hello state = + Hello.World "World" state . join "Hello " + """; + + var hello = ContextUtils.getMethodFromModule(ctx, code, "hello"); + + assertEquals("Ciao", hello.execute(true).asString()); + assertEquals("Hello World", hello.execute(false).asString()); + } } From 039ba4098ead288e09bab67badba453f4db37de3 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 26 Oct 2024 07:52:10 +0200 Subject: [PATCH 18/38] Fixing typo --- .../enso/interpreter/node/controlflow/caseexpr/CaseNode.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java index 97d48d3024cf..f0486e35c265 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/CaseNode.java @@ -61,7 +61,7 @@ public static CaseNode build(ExpressionNode scrutinee, BranchNode[] cases, boole * *

It is important that this is the first specialization. * - * @param frame the stack frame in which to executeq + * @param frame the stack frame in which to execute * @param error the error being matched against * @return the result of executing the case expression on {@code error} */ From 9f75eb15df673ea57ff71e14402701fc7edccc62 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 26 Oct 2024 07:53:05 +0200 Subject: [PATCH 19/38] No need for boolean literal right now --- .../node/expression/literal/LiteralNode.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/literal/LiteralNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/literal/LiteralNode.java index 397589357d73..69a3861a6a0d 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/literal/LiteralNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/literal/LiteralNode.java @@ -51,16 +51,6 @@ public static LiteralNode build(double value) { return new LiteralNode(value); } - /** - * Creates an instance of the literal node. - * - * @param value boolean value for the node to represent - * @return a node representing the literal given by {@code value} - */ - public static LiteralNode build(boolean value) { - return new LiteralNode(value); - } - /** * Creates an instance of the literal node. * From dbaf5f3ed305678e94b5de3b411772a3e52dc337 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 26 Oct 2024 08:04:20 +0200 Subject: [PATCH 20/38] Recalculate offset when flattenToParent --- .../pass/analyse/FramePointerAnalysis.scala | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala index f6ecbb64647c..df3579f680e6 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala @@ -95,7 +95,12 @@ case object FramePointerAnalysis extends IRPass { } private def updateSymbolNames(e: IR, s: Graph.Scope): Unit = { - val symbols = s.allDefinitions.map(_.symbol) + var symbols = s.allDefinitions.map(_.symbol) + s.childScopes.foreach(ch => + if (ch.flattenToParent) { + symbols ++= ch.allDefinitions.map(_.symbol) + } + ) updateMeta(e, FrameVariableNames.create(symbols)) } @@ -360,7 +365,28 @@ case object FramePointerAnalysis extends IRPass { "Def occurrence must be in the given scope" ) ) - idxInScope + LocalScope.internalSlotsSize + val parentOffset = if (scope.flattenToParent && scope.parent.nonEmpty) { + val p = scope.parent.get + var off = p.allDefinitions.size + val found = p.childScopes.find(ch => { + if (ch == scope) { + true + } else { + if (ch.flattenToParent) { + off += ch.allDefinitions.size + } + false + } + }) + org.enso.common.Asserts.assertInJvm( + found != null, + "Child must be found: " + scope + " among " + p.childScopes + ) + off + } else { + 0 + } + parentOffset + idxInScope + LocalScope.internalSlotsSize } /** Returns the *scope distance* of the given `childScope` to the given `parentScope`. From e852d8aef09610ef521d2d71e020e1c639420dce Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 26 Oct 2024 08:04:37 +0200 Subject: [PATCH 21/38] Adjust the test to IfThenElse element --- .../org/enso/compiler/test/pass/analyse/TailCallTest.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/TailCallTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/TailCallTest.scala index fd4d86c4ced2..dee20393d6a0 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/TailCallTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/TailCallTest.scala @@ -7,6 +7,7 @@ import org.enso.compiler.core.ir.{Expression, Function, Pattern, Warning} import org.enso.compiler.core.ir.module.scope.definition import org.enso.compiler.core.ir.expression.Application import org.enso.compiler.core.ir.expression.Case +import org.enso.compiler.core.ir.expression.IfThenElse import org.enso.compiler.pass.PassConfiguration._ import org.enso.compiler.pass.analyse.TailCall.TailPosition import org.enso.compiler.pass.analyse.{AliasAnalysis, TailCall} @@ -238,9 +239,9 @@ class TailCallTest extends MiniPassTest { fnBody .asInstanceOf[Expression.Block] .returnValue - .asInstanceOf[Application.Prefix] - .arguments(2) - .value + .asInstanceOf[IfThenElse] + .falseBranch + .get .asInstanceOf[Expression.Block] .returnValue .asInstanceOf[Application.Prefix] From 00a69f79e054f9d628bd7ba3a8f9ffa848e4a3b9 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 26 Oct 2024 09:32:22 +0200 Subject: [PATCH 22/38] Adjusting the stacktrace: implementation details disappear with IfThenElse approach --- .../org/enso/interpreter/test/asserts/AssertionsTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/asserts/AssertionsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/asserts/AssertionsTest.java index 3ea35198860b..3d37641c18e9 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/asserts/AssertionsTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/asserts/AssertionsTest.java @@ -117,9 +117,8 @@ public void assertionFailureDisplaysStackTrace() { assertThat(e.getStackTrace().length, greaterThan(5)); assertThat(e.getStackTrace()[0].toString(), containsString("Panic")); assertThat(e.getStackTrace()[1].toString(), containsString("Runtime.assert")); - // Ignore the next two frames as they are implementation details - assertThat(e.getStackTrace()[4].toString(), containsString("foo")); - assertThat(e.getStackTrace()[5].toString(), containsString("main")); + assertThat(e.getStackTrace()[2].toString(), containsString("foo")); + assertThat(e.getStackTrace()[3].toString(), containsString("main")); } } From c44030589fe5a6410d0f949cbba561d97cea4204 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 26 Oct 2024 10:14:26 +0200 Subject: [PATCH 23/38] Use if_then function to get mixfix functions semantics --- .../test/pass/desugar/LambdaShorthandToLambdaTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/desugar/LambdaShorthandToLambdaTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/desugar/LambdaShorthandToLambdaTest.scala index 8201be729048..7a38716ab16e 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/desugar/LambdaShorthandToLambdaTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/desugar/LambdaShorthandToLambdaTest.scala @@ -190,7 +190,7 @@ class LambdaShorthandToLambdaTest extends CompilerTest { val ir = """ - |if _ then a + |_.if_then a |""".stripMargin.preprocessExpression.get.desugar ir shouldBe an[Function.Lambda] From 9c747d2462c82f3cd2c89be6001f30693f6d7295 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 28 Oct 2024 20:39:59 +0100 Subject: [PATCH 24/38] Towards single Scope for all if-then-else branches --- .../pass/analyse/PassPersistance.java | 4 +- .../compiler/pass/analyse/AliasAnalysis.scala | 13 ++--- .../pass/analyse/FramePointerAnalysis.scala | 33 ++---------- .../pass/analyse/alias/graph/Graph.scala | 52 +++---------------- .../core/ir/expression/errors/Redefined.scala | 3 ++ 5 files changed, 21 insertions(+), 84 deletions(-) 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 829c0f6a530c..210cb6b49586 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 @@ -102,12 +102,11 @@ public PersistAliasAnalysisGraphScope() { @Override @SuppressWarnings("unchecked") protected Graph.Scope readObject(Input in) throws IOException { - var flatten = in.readBoolean(); var childScopes = in.readInline(scala.collection.immutable.List.class); var occurrencesValues = (scala.collection.immutable.Set) in.readObject(); var occurrences = occurrencesValues.map(v -> Tuple2$.MODULE$.apply(v.id(), v)).toMap(null); var allDefinitions = in.readInline(scala.collection.immutable.List.class); - var parent = new Graph.Scope(flatten, childScopes, occurrences, allDefinitions); + var parent = new Graph.Scope(childScopes, occurrences, allDefinitions); var optionParent = Option.apply(parent); childScopes.forall( (object) -> { @@ -121,7 +120,6 @@ protected Graph.Scope readObject(Input in) throws IOException { @Override @SuppressWarnings("unchecked") protected void writeObject(Graph.Scope obj, Output out) throws IOException { - out.writeBoolean(obj.flattenToParent()); out.writeInline(scala.collection.immutable.List.class, obj.childScopes()); out.writeObject(obj.occurrences().values().toSet()); out.writeInline(scala.collection.immutable.List.class, obj.allDefinitions()); diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala index efc848971365..bcfb27dc053e 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala @@ -416,7 +416,7 @@ case object AliasAnalysis extends IRPass { ) case binding @ Expression.Binding(name, expression, _, _) => if ( - !parentScope.hasSymbolOccurrenceAs[GraphOccurrence.Def](name.name) + true // !parentScope.hasSymbolOccurrenceAs[GraphOccurrence.Def](name.name) ) { val isSuspended = expression match { case Expression.Block(_, _, _, isSuspended, _) => isSuspended @@ -450,7 +450,8 @@ case object AliasAnalysis extends IRPass { ) ) } else { - errors.Redefined.Binding(binding) + // errors.Redefined.Binding(binding) + binding } case app: Application => analyseApplication(app, graph, parentScope) @@ -787,7 +788,7 @@ case object AliasAnalysis extends IRPass { if (!isConstructorNameInPatternContext && !name.isMethod) { graph.resolveLocalUsage(occurrence) } else { - graph.resolveGlobalUsage(occurrence) + // graph.resolveGlobalUsage(occurrence) } } name.updateMetadata( @@ -810,9 +811,9 @@ case object AliasAnalysis extends IRPass { graph: Graph, parentScope: Scope ): IfThenElse = { - val condScope = parentScope.addChild(flattenToParent = true) - val trueScope = parentScope.addChild(flattenToParent = true) - val falseScope = parentScope.addChild(flattenToParent = true) + val condScope = parentScope; // .addChild() + val trueScope = parentScope; // .addChild() + val falseScope = parentScope; // .addChild() ir.copy( cond = analyseExpression(ir.cond, graph, condScope), trueBranch = analyseExpression(ir.trueBranch, graph, trueScope), diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala index df3579f680e6..6b56881ccef9 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala @@ -95,12 +95,7 @@ case object FramePointerAnalysis extends IRPass { } private def updateSymbolNames(e: IR, s: Graph.Scope): Unit = { - var symbols = s.allDefinitions.map(_.symbol) - s.childScopes.foreach(ch => - if (ch.flattenToParent) { - symbols ++= ch.allDefinitions.map(_.symbol) - } - ) + val symbols = s.allDefinitions.map(_.symbol) updateMeta(e, FrameVariableNames.create(symbols)) } @@ -365,27 +360,7 @@ case object FramePointerAnalysis extends IRPass { "Def occurrence must be in the given scope" ) ) - val parentOffset = if (scope.flattenToParent && scope.parent.nonEmpty) { - val p = scope.parent.get - var off = p.allDefinitions.size - val found = p.childScopes.find(ch => { - if (ch == scope) { - true - } else { - if (ch.flattenToParent) { - off += ch.allDefinitions.size - } - false - } - }) - org.enso.common.Asserts.assertInJvm( - found != null, - "Child must be found: " + scope + " among " + p.childScopes - ) - off - } else { - 0 - } + val parentOffset = 0 parentOffset + idxInScope + LocalScope.internalSlotsSize } @@ -402,9 +377,7 @@ case object FramePointerAnalysis extends IRPass { var currScope: Option[Graph.Scope] = Some(childScope) var scopeDistance = 0 while (currScope.isDefined && currScope.get != parentScope) { - if (!currScope.get.flattenToParent) { - scopeDistance += 1 - } + scopeDistance += 1 currScope = currScope.get.parent } scopeDistance diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala index 28d9615ce23a..167cc0dda57e 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala @@ -16,18 +16,14 @@ sealed class Graph( ) extends Serializable { private var sourceLinks: Map[Graph.Id, Set[Graph.Link]] = new HashMap() private var targetLinks: Map[Graph.Id, Set[Graph.Link]] = new HashMap() - private var frozen: Boolean = false { links.foreach(addSourceTargetLink) } - private var globalSymbols: Map[Graph.Symbol, GraphOccurrence.Global] = - Map() - /** @return the next counter value */ - def nextIdCounter: Int = _nextIdCounter + private[compiler] def nextIdCounter: Int = _nextIdCounter /** @return a deep structural copy of `this` */ def deepCopy( @@ -37,29 +33,15 @@ sealed class Graph( this.rootScope.deepCopy(scope_mapping), this.nextIdCounter ) - copy.links = this.links - copy.sourceLinks = this.sourceLinks - copy.targetLinks = this.targetLinks - copy.globalSymbols = this.globalSymbols + copy.links = this.links + copy.sourceLinks = this.sourceLinks + copy.targetLinks = this.targetLinks copy } def getLinks(): Set[Graph.Link] = links - def freeze(): Unit = { - frozen = true - } - - /** Registers a requested global symbol in the aliasing scope. - * - * @param sym the symbol occurrence - */ - def addGlobalSymbol(sym: GraphOccurrence.Global): Unit = { - org.enso.common.Asserts.assertInJvm(!frozen) - if (!globalSymbols.contains(sym.symbol)) { - globalSymbols = globalSymbols + (sym.symbol -> sym) - } - } + def freeze(): Unit = {} /** Creates a deep copy of the aliasing graph structure. * @@ -126,24 +108,6 @@ sealed class Graph( ) } - /** Resolves any links for the given usage of a symbol, assuming the symbol - * is global (i.e. method, constructor etc.) - * - * @param occurrence the symbol usage - * @return the link, if it exists - */ - def resolveGlobalUsage( - occurrence: GraphOccurrence.Use - ): Option[Graph.Link] = { - scopeFor(occurrence.id) match { - case Some(scope) => - globalSymbols - .get(occurrence.symbol) - .map(g => Graph.Link(occurrence.id, scope.scopesToRoot + 1, g.id)) - case None => None - } - } - /** Returns a string representation of the graph. * * @return a string representation of `this` @@ -359,7 +323,6 @@ object Graph { * Note that there may not be a link for all these definitions. */ sealed class Scope( - val flattenToParent: Boolean = false, var childScopes: List[Scope] = List(), var occurrences: Map[Id, GraphOccurrence] = HashMap(), var allDefinitions: List[GraphOccurrence.Def] = List() @@ -407,7 +370,6 @@ object Graph { ) val newScope = new Scope( - this.flattenToParent, childScopeCopies.toList, occurrences, allDefinitions @@ -442,8 +404,8 @@ object Graph { * @param is this scope "just virtual" and will be flatten to parent at the end? * @return a scope that is a child of `this` */ - def addChild(flattenToParent: Boolean = false): Scope = { - val scope = new Scope(flattenToParent) + def addChild(): Scope = { + val scope = new Scope() scope.parent = Some(this) childScopes ::= scope diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/Redefined.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/Redefined.scala index e375d7e1e179..c23a6d921d02 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/Redefined.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/Redefined.scala @@ -755,10 +755,13 @@ object Redefined { || diagnostics != this.diagnostics || id != this.id ) { + /* val res = Binding(invalidBinding, passData) res.diagnostics = diagnostics res.id = id res + */ + this } else this } From 4b9e9930dba0cd08baaea5e25c619756a3871066 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Tue, 29 Oct 2024 19:46:37 +0100 Subject: [PATCH 25/38] Cannot have argument b and local variable b anymore --- test/Base_Tests/src/Semantic/Instrumentor_Spec.enso | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Base_Tests/src/Semantic/Instrumentor_Spec.enso b/test/Base_Tests/src/Semantic/Instrumentor_Spec.enso index e9b762659496..05a7d22aa96a 100644 --- a/test/Base_Tests/src/Semantic/Instrumentor_Spec.enso +++ b/test/Base_Tests/src/Semantic/Instrumentor_Spec.enso @@ -2,7 +2,7 @@ from Standard.Base import all import Standard.Base.Data.Vector.Builder from Standard.Test import all -fib n b=1 = if n <= 1 then b else +fib n d=1 = if n <= 1 then d else a = fib n-1 b = fib n-2 a+b From 1b7ad3c8dbd5d1f6d42fbd3a0961dc4d00eccd30 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Tue, 29 Oct 2024 19:57:54 +0100 Subject: [PATCH 26/38] Give IfThenElseNode a location --- .../main/scala/org/enso/interpreter/runtime/IrToTruffle.scala | 4 +++- 1 file changed, 3 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 0f158c830d54..f9289e831354 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 @@ -1440,7 +1440,9 @@ class IrToTruffle( val trueNode = this.run(ife.trueBranch, subjectToInstrumentation) val falseNode = ife.falseBranch.map(this.run(_, subjectToInstrumentation)).orNull - IfThenElseNode.build(condNode, trueNode, falseNode) + val ifeNode = IfThenElseNode.build(condNode, trueNode, falseNode) + setLocation(ifeNode, ife.location) + ifeNode } /** Performs code generation for an Enso case expression. From 35ab5dc5cb5492aed96bc895635d412682909c33 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Tue, 29 Oct 2024 21:38:43 +0100 Subject: [PATCH 27/38] Avoiding t1 and t2 name clash --- .../src/Common_Table_Operations/Join/Join_Spec.enso | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Table_Tests/src/Common_Table_Operations/Join/Join_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Join/Join_Spec.enso index cc12db493316..33b80c969af8 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Join/Join_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Join/Join_Spec.enso @@ -479,9 +479,9 @@ add_join_specs suite_builder setup = Problems.assume_no_problems r2 if setup.supports_custom_objects then - t1 = table_builder [["X", [My_Type.Value 1 2, 2.0, 2]], ["Y", [10, 20, 30]]] - t2 = table_builder [["Z", [2.0, 1.5, 2.0]], ["W", [1, 2, 3]]] - r3 = t1.join t2 join_kind=Join_Kind.Inner on=(Join_Condition.Equals "X" "Z") on_problems=..Report_Warning + t8 = table_builder [["X", [My_Type.Value 1 2, 2.0, 2]], ["Y", [10, 20, 30]]] + t9 = table_builder [["Z", [2.0, 1.5, 2.0]], ["W", [1, 2, 3]]] + r3 = t8.join t9 join_kind=Join_Kind.Inner on=(Join_Condition.Equals "X" "Z") on_problems=..Report_Warning r3.column_names.should_equal ["X", "Y", "Z", "W"] r4 = r3.sort ["Y", "W"] r4.at "X" . to_vector . should_equal [2.0, 2.0, 2, 2] From c0658825174ce8fc8445288e3adab6c9b12bb7a3 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Tue, 5 Nov 2024 19:25:27 +0100 Subject: [PATCH 28/38] Use internal variable name --- .../org/enso/compiler/pass/analyse/alias/graph/Graph.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala index 869bde7d51bf..4d8c993bfc91 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala @@ -437,7 +437,7 @@ object Graph { * @param definition The definition to add. */ private[graph] def addDefinition(definition: GraphOccurrence.Def): Unit = { - _allDefinitions = allDefinitions ++ List(definition) + _allDefinitions = _allDefinitions ++ List(definition) } /** Finds an occurrence for the provided ID in the current scope, if it From d36fc2e834f7b0a0586905b962fb5e1d877fa572 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Wed, 6 Nov 2024 09:42:08 +0100 Subject: [PATCH 29/38] Assert currentFrame is not null --- .../node/scope/ReadLocalVariableNode.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/scope/ReadLocalVariableNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/scope/ReadLocalVariableNode.java index 1881a77f3fe8..4ee779798512 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/scope/ReadLocalVariableNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/scope/ReadLocalVariableNode.java @@ -45,10 +45,13 @@ public static ReadLocalVariableNode build(FramePointer pointer) { */ @Specialization(rewriteOn = FrameSlotTypeException.class) protected long readLong(VirtualFrame frame) throws FrameSlotTypeException { - if (getFramePointer().parentLevel() == 0) - return frame.getLong(getFramePointer().frameSlotIdx()); - MaterializedFrame currentFrame = getProperFrame(frame); - return currentFrame.getLong(getFramePointer().frameSlotIdx()); + var fp = getFramePointer(); + if (fp.parentLevel() == 0) { + return frame.getLong(fp.frameSlotIdx()); + } + var currentFrame = getProperFrame(frame); + assert currentFrame != null : "No frame for " + fp; + return currentFrame.getLong(fp.frameSlotIdx()); } /** @@ -96,9 +99,12 @@ public MaterializedFrame getParentFrame(Frame frame) { */ @ExplodeLoop public MaterializedFrame getProperFrame(Frame frame) { - MaterializedFrame currentFrame = getParentFrame(frame); - for (int i = 1; i < getFramePointer().parentLevel(); i++) { - currentFrame = getParentFrame(currentFrame); + var currentFrame = getParentFrame(frame); + var fp = getFramePointer(); + for (int i = 1; i < fp.parentLevel(); i++) { + var next = getParentFrame(currentFrame); + assert next != null : "No parent frame for " + fp; + currentFrame = next; } return currentFrame; } From bfc4969177fd744a074bd63e663c851620c40106 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Wed, 6 Nov 2024 09:42:40 +0100 Subject: [PATCH 30/38] Use GraphBuilder more --- .../enso/compiler/pass/analyse/test/AliasAnalysisTest.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala index a7dd4252b6c5..1997b776df13 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala @@ -19,7 +19,6 @@ import org.enso.compiler.pass.analyse.AliasAnalysis import org.enso.compiler.pass.analyse.alias.graph.Graph.Link import org.enso.compiler.pass.analyse.alias.AliasMetadata import org.enso.compiler.pass.analyse.alias.graph.{ - Graph, GraphBuilder, GraphOccurrence } @@ -90,9 +89,9 @@ class AliasAnalysisTest extends CompilerTest { "The analysis scope" should { val builder = GraphBuilder.create() - val flatScope = new Graph.Scope() + val flatScope = GraphBuilder.create().toScope(); - val complexScope = new Graph.Scope() + val complexScope = GraphBuilder.create().toScope(); val complexBuilder = GraphBuilder.create(null, complexScope) val child1 = complexBuilder.addChild() val child2 = complexBuilder.addChild() From aa93b06019f709a2d9c726d450719b49e86fdc5b Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 7 Nov 2024 07:55:39 +0100 Subject: [PATCH 31/38] Expanding AliasAnalysisTest with if/then section --- .../pass/analyse/PassPersistance.java | 6 +- .../analyse/alias/graph/GraphBuilder.java | 21 ++++- .../enso/compiler/context/LocalScope.scala | 4 +- .../pass/analyse/alias/graph/Graph.scala | 80 +++++++++++++++---- .../pass/analyse/test/AliasAnalysisTest.scala | 74 ++++++++++++++++- 5 files changed, 160 insertions(+), 25 deletions(-) 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 a9e72b2fa3c4..71364949e408 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 @@ -93,17 +93,18 @@ protected IgnoredBindings.State readObject(Input in) @org.openide.util.lookup.ServiceProvider(service = Persistance.class) public static final class PersistAliasAnalysisGraphScope extends Persistance { public PersistAliasAnalysisGraphScope() { - super(Graph.Scope.class, false, 1269); + super(Graph.Scope.class, false, 1270); } @Override @SuppressWarnings("unchecked") protected Graph.Scope readObject(Input in) throws IOException { + var flatten = in.readBoolean(); var childScopes = in.readInline(scala.collection.immutable.List.class); var occurrencesValues = (scala.collection.immutable.Set) in.readObject(); var occurrences = occurrencesValues.map(v -> Tuple2$.MODULE$.apply(v.id(), v)).toMap(null); var allDefinitions = in.readInline(scala.collection.immutable.List.class); - var parent = new Graph.Scope(childScopes, occurrences, allDefinitions); + var parent = new Graph.Scope(flatten, childScopes, occurrences, allDefinitions); childScopes.forall( (object) -> { var ch = (Graph.Scope) object; @@ -116,6 +117,7 @@ protected Graph.Scope readObject(Input in) throws IOException { @Override @SuppressWarnings("unchecked") protected void writeObject(Graph.Scope obj, Output out) throws IOException { + out.writeBoolean(obj.flattenToParent()); out.writeInline(scala.collection.immutable.List.class, obj.childScopes()); out.writeObject(obj.occurrences().values().toSet()); out.writeInline(scala.collection.immutable.List.class, obj.allDefinitions()); diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/alias/graph/GraphBuilder.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/alias/graph/GraphBuilder.java index fa77831512a4..a365f8b79777 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/alias/graph/GraphBuilder.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/alias/graph/GraphBuilder.java @@ -7,10 +7,12 @@ public final class GraphBuilder { private final Graph graph; private final Graph.Scope scope; + private final boolean flattenToParent; - private GraphBuilder(Graph graph, Graph.Scope scope) { + private GraphBuilder(Graph graph, Graph.Scope scope, boolean flattenToParent) { this.graph = graph; this.scope = scope; + this.flattenToParent = flattenToParent; } /** @@ -31,16 +33,27 @@ public static GraphBuilder create() { * @return builder operating on the graph {@code g} starting at scope {@code s} */ public static GraphBuilder create(Graph g, Graph.Scope s) { - return new GraphBuilder(g, s); + return new GraphBuilder(g, s, false); } /** - * Creates a child scope and returns a builder for it. + * Creates a child scope and returns a builder for it. Same as calling {@link #addChild(boolean)} + * with {@code false} argument. * * @return new builder for newly created scope, but the same graph */ public GraphBuilder addChild() { - return new GraphBuilder(graph, scope.addChild()); + return addChild(false); + } + + /** + * Creates a child scope and returns a builder for it. + * + * @param flattenToParent flatten definitions into parent + * @return new builder for newly created scope, but the same graph + */ + public GraphBuilder addChild(boolean flattenToParent) { + return new GraphBuilder(graph, scope.addChild(flattenToParent), flattenToParent); } /** diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/context/LocalScope.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/context/LocalScope.scala index 8618081070f5..d18715c711f8 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/context/LocalScope.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/context/LocalScope.scala @@ -58,7 +58,7 @@ class LocalScope( log: BiFunction[String, Array[Object], Void] ): java.util.List[String] = { def symbols(): java.util.List[String] = { - val r = scope.allDefinitions.map(_.symbol) + val r = scope.allDefinitionsWithFlattened.map(_.symbol) r.asJava } val meta = if (symbolsProvider == null) null else symbolsProvider() @@ -141,7 +141,7 @@ class LocalScope( * internal slots, that are prepended to every frame. */ private def gatherLocalFrameSlotIdxs(): Map[AliasGraph.Id, Int] = { - scope.allDefinitions.zipWithIndex.map { case (definition, i) => + scope.allDefinitionsWithFlattened.zipWithIndex.map { case (definition, i) => definition.id -> (i + LocalScope.internalSlotsSize) }.toMap } diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala index 4d8c993bfc91..f224f1dde438 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala @@ -4,6 +4,7 @@ package alias.graph import org.enso.compiler.core.CompilerError import org.enso.compiler.debug.Debug +import org.enso.compiler.pass.analyse.FramePointer import org.enso.compiler.pass.analyse.alias.graph.Graph.Scope import scala.collection.immutable.HashMap @@ -13,7 +14,7 @@ import scala.annotation.unused /** A graph containing aliasing information for a given root scope in Enso. */ sealed class Graph( - val rootScope: Graph.Scope = new Graph.Scope(), + val rootScope: Graph.Scope = new Graph.Scope(false), private var _nextIdCounter: Int = 0, private var links: Set[Graph.Link] = Set() ) { @@ -104,6 +105,48 @@ sealed class Graph( link }) } + final def resolveFramePointer( + occurrence: GraphOccurrence.Def + ): Option[FramePointer] = { + var useScope = scopeFor(occurrence.id) + while (useScope.nonEmpty) { + if (!useScope.get.flattenToParent) { + return useScope + .map(_.allDefinitionsWithFlattened.indexOf(occurrence)) + .filter(_ >= 0) + .map(new FramePointer(0, _)) + } + useScope = useScope.get.parent + } + None + } + + final def resolveFramePointer( + occurrence: GraphOccurrence.Use + ): Option[FramePointer] = { + val link = resolveLocalUsage(occurrence) + if (link.isEmpty) { + return None + } + var parentsUp = 0 + val useScope = scopeFor(occurrence.id) + if (useScope.nonEmpty) { + var currentScope = useScope.get + while (currentScope != null) { + if (!currentScope.flattenToParent) { + val at = currentScope.allDefinitionsWithFlattened.indexWhere( + _.id == link.get.target + ) + if (at >= 0) { + return Some(new FramePointer(parentsUp, at)) + } + parentsUp += 1 + } + currentScope = currentScope.parent.orNull + } + } + None + } private def addSourceTargetLink(link: Graph.Link): Unit = { // commented out: used from DebugEvalNode @@ -320,7 +363,8 @@ object Graph { * @param allDefinitions all definitions in this scope, including synthetic ones. * Note that there may not be a link for all these definitions. */ - sealed class Scope( + sealed class Scope private[Graph] ( + private[analyse] val flattenToParent: Boolean, private[Graph] var _childScopes: List[Scope] = List(), private[Graph] var _occurrences: Map[Id, GraphOccurrence] = HashMap(), private[Graph] var _allDefinitions: List[GraphOccurrence.Def] = List() @@ -328,10 +372,15 @@ object Graph { private[Graph] var _parent: Scope = null - def childScopes = _childScopes - def occurrences = _occurrences + def childScopes = _childScopes + def occurrences = _occurrences def allDefinitions = _allDefinitions - def parent = if (this._parent eq null) None else Some(_parent) + def allDefinitionsWithFlattened: List[GraphOccurrence.Def] = { + _allDefinitions ++ _childScopes + .filter(_.flattenToParent) + .flatMap(_.allDefinitionsWithFlattened) + } + def parent = if (this._parent eq null) None else Some(_parent) /** Counts the number of scopes from this scope to the root. * @@ -340,7 +389,8 @@ object Graph { * @return the number of scopes from this scope to the root */ private[analyse] def scopesToRoot: Int = { - parent.flatMap(scope => Some(scope.scopesToRoot + 1)).getOrElse(0) + val plusMine = if (flattenToParent) 0 else 1 + parent.flatMap(scope => Some(scope.scopesToRoot + plusMine)).getOrElse(0) } /** Sets the parent of the scope. @@ -375,9 +425,10 @@ object Graph { ) val newScope = new Scope( + flattenToParent, childScopeCopies.toList, - occurrences, - allDefinitions + _occurrences, + _allDefinitions ) mapping.put(this, newScope) newScope @@ -406,14 +457,13 @@ object Graph { /** Creates and returns a scope that is a child of this one. * - * @param is this scope "just virtual" and will be flatten to parent at the end? + * @param flattenToParent is this scope "just virtual" and will be flatten to parent at the end? * @return a scope that is a child of `this` */ - private[graph] def addChild(): Scope = { - val scope = new Scope() + private[graph] def addChild(flattenToParent: Boolean): Scope = { + val scope = new Scope(flattenToParent) scope._parent = this - _childScopes ::= scope - + _childScopes = _childScopes.appended(scope) scope } @@ -656,7 +706,7 @@ object Graph { } private def removeScopeFromParent(scope: Scope): Unit = { - _childScopes = childScopes.filter(_ != scope) + _childScopes = _childScopes.filter(_ != scope) } /** Disassociates this Scope from its parent. @@ -676,7 +726,7 @@ object Graph { * @param scopeCount the number of scopes that the link traverses * @param target the target ID of the link in the graph */ - sealed private[analyse] case class Link( + sealed private[analyse] case class Link private[Graph] ( source: Id, scopeCount: Int, target: Id diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala index 1997b776df13..29f5f217c3c9 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala @@ -89,9 +89,9 @@ class AliasAnalysisTest extends CompilerTest { "The analysis scope" should { val builder = GraphBuilder.create() - val flatScope = GraphBuilder.create().toScope(); + val flatScope = GraphBuilder.create().toScope() - val complexScope = GraphBuilder.create().toScope(); + val complexScope = GraphBuilder.create().toScope() val complexBuilder = GraphBuilder.create(null, complexScope) val child1 = complexBuilder.addChild() val child2 = complexBuilder.addChild() @@ -232,6 +232,76 @@ class AliasAnalysisTest extends CompilerTest { complexScope.scopesToRoot shouldEqual 0 } } + "if_then_doubled example" should { + val root = GraphBuilder.create() + + val ifCond1 = root.addChild() + val ifTrue1 = root.addChild(true) + + val ifCond2 = root.addChild() + val ifTrue2 = root.addChild(true) + + val aDef = root.newDef("a", genId, None) + root.add(aDef) + root.addDefinition(aDef) + + val aUse1 = ifCond1.newUse("a", genId, None) + ifCond1.add(aUse1) + val aUse2 = ifCond2.newUse("a", genId, None) + ifCond2.add(aUse2) + + val xDef1 = ifTrue1.newDef("x", genId, None) + ifTrue1.add(xDef1) + ifTrue1.addDefinition(xDef1) + val xUse1 = ifTrue1.newUse("x", genId, None) + ifTrue1.add(xUse1) + + val xDef2 = ifTrue1.newDef("x", genId, None) + ifTrue2.add(xDef2) + ifTrue2.addDefinition(xDef2) + val xUse2 = ifTrue2.newUse("x", genId, None) + ifTrue2.add(xUse2) + + val rootScope = root.toScope() + val rootGraph = root.toGraph() + + "have four subscopes" in { + rootScope.scopeCount shouldEqual 5 + } + + "frame needs two slots for x" in { + val all = rootScope.allDefinitionsWithFlattened + all shouldEqual List(aDef, xDef1, xDef2) + } + + "are uses different" in { + xUse1.id should not equal xUse2.id + } + + "frame pointer for definition of a" in { + val fp = rootGraph.resolveFramePointer(aDef) + fp.get.parentLevel shouldEqual 0 + fp.get.frameSlotIdx shouldEqual 0 + } + + "xUse1 uses xDef1" in { + val link = rootGraph.resolveLocalUsage(xUse1) + link.get.target shouldEqual xDef1.id + + val fp = rootGraph.resolveFramePointer(xUse1) + fp.get.parentLevel.shouldEqual(0) + fp.get.frameSlotIdx.shouldEqual(1) + } + + "xUse2 uses xDef2" in { + val link = rootGraph.resolveLocalUsage(xUse2) + link.get.target shouldEqual xDef2.id + + val fp = rootGraph.resolveFramePointer(xUse2) + fp.get.parentLevel.shouldEqual(0) + fp.get.frameSlotIdx.shouldEqual(2) + } + } "The Aliasing graph" should { val builder = GraphBuilder.create() From fbe2324651d6f1d6658dc8176f5a98645ef44ced Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 7 Nov 2024 09:59:11 +0100 Subject: [PATCH 32/38] scalafmtAll --- .../org/enso/compiler/pass/analyse/alias/graph/Graph.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala index f224f1dde438..ea48c6ba8f43 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala @@ -372,8 +372,8 @@ object Graph { private[Graph] var _parent: Scope = null - def childScopes = _childScopes - def occurrences = _occurrences + def childScopes = _childScopes + def occurrences = _occurrences def allDefinitions = _allDefinitions def allDefinitionsWithFlattened: List[GraphOccurrence.Def] = { _allDefinitions ++ _childScopes From 933d74ca7e865006e5e108e0dc7ff4e4d07b084c Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 14 Dec 2024 05:10:20 +0100 Subject: [PATCH 33/38] require non null trueBranch --- .../org/enso/interpreter/node/controlflow/IfThenElseNode.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java index 683423fad930..b8b13f5eec30 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java @@ -35,7 +35,7 @@ public final class IfThenElseNode extends ExpressionNode { IfThenElseNode(ExpressionNode cond, ExpressionNode trueBranch, ExpressionNode falseBranch) { this.cond = Objects.requireNonNull(cond); - this.trueBranch = trueBranch; + this.trueBranch = Objects.requireNonNull(trueBranch); this.falseBranch = falseBranch; this.with = IfThenElseNodeFactory.WithConditionNodeGen.create(); } From 71cedf2046335d4259431dbac984f6ef30e94afe Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 14 Dec 2024 05:12:30 +0100 Subject: [PATCH 34/38] executeAppend with frame --- .../org/enso/interpreter/node/controlflow/IfThenElseNode.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java index b8b13f5eec30..30c43186d9bd 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java @@ -117,7 +117,7 @@ final Object doWarning( var without = warnings.removeWarnings(cond); var result = delegate.executeIf(frame, without, trueBranch, falseBranch); - return appendWarningNode.executeAppend(null, result, ws); + return appendWarningNode.executeAppend(frame, result, ws); } catch (UnsupportedMessageException e) { var ctx = EnsoContext.get(this); throw ctx.raiseAssertionPanic(this, null, e); From f2d5574ca2a02ade8ab1fce5c16ac79b5e0ea2a6 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 14 Dec 2024 05:21:25 +0100 Subject: [PATCH 35/38] Note applies to both case and if/then/else --- .../java/org/enso/compiler/pass/analyse/TailCall.java | 10 +++++----- .../compiler/test/pass/analyse/TailCallMegaPass.scala | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/TailCall.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/TailCall.java index 55a19f2996bb..015e63c02c4a 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/TailCall.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/TailCall.java @@ -210,7 +210,7 @@ public Expression transformExpression(Expression ir) { case IfThenElse ite -> { if (isInTailPos) { markAsTail(ite); - // Note [Analysing Branches in Case Expressions] + // Note [Analysing Branches in Conditional Expressions] markAsTail(ite.trueBranch()); if (ite.falseBranchOrNull() != null) { markAsTail(ite.falseBranchOrNull()); @@ -220,7 +220,7 @@ public Expression transformExpression(Expression ir) { case Case.Expr e -> { if (isInTailPos) { markAsTail(ir); - // Note [Analysing Branches in Case Expressions] + // Note [Analysing Branches in Conditional Expressions] e.branches().foreach(b -> markAsTail(b)); } } @@ -259,7 +259,7 @@ private void collectTailCandidatesExpression( case Function function -> collectTailCandicateFunction(function, tailCandidates); case IfThenElse ite -> { if (isInTailPos) { - // Note [Analysing Branches in Case Expressions] + // Note [Analysing Branches in Conditional Expressions] tailCandidates.put(ite.trueBranch(), true); if (ite.falseBranchOrNull() != null) { tailCandidates.put(ite.falseBranchOrNull(), true); @@ -365,7 +365,7 @@ private void collectTailCandidatesCase( switch (caseExpr) { case Case.Expr expr -> { if (isInTailPos) { - // Note [Analysing Branches in Case Expressions] + // Note [Analysing Branches in Conditional Expressions] expr.branches() .foreach( b -> { @@ -378,7 +378,7 @@ private void collectTailCandidatesCase( } } - /* Note [Analysing Branches in Case Expressions] + /* Note [Analysing Branches in Conditional Expressions] * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * When performing tail call analysis on a case expression it is very * important to recognise that the branches of a case expression should all diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/TailCallMegaPass.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/TailCallMegaPass.scala index 7758bda458a5..4819b1e9a821 100644 --- a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/TailCallMegaPass.scala +++ b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/analyse/TailCallMegaPass.scala @@ -361,7 +361,7 @@ case object TailCallMegaPass extends IRPass { caseExpr .copy( scrutinee = analyseExpression(scrutinee, isInTailPosition = false), - // Note [Analysing Branches in Case Expressions] + // Note [Analysing Branches in Conditional Expressions] branches = branches.map(analyseCaseBranch(_, isInTailPosition)) ) case _: Case.Branch => @@ -370,9 +370,9 @@ case object TailCallMegaPass extends IRPass { updateMetaIfInTailPosition(isInTailPosition, newCaseExpr) } - /* Note [Analysing Branches in Case Expressions] + /* Note [Analysing Branches in Conditional Expressions] * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - * When performing tail call analysis on a case expression it is very + * When performing tail call analysis on a case or if/then/else expression it is very * important to recognise that the branches of a case expression should all * have the same tail call state. The branches should only be marked as being * in tail position when the case expression _itself_ is in tail position. From 63dde4e9a983b467ffaa2bd4fb0feb94e73bb402 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 14 Dec 2024 06:14:07 +0100 Subject: [PATCH 36/38] Trying to mimic failure in Index_Sub_Range --- .../org/enso/compiler/test/IfThenElseTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java index ecbead554312..71c8e82d2941 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/IfThenElseTest.java @@ -68,6 +68,20 @@ public void simpleIfThen() throws Exception { assertTrue("Expect Nothing", check.execute(false).isNull()); } + @Test + public void indexSubRange() throws Exception { + var code = + """ + check step first = case step of + _ -> "Every " + step.to_display_text + (if first == 0 then "" else " from " + first.to_display_text) + """; + + var check = ContextUtils.getMethodFromModule(ctx, code, "check"); + + assertEquals("Every Prefix", check.execute("Prefix", 0).asString()); + assertEquals("Every Count from 1", check.execute("Count", 1).asString()); + } + @Test public void variableDefinedInThen() throws Exception { var code = """ From 46e21af7a082717da0cc6d526d16ec1438311346 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 14 Dec 2024 06:36:59 +0100 Subject: [PATCH 37/38] Associate runModule exceptions with name of the module --- .../scala/org/enso/compiler/pass/PassManager.scala | 14 +++++++++++++- 1 file changed, 13 insertions(+), 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..75c9cb4842d0 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 @@ -96,7 +96,19 @@ class PassManager( (factory, ctx) => factory.createForModuleCompilation(ctx), miniPassCompile = (miniPass, ir) => MiniIRPass.compile[Module](classOf[Module], ir, miniPass), - megaPassCompile = (megaPass, ir, ctx) => megaPass.runModule(ir, ctx) + megaPassCompile = (megaPass, ir, ctx) => + try { + megaPass.runModule(ir, ctx) + } catch { + case ex: Exception => + val newEx = new IllegalStateException( + "Error processing " + ctx.module.getName + "\n" + ex + .getClass() + .getName() + ": " + ex.getMessage + ) + newEx.setStackTrace(ex.getStackTrace) + throw newEx + } ) } From 41162aa02cbd11280d52af1d9435a1f057687d25 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 14 Dec 2024 08:05:09 +0100 Subject: [PATCH 38/38] Sharing of an IR at multiple places is common. Only detect cases when FrameAnalysisMeta is different. --- .../pass/analyse/FramePointerAnalysis.scala | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala index c9d132585204..6768bf07dd2c 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala @@ -323,16 +323,36 @@ case object FramePointerAnalysis extends IRPass { ir: IR, newMeta: FrameAnalysisMeta ): Unit = { + + def toString(ir: IR): String = { + ir.getClass().getName() + "@" + Integer.toHexString( + System.identityHashCode(ir) + ) + } + + ir match { + case ca: CallArgument + if ca.location.isDefined && ca.location.orNull.start == 1946 && ca.location.orNull.end == 1950 => + val ex = new Exception("Assigning ca: " + ca) + ex.setStackTrace(ex.getStackTrace().slice(0, 20)) + ex.printStackTrace() + case _ => + } ir.passData().get(this) match { case None => ir.passData() .update(this, newMeta) case Some(meta) => - val ex = new IllegalStateException( - "Unexpected FrameAnalysisMeta associated with IR " + ir + "\nOld: " + meta + " new " + newMeta - ) - ex.setStackTrace(ex.getStackTrace().slice(0, 10)) - throw ex + if (meta != newMeta) { + val ex = new IllegalStateException( + "Unexpected FrameAnalysisMeta associated with IR " + toString( + ir + ) + "\n" + ir + "\nOld: " + meta + " new " + newMeta + ) + ex.setStackTrace(ex.getStackTrace().slice(0, 10)) + ex.printStackTrace() + // throw ex + } } }