diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bd67d42f733..f12148ad94f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -241,6 +241,12 @@ [11374]: https://github.com/enso-org/enso/pull/11374 [11671]: https://github.com/enso-org/enso/pull/11671 +#### 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 diff --git a/build.sbt b/build.sbt index 35db530cef79..cd8e259bc1da 100644 --- a/build.sbt +++ b/build.sbt @@ -2603,7 +2603,18 @@ lazy val publishLocalSetting: SettingsDefinition = Seq( packageSrc / publishArtifact := false ) -def customFrgaalJavaCompilerSettings(targetJdk: String) = { +def customFrgaalJavaCompilerSettings( + targetJdk: String, + useSystemPath: Boolean = false +) = { + var opts = Seq( + "-source", + frgaalSourceLevel, + "--enable-preview" + ) + if (useSystemPath) { + opts = opts ++ Seq("-XDignore.symbol.file") + } // There might be slightly different Frgaal compiler configuration for // both Compile and Test configurations Seq(Compile, Test).flatMap { config => @@ -2634,11 +2645,7 @@ def customFrgaalJavaCompilerSettings(targetJdk: String) = { // accessible from the non-meta build definition. libraryDependencies += FrgaalJavaCompiler.frgaal, // Ensure that our tooling uses the right Java version for checking the code. - Compile / javacOptions ++= Seq( - "-source", - frgaalSourceLevel, - "--enable-preview" - ) + Compile / javacOptions ++= opts ) } @@ -3449,6 +3456,7 @@ lazy val `runtime-compiler-dump-igv` = (project in file("engine/runtime-compiler-dump-igv")) .enablePlugins(JPMSPlugin) .settings( + customFrgaalJavaCompilerSettings("21", true), scalaModuleDependencySetting, javaModuleName := "org.enso.runtime.compiler.dump.igv", Compile / internalModuleDependencies := { diff --git a/engine/runtime-compiler-dump-igv/src/main/java/org/enso/compiler/dump/igv/EnsoModuleAST.java b/engine/runtime-compiler-dump-igv/src/main/java/org/enso/compiler/dump/igv/EnsoModuleAST.java index 5d756fe3a4f7..fdbcfa76d9d2 100644 --- a/engine/runtime-compiler-dump-igv/src/main/java/org/enso/compiler/dump/igv/EnsoModuleAST.java +++ b/engine/runtime-compiler-dump-igv/src/main/java/org/enso/compiler/dump/igv/EnsoModuleAST.java @@ -22,6 +22,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.Data; import org.enso.compiler.core.ir.module.scope.Export; @@ -304,6 +305,19 @@ private ASTNode buildTree(Expression expression) { } yield caseNode; } + case IfThenElse ife -> { + Map props = Map.of("hasElseBranch", ife.falseBranch().isDefined()); + var ifeNode = newNode(ife, props); + var condNode = buildTree(ife.cond()); + var trueNode = buildTree(ife.trueBranch()); + createEdge(ifeNode, condNode, "condition"); + createEdge(ifeNode, trueNode, "trueBranch"); + if (ife.falseBranch().isDefined()) { + var falseNode = buildTree(ife.falseBranchOrNull()); + createEdge(ifeNode, falseNode, "falseBranch"); + } + yield ifeNode; + } case Case.Branch caseBranch -> { Map props = Map.of("isTerminal", caseBranch.terminalBranch()); var branchNode = newNode(caseBranch, props); @@ -467,6 +481,8 @@ private ASTNode newNode(IR ir, Map props) { return existingNode; } var node = bldr.build(); + assert !nodes.containsKey(node.getId()) + : "Previous node: " + nodes.get(node.getId()) + " new node: " + node; nodes.put(node.getId(), node); if (currentBlockBldr() != null) { currentBlockBldr().addNode(node); 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 3a63743b5f87..c21f2d947949 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 @@ -97,17 +97,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, 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; @@ -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..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 @@ -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,10 +207,20 @@ 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 Conditional Expressions] + markAsTail(ite.trueBranch()); + if (ite.falseBranchOrNull() != null) { + markAsTail(ite.falseBranchOrNull()); + } + } + } 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)); } } @@ -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 Conditional 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); @@ -345,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 -> { @@ -358,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-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/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index d246cb3a3e8d..226076f257b4 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 @@ -98,7 +98,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 + } ) } 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 b021ca0c0afa..1773e63ea506 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 } @@ -385,7 +386,8 @@ case object AliasAnalysis extends IRPass { isConstructorNameInPatternContext = false, builder ) - case cse: Case => analyseCase(cse, builder) + case ife: IfThenElse => analyseIfThenElse(ife, builder) + case cse: Case => analyseCase(cse, builder) case block: Expression.Block => val currentScope = if (!block.suspended) builder else builder.addChild() @@ -752,6 +754,28 @@ case object AliasAnalysis extends IRPass { ) } + /** Performs alias analysis on a if then else expression. + * + * @param ir the expression to analyse + * @param builder the graph builder + * @return `ir`, possibly with alias analysis information attached + */ + private def analyseIfThenElse( + ir: IfThenElse, + builder: GraphBuilder + ): IfThenElse = { + val condScope = builder; // .addChild() + val trueScope = builder; // .addChild() + val falseScope = builder; // .addChild() + ir.copy( + cond = analyseExpression(ir.cond, condScope), + trueBranch = analyseExpression(ir.trueBranch, trueScope), + falseBranchOrNull = ir.falseBranch.map { + analyseExpression(_, falseScope) + }.orNull + ) + } + /** Performs alias analysis on a case expression. * * @param ir the case expression to analyse @@ -759,7 +783,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, builder: GraphBuilder ): Case = { 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 fba9ccbd01b8..4573263051a1 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,31 @@ 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 = { + 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. * * 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 c51791cbdf14..485b1cfc7c66 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 @@ -21,6 +21,7 @@ import org.enso.compiler.core.ir.expression.{ Comment, Error, Foreign, + IfThenElse, Operator } import org.enso.compiler.core.CompilerError @@ -115,6 +116,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, _, _, _) => @@ -316,6 +319,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-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 706993c3225f..6e704cdfb8c9 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 => @@ -318,16 +322,27 @@ case object FramePointerAnalysis extends IRPass { ir: IR, newMeta: FrameAnalysisMeta ): Unit = { + + def toString(ir: IR): String = { + ir.getClass().getName() + "@" + Integer.toHexString( + System.identityHashCode(ir) + ) + } 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 + } } } @@ -356,7 +371,8 @@ case object FramePointerAnalysis extends IRPass { "Def occurrence must be in the given scope" ) ) - idxInScope + LocalScope.internalSlotsSize + val parentOffset = 0 + parentOffset + idxInScope + LocalScope.internalSlotsSize } /** Returns the *scope distance* of the given `childScope` to the given `parentScope`. @@ -372,8 +388,8 @@ case object FramePointerAnalysis extends IRPass { var currScope: Option[Graph.Scope] = Some(childScope) var scopeDistance = 0 while (currScope.isDefined && currScope.get != parentScope) { - currScope = currScope.get.parent 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 c9acc168c9a1..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 @@ -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() @@ -331,7 +375,12 @@ object Graph { 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. @@ -374,7 +424,12 @@ object Graph { childScopeCopies += scope.deepCopy(mapping) ) val newScope = - new Scope(childScopeCopies.toList, occurrences, allDefinitions) + new Scope( + flattenToParent, + childScopeCopies.toList, + _occurrences, + _allDefinitions + ) mapping.put(this, newScope) newScope } @@ -402,13 +457,13 @@ object Graph { /** Creates and returns a scope that is a child of this one. * + * @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 } @@ -432,7 +487,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 @@ -651,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. @@ -671,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-compiler/src/main/scala/org/enso/compiler/pass/lint/UnusedBindings.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/lint/UnusedBindings.scala index 1b27dafa757e..e02590b4e252 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/lint/UnusedBindings.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/lint/UnusedBindings.scala @@ -34,6 +34,7 @@ case object UnusedBindings extends IRPass { override type Config = IRPass.Configuration.Default override lazy val precursorPasses: Seq[IRProcessingPass] = List( + AliasAnalysis, ComplexType, GenerateMethodBodies, IgnoredBindings, 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 d393ca366be1..ee1dbc02d0bd 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 { @@ -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 = """ @@ -96,6 +110,44 @@ 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"); + var res = check.execute(true); + fail("The code should not compile, but returned: " + res); + } catch (PolyglotException ex) { + MatcherAssert.assertThat( + ex.getMessage(), Matchers.containsString("name `xt` could not be found")); + } + } + @Test public void variableUsedAfterTheBranch() throws Exception { try { @@ -110,7 +162,8 @@ public void variableUsedAfterTheBranch() throws Exception { """; var check = ContextUtils.getMethodFromModule(ctx, code, "check"); - fail("Expecting error, but got: " + check); + var res = check.execute(false); + fail("Expecting error, but got: " + res); } catch (PolyglotException ex) { assertThat( ex.getMessage(), @@ -160,7 +213,6 @@ public void javaScriptBooleanIsSupported() throws Exception { assertEquals("No", check.execute("Ne").asString()); } - @Ignore @Test public void truffleObjectConvertibleToBooleanIsSupported() throws Exception { var code = @@ -296,4 +348,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()); + } } 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")); } } 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 d71bb2d680f3..a15f127c2c81 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() @@ -233,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() 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 0d0bfcd392bc..2edb56936e6f 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 @@ -362,7 +362,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 => @@ -371,9 +371,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. 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 86218bf57379..119e0269cd82 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,10 +239,15 @@ class TailCallTest extends MiniPassTest { fnBody .asInstanceOf[Expression.Block] .returnValue + .asInstanceOf[IfThenElse] + .falseBranch + .get + /* .asInstanceOf[Application.Prefix] .arguments .apply(2) .value + */ .asInstanceOf[Expression.Block] .returnValue .asInstanceOf[Application.Prefix] 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 7f0fa88c8660..5d38ff84f5f0 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] 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 14e2fd759503..77b62125e929 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 @@ -26,6 +26,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; @@ -918,6 +919,20 @@ 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); + 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) { + 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/java/org/enso/compiler/core/ir/IrPersistance.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/IrPersistance.java index 4132ba57a683..5bb1917750c2 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 @@ -8,6 +8,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; @@ -49,6 +50,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/java/org/enso/compiler/core/ir/expression/IfThenElse.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/expression/IfThenElse.java new file mode 100644 index 000000000000..0b82c00b0bfd --- /dev/null +++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/expression/IfThenElse.java @@ -0,0 +1,64 @@ +package org.enso.compiler.core.ir.expression; + +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.IdentifiedLocation; +import org.enso.compiler.core.ir.MetadataStorage; +import org.enso.runtime.parser.dsl.GenerateFields; +import org.enso.runtime.parser.dsl.GenerateIR; +import org.enso.runtime.parser.dsl.IRChild; + +/** Enso if/then(else) expression. */ +@GenerateIR(interfaces = {Expression.class}) +public final class IfThenElse extends IfThenElseGen { + @GenerateFields + public IfThenElse( + @IRChild Expression condition, + @IRChild Expression trueBranch, + @IRChild(required = false) Expression falseBranchOrNull, + IdentifiedLocation identifiedLocation, + MetadataStorage passData) { + super(condition, trueBranch, falseBranchOrNull, identifiedLocation, passData); + } + + public Expression cond() { + return condition(); + } + + public IfThenElse copy(Expression cond) { + return super.copy( + this.diagnostics, + this.passData, + this.location, + this.id, + cond, + this.trueBranch(), + this.falseBranchOrNull()); + } + + public IfThenElse copy(Expression cond, Expression trueBranch, Expression falseBranchOrNull) { + return super.copy( + this.diagnostics, + this.passData, + this.location, + this.id, + cond, + trueBranch, + falseBranchOrNull); + } + + public scala.Option falseBranch() { + return scala.Option.apply(falseBranchOrNull()); + } + + @Override + public String showCode(int indent) { + var newIndent = indent + indentLevel; + var headerStr = "if " + cond().showCode(0) + " then\n" + trueBranch().showCode(newIndent); + var elseStr = + switch (falseBranchOrNull()) { + case Expression f -> " ".repeat(indent) + "else\n" + f.showCode(newIndent); + case null -> ""; + }; + return headerStr + "\n" + elseStr; + } +} 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..30c43186d9bd --- /dev/null +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/controlflow/IfThenElseNode.java @@ -0,0 +1,154 @@ +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.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; +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; +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.") +public final class IfThenElseNode extends ExpressionNode { + private @Child ExpressionNode cond; + private @Child ExpressionNode trueBranch; + private @Child ExpressionNode falseBranch; + private @Child WithCondition with; + + IfThenElseNode(ExpressionNode cond, ExpressionNode trueBranch, ExpressionNode falseBranch) { + this.cond = Objects.requireNonNull(cond); + this.trueBranch = Objects.requireNonNull(trueBranch); + this.falseBranch = falseBranch; + this.with = IfThenElseNodeFactory.WithConditionNodeGen.create(); + } + + public static IfThenElseNode build(ExpressionNode c, ExpressionNode t, ExpressionNode fOrNull) { + return new IfThenElseNode(c, t, fOrNull); + } + + @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 { + 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); + } + + @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( + 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); + + var result = delegate.executeIf(frame, without, trueBranch, falseBranch); + return appendWarningNode.executeAppend(frame, result, ws); + } catch (UnsupportedMessageException e) { + var ctx = EnsoContext.get(this); + throw ctx.raiseAssertionPanic(this, null, e); + } + } + + @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 010550d42f0c..d75dfd27bb6e 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 @@ -13,7 +13,9 @@ import com.oracle.truffle.api.profiles.CountingConditionProfile; import org.enso.interpreter.node.ExpressionNode; import org.enso.interpreter.runtime.EnsoContext; -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; @@ -63,7 +65,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; } @@ -75,13 +77,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, @@ -109,7 +111,7 @@ Object doWarning( "!warnings.hasWarnings(object)" }) @ExplodeLoop - public Object doMatch( + final Object doMatch( VirtualFrame frame, Object object, @Shared("warnsLib") @CachedLibrary(limit = "3") WarningsLibrary warnings) { 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 f96024654fa9..c5fee5788446 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 @@ -15,12 +15,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, 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 b9a231579d40..df5b735896d7 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/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; } 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 b8c64198afb0..4360896a9f6e 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 @@ -35,6 +35,7 @@ import org.enso.compiler.core.ir.expression.{ Comment, Error, Foreign, + IfThenElse, Operator, Section } @@ -73,6 +74,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.foreign.HostValueToEnsoNode import org.enso.interpreter.node.expression.builtin.BuiltinRootNode @@ -1268,6 +1270,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 => @@ -1389,12 +1393,27 @@ 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 + val ifeNode = IfThenElseNode.build(condNode, trueNode, falseNode) + setLocation(ifeNode, ife.location) + ifeNode + } + /** 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 = diff --git a/test/Base_Internal_Tests/src/Instrumentor_Spec.enso b/test/Base_Internal_Tests/src/Instrumentor_Spec.enso index b12a88d63bb1..4088eb7475cc 100644 --- a/test/Base_Internal_Tests/src/Instrumentor_Spec.enso +++ b/test/Base_Internal_Tests/src/Instrumentor_Spec.enso @@ -3,7 +3,7 @@ import Standard.Base.Internal.Instrumentor 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 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 af7a04900cff..2188a22844fb 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 @@ -486,9 +486,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]