Recognize if-like Tree.MultiSegmentApp as IfThenElse IR

CHANGELOG.md

@@ -41,6 +41,12 @@
 
 [11374]: https://github.com/enso-org/enso/pull/11374
 
+#### 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

engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java

@@ -96,7 +96,7 @@ protected IgnoredBindings.State readObject(Input in)
   @org.openide.util.lookup.ServiceProvider(service = Persistance.class)
   public static final class PersistAliasAnalysisGraphScope extends Persistance<Graph.Scope> {
     public PersistAliasAnalysisGraphScope() {
-      super(Graph.Scope.class, false, 1267);
+      super(Graph.Scope.class, false, 1269);
     }
 
     @Override

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<IR, Boolean> 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);

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()
@@ -414,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
@@ -448,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)
@@ -794,14 +797,37 @@ 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 = {
+    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),
+      falseBranchOrNull =
+        ir.falseBranch.map(analyseExpression(_, graph, falseScope)).orNull
+    )
+  }
+
   /** Performs alias analysis on a case expression.
     *
     * @param ir          the case expression to analyse
     * @param graph       the graph in which the analysis is taking place
     * @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

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

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

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 =>
@@ -356,7 +360,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 +377,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
   }

engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala

@@ -26,7 +26,7 @@ sealed class Graph(
 
   /** @return the next counter value
     */
-  def nextIdCounter: Int = _nextIdCounter
+  private[compiler] def nextIdCounter: Int = _nextIdCounter
 
   /** @return a deep structural copy of `this` */
   final def deepCopy(
@@ -364,7 +364,11 @@ object Graph {
             childScopeCopies += scope.deepCopy(mapping)
           )
           val newScope =
-            new Scope(childScopeCopies.toList, occurrences, allDefinitions)
+            new Scope(
+              childScopeCopies.toList,
+              occurrences,
+              allDefinitions
+            )
           mapping.put(this, newScope)
           newScope
       }
@@ -392,6 +396,7 @@ 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`
       */
     final def addChild(): Scope = {

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 =
@@ -296,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());
+  }
 }