Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Recognize if-like Tree.MultiSegmentApp as IfThenElse IR #11365

Draft
wants to merge 55 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
6abf764
Recognize if-like Tree.MultiSegmentApp as IfThenElse IR
JaroslavTulach Oct 19, 2024
f5ca8e0
Converting IfThenElse IR to case statement
JaroslavTulach Oct 20, 2024
f998708
Support if_then without else
JaroslavTulach Oct 20, 2024
fba42c5
Register Literal.Bool and Empty for persistance
JaroslavTulach Oct 20, 2024
7c7c5b3
Note in changelog
JaroslavTulach Oct 20, 2024
94b07b4
Handle missing else branch without NPE
JaroslavTulach Oct 20, 2024
f6f0938
Testing behavior of a variable being defined in branch
JaroslavTulach Oct 24, 2024
3a2906b
Variable cannot be use after the if branch is over
JaroslavTulach Oct 24, 2024
3a62199
Bringing in changes from develop, mainly #11395
JaroslavTulach Oct 25, 2024
ffab938
Removing changes related to IfThenElseToCase IR conversions
JaroslavTulach Oct 25, 2024
382a0df
Processing IfThenElse IR via controlflow.IfThenElseNode
JaroslavTulach Oct 25, 2024
6b6be17
IfThenElse can access local variables
JaroslavTulach Oct 25, 2024
2835bb4
IfThenElse uses addChild(flattenToParent=true) to process, but isolat…
JaroslavTulach Oct 25, 2024
97c90b8
@Specialization methods don't have to be public
JaroslavTulach Oct 25, 2024
3420d33
Using @Specialization to handle various IfThenElseNode situations
JaroslavTulach Oct 25, 2024
063e7f0
Handling Warnings WithCondition node
JaroslavTulach Oct 25, 2024
4f4fa1d
javafmtAll
JaroslavTulach Oct 25, 2024
c2c6b24
Test showing that definition of a variable in if branch overrides global
JaroslavTulach Oct 26, 2024
039ba40
Fixing typo
JaroslavTulach Oct 26, 2024
9f75eb1
No need for boolean literal right now
JaroslavTulach Oct 26, 2024
dbaf5f3
Recalculate offset when flattenToParent
JaroslavTulach Oct 26, 2024
e852d8a
Adjust the test to IfThenElse element
JaroslavTulach Oct 26, 2024
00a69f7
Adjusting the stacktrace: implementation details disappear with IfThe…
JaroslavTulach Oct 26, 2024
c440305
Use if_then function to get mixfix functions semantics
JaroslavTulach Oct 26, 2024
9c747d2
Towards single Scope for all if-then-else branches
JaroslavTulach Oct 28, 2024
546e103
Merging with most recent develop and resolving clashes with #11419
JaroslavTulach Oct 29, 2024
4b9e993
Cannot have argument b and local variable b anymore
JaroslavTulach Oct 29, 2024
1b7ad3c
Give IfThenElseNode a location
JaroslavTulach Oct 29, 2024
35ab5dc
Avoiding t1 and t2 name clash
JaroslavTulach Oct 29, 2024
6ab657d
Merge remote-tracking branch 'origin/develop' into wip/jtulach/IfThen…
JaroslavTulach Nov 1, 2024
0224c00
Merging with GraphBuilder in develop
JaroslavTulach Nov 5, 2024
c065882
Use internal variable name
JaroslavTulach Nov 5, 2024
d36fc2e
Assert currentFrame is not null
JaroslavTulach Nov 6, 2024
bfc4969
Use GraphBuilder more
JaroslavTulach Nov 6, 2024
aa93b06
Expanding AliasAnalysisTest with if/then section
JaroslavTulach Nov 7, 2024
fbe2324
scalafmtAll
JaroslavTulach Nov 7, 2024
9f6d277
Merge remote-tracking branch 'origin/develop' into wip/jtulach/IfThen…
JaroslavTulach Nov 22, 2024
e2a35e1
Merge remote-tracking branch 'origin/develop' into wip/jtulach/IfThen…
JaroslavTulach Dec 14, 2024
933d74c
require non null trueBranch
JaroslavTulach Dec 14, 2024
71cedf2
executeAppend with frame
JaroslavTulach Dec 14, 2024
f2d5574
Note applies to both case and if/then/else
JaroslavTulach Dec 14, 2024
63dde4e
Trying to mimic failure in Index_Sub_Range
JaroslavTulach Dec 14, 2024
46e21af
Associate runModule exceptions with name of the module
JaroslavTulach Dec 14, 2024
41162aa
Sharing of an IR at multiple places is common. Only detect cases when…
JaroslavTulach Dec 14, 2024
25bc40f
Merge remote-tracking branch 'origin/develop' into wip/jtulach/IfThen…
JaroslavTulach Feb 3, 2025
bd73fcd
Using system path when compiling runtime-compiler-dump-igv
JaroslavTulach Feb 3, 2025
ad7b2a7
Use == and not equals when comparing passData
JaroslavTulach Feb 3, 2025
2c09be4
Actually invoke the check function
JaroslavTulach Feb 3, 2025
1cab364
Dump IfThenElse into IGV structure
JaroslavTulach Feb 3, 2025
30b2740
Removing dump statement
JaroslavTulach Feb 3, 2025
40edc67
Writing IfThenElse in Java. Minus 158 lines of Scala. Plus 52 lines o…
JaroslavTulach Feb 4, 2025
fbdd31c
Must implement showCode
JaroslavTulach Feb 4, 2025
1b990f5
Merging with most recent develop
JaroslavTulach Feb 25, 2025
f10da99
scalafmt
JaroslavTulach Feb 25, 2025
9bb5235
Detect redefined bindings
JaroslavTulach Feb 26, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,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
Expand Down
20 changes: 14 additions & 6 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -2598,7 +2598,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 =>
Expand Down Expand Up @@ -2629,11 +2640,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
)
}

Expand Down Expand Up @@ -3441,6 +3448,7 @@ lazy val `runtime-compiler-dump-igv` =
(project in file("engine/runtime-compiler-dump-igv"))
.enablePlugins(JPMSPlugin)
.settings(
customFrgaalJavaCompilerSettings("21", true),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scalaModuleDependencySetting,
javaModuleName := "org.enso.runtime.compiler.dump.igv",
Compile / internalModuleDependencies := {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Graph.Scope> {
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<GraphOccurrence>) 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;
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
}
Expand Down Expand Up @@ -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 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);
Expand Down Expand Up @@ -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 -> {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,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
}
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.enso.compiler.core.ir.expression.{
Case,
Comment,
Error,
IfThenElse,
Operator,
Section
}
Expand Down Expand Up @@ -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()
Expand All @@ -410,7 +412,7 @@ case object AliasAnalysis extends IRPass {
)
)
case binding @ Expression.Binding(name, expression, _, _) =>
if (builder.findDef(name.name) == -1) {
if (true /* XXX: builder.findDef(name.name) == -1 */ ) {
val isSuspended = expression match {
case Expression.Block(_, _, _, isSuspended, _) => isSuspended
case _ => false
Expand Down Expand Up @@ -439,7 +441,8 @@ case object AliasAnalysis extends IRPass {
)
)
} else {
errors.Redefined.Binding(binding)
// errors.Redefined.Binding(binding)
binding
}
case app: Application =>
analyseApplication(app, builder)
Expand Down Expand Up @@ -756,14 +759,36 @@ 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
* @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,
builder: GraphBuilder
): Case = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import org.enso.compiler.core.ir.expression.{
Comment,
Error,
Foreign,
IfThenElse,
Operator
}
import org.enso.compiler.core.ir.{
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import org.enso.compiler.core.ir.expression.{
Comment,
Error,
Foreign,
IfThenElse,
Operator
}
import org.enso.compiler.core.CompilerError
Expand Down Expand Up @@ -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, _, _, _) =>
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading