From 3a04f78400831d657cc1234ce1c85db9bd3211a8 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 31 Oct 2024 18:05:05 +0100 Subject: [PATCH 1/4] Don't expose setters for Scope variables --- .../pass/analyse/PassPersistance.java | 8 +++--- .../pass/analyse/alias/graph/Graph.scala | 25 +++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java index c4634ba8bba4..cabb872879f0 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 @@ -1,6 +1,7 @@ package org.enso.compiler.pass.analyse; import java.io.IOException; + import org.enso.common.CachePreferences; import org.enso.compiler.pass.analyse.alias.AliasMetadata; import org.enso.compiler.pass.analyse.alias.graph.Graph; @@ -26,6 +27,7 @@ import org.enso.persist.Persistable; import org.enso.persist.Persistance; import org.openide.util.lookup.ServiceProvider; + import scala.Option; import scala.Tuple2$; @@ -107,11 +109,10 @@ protected Graph.Scope readObject(Input in) throws IOException { 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 optionParent = Option.apply(parent); childScopes.forall( (object) -> { var ch = (Graph.Scope) object; - ch.parent_$eq(optionParent); + ch.withParent(parent); return null; }); return parent; @@ -155,13 +156,12 @@ protected void writeObject(Graph obj, Output out) throws IOException { } private static void assignParents(Graph.Scope scope) { - var option = Option.apply(scope); scope .childScopes() .foreach( (ch) -> { assignParents(ch); - ch.parent_$eq(option); + ch.withParent(scope); return null; }); } 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 6292a9404701..a6e15a404cfb 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 @@ -318,12 +318,17 @@ object Graph { * Note that there may not be a link for all these definitions. */ sealed class Scope( - var childScopes: List[Scope] = List(), - var occurrences: Map[Id, GraphOccurrence] = HashMap(), - var allDefinitions: List[GraphOccurrence.Def] = List() + private[Graph] var _childScopes: List[Scope] = List(), + private[Graph] var _occurrences: Map[Id, GraphOccurrence] = HashMap(), + private[Graph] var _allDefinitions: List[GraphOccurrence.Def] = List() ) { - var parent: Option[Scope] = None + private[Graph] var _parent: Option[Scope] = None + + def childScopes = _childScopes + def occurrences = _occurrences + def allDefinitions = _allDefinitions + def parent = _parent /** Counts the number of scopes from this scope to the root. * @@ -343,7 +348,7 @@ object Graph { */ final def withParent(parentScope: Scope): this.type = { org.enso.common.Asserts.assertInJvm(parent.isEmpty) - this.parent = Some(parentScope) + this._parent = Some(parentScope) this } @@ -396,8 +401,8 @@ object Graph { */ final def addChild(): Scope = { val scope = new Scope() - scope.parent = Some(this) - childScopes ::= scope + scope._parent = Some(this) + _childScopes ::= scope scope } @@ -412,7 +417,7 @@ object Graph { s"Multiple occurrences found for ID ${occurrence.id}." ) } else { - occurrences += ((occurrence.id, occurrence)) + _occurrences += ((occurrence.id, occurrence)) } } @@ -422,7 +427,7 @@ object Graph { * @param definition The definition to add. */ final 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 @@ -641,7 +646,7 @@ object Graph { } private def removeScopeFromParent(scope: Scope): Unit = { - childScopes = childScopes.filter(_ != scope) + _childScopes = childScopes.filter(_ != scope) } /** Disassociates this Scope from its parent. From 2b8ad1e8afe3ab41136a09c0aaf33ea2822e7b54 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 31 Oct 2024 19:41:14 +0100 Subject: [PATCH 2/4] Rewritting GraphOccurrence in Java --- .../pass/analyse/PassPersistance.java | 3 - .../analyse/alias/graph/GraphOccurrence.java | 132 ++++++++++++++++++ .../compiler/pass/analyse/AliasAnalysis.scala | 47 +++---- .../pass/analyse/alias/graph/Graph.scala | 2 +- .../analyse/alias/graph/GraphOccurrence.scala | 54 ------- 5 files changed, 155 insertions(+), 83 deletions(-) create mode 100644 engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/alias/graph/GraphOccurrence.java delete mode 100644 engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/GraphOccurrence.scala 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 cabb872879f0..b5ded69b4013 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 @@ -1,7 +1,6 @@ package org.enso.compiler.pass.analyse; import java.io.IOException; - import org.enso.common.CachePreferences; import org.enso.compiler.pass.analyse.alias.AliasMetadata; import org.enso.compiler.pass.analyse.alias.graph.Graph; @@ -27,8 +26,6 @@ import org.enso.persist.Persistable; import org.enso.persist.Persistance; import org.openide.util.lookup.ServiceProvider; - -import scala.Option; import scala.Tuple2$; @Persistable(clazz = CachePreferenceAnalysis.WeightInfo.class, id = 1111) diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/alias/graph/GraphOccurrence.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/alias/graph/GraphOccurrence.java new file mode 100644 index 000000000000..3d475765966a --- /dev/null +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/alias/graph/GraphOccurrence.java @@ -0,0 +1,132 @@ +package org.enso.compiler.pass.analyse.alias.graph; + +import java.util.UUID; +import org.enso.compiler.core.ExternalID; +import org.enso.compiler.core.Identifier; + +/** + * An occurrence of a given symbol in the aliasing graph. Note that this is not present in the + * metadata attached to the [[org.enso.compiler.core.IR]] elements, but only in the alias [[Graph]]. + */ +public sealed interface GraphOccurrence permits GraphOccurrence.Def, GraphOccurrence.Use { + public abstract int id(); + + public abstract String symbol(); + + /** The definition of a symbol in the aliasing graph. */ + public static final class Def implements GraphOccurrence { + private final int id; + private final String symbol; + private final @Identifier UUID identifier; + private final @ExternalID UUID externalId; + private final boolean isLazy; + + /** + * The definition of a symbol in the aliasing graph. + * + * @param id the identifier of the name in the graph + * @param symbol the text of the name + * @param identifier the identifier of the symbol + * @param externalId the external identifier for the IR node defining the symbol + * @param isLazy whether or not the symbol is defined as lazy + */ + public Def( + int id, String symbol, UUID identifier, scala.Option externalId, boolean isLazy) { + this.id = id; + this.externalId = externalId.nonEmpty() ? externalId.get() : null; + this.identifier = identifier; + this.isLazy = isLazy; + this.symbol = symbol; + } + + public Def(int id, String symbol, UUID identifier, scala.Option externalId) { + this(id, symbol, identifier, externalId, false); + } + + @Override + public int id() { + return this.id; + } + + @Override + public String symbol() { + return this.symbol; + } + + public UUID identifier() { + return identifier; + } + + public scala.Option externalId() { + return scala.Option.apply(externalId); + } + + public boolean isLazy() { + return isLazy; + } + + public static scala.Option, Boolean>> + unapply(Object obj) { + if (obj instanceof Def d) { + var extId = scala.Option.apply(d.externalId); + var tuple = new scala.Tuple5<>(d.id, d.symbol, d.identifier, extId, d.isLazy); + return scala.Option.apply(tuple); + } + return scala.Option.empty(); + } + } + + /** A usage of a symbol in the aliasing graph */ + public static final class Use implements GraphOccurrence { + private final int id; + private final String symbol; + private final @Identifier UUID identifier; + private final @ExternalID UUID externalId; + + /** + * A usage of a symbol in the aliasing graph + * + *

Name usages _need not_ correspond to name definitions, as dynamic symbol resolution means + * that a name used at runtime _may not_ be statically visible in the scope. + * + * @param id the identifier of the name in the graph + * @param symbol the text of the name + * @param identifier the identifier of the symbol + * @param externalId the external identifier for the IR node defining the symbol + */ + public Use(int id, String symbol, UUID identifier, scala.Option externalId) { + this.id = id; + this.symbol = symbol; + this.externalId = externalId.nonEmpty() ? externalId.get() : null; + this.identifier = identifier; + } + + @Override + public int id() { + return this.id; + } + + @Override + public String symbol() { + return this.symbol; + } + + public UUID identifier() { + return identifier; + } + + public scala.Option externalId() { + return scala.Option.apply(externalId); + } + + public static scala.Option>> unapply( + Object obj) { + if (obj instanceof Use u) { + var extId = scala.Option.apply(u.externalId); + var tuple = new scala.Tuple4<>(u.id, u.symbol, u.identifier, extId); + return scala.Option.apply(tuple); + } + return scala.Option.empty(); + } + } +} 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 e851b84b1848..ff4bec1cdb2c 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 @@ -421,14 +421,13 @@ case object AliasAnalysis extends IRPass { case _ => false } val occurrenceId = graph.nextId() - val occurrence = - GraphOccurrence.Def( - occurrenceId, - name.name, - binding.getId(), - binding.getExternalId, - isSuspended - ) + val occurrence = new GraphOccurrence.Def( + occurrenceId, + name.name, + binding.getId(), + binding.getExternalId, + isSuspended + ) parentScope.add(occurrence) parentScope.addDefinition(occurrence) @@ -489,13 +488,12 @@ case object AliasAnalysis extends IRPass { } val labelId = graph.nextId() - val definition = - GraphOccurrence.Def( - labelId, - label.name, - label.getId, - label.getExternalId - ) + val definition = new GraphOccurrence.Def( + labelId, + label.name, + label.getId, + label.getExternalId + ) parentScope.add(definition) parentScope.addDefinition(definition) @@ -548,7 +546,7 @@ case object AliasAnalysis extends IRPass { // Synthetic `self` must not be added to the scope, but it has to be added as a // definition for frame index metadata val occurrenceId = graph.nextId() - val definition = GraphOccurrence.Def( + val definition = new GraphOccurrence.Def( occurrenceId, selfName.name, arg.getId(), @@ -587,7 +585,7 @@ case object AliasAnalysis extends IRPass { ) val occurrenceId = graph.nextId() - val definition = GraphOccurrence.Def( + val definition = new GraphOccurrence.Def( occurrenceId, name.name, arg.getId(), @@ -764,18 +762,17 @@ case object AliasAnalysis extends IRPass { val occurrenceId = graph.nextId() if (isInPatternContext && !isConstructorNameInPatternContext) { - val definition = - GraphOccurrence.Def( - occurrenceId, - name.name, - name.getId, - name.getExternalId - ) + val definition = new GraphOccurrence.Def( + occurrenceId, + name.name, + name.getId, + name.getExternalId + ) parentScope.add(definition) parentScope.addDefinition(definition) } else { val occurrence = - GraphOccurrence.Use( + new GraphOccurrence.Use( occurrenceId, name.name, name.getId, 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 a6e15a404cfb..1101955ca624 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 @@ -494,7 +494,7 @@ object Graph { case None => parent.flatMap(_.resolveUsage(occurrence, parentCounter + 1)) case Some(target) => - Some(Graph.Link(occurrence.id, parentCounter, target.id)) + Some(Graph.Link(occurrence.id, parentCounter, target.id())) } } diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/GraphOccurrence.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/GraphOccurrence.scala deleted file mode 100644 index d23feb1cf213..000000000000 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/GraphOccurrence.scala +++ /dev/null @@ -1,54 +0,0 @@ -package org.enso.compiler.pass.analyse.alias.graph - -import org.enso.compiler.core.{ExternalID, Identifier} -import org.enso.compiler.pass.analyse.alias.graph.Graph.Id - -import java.util.UUID - -/** An occurrence of a given symbol in the aliasing graph. - * Note that this is not present in the metadata attached to the [[org.enso.compiler.core.IR]] elements, - * but only in the alias [[Graph]]. - */ -sealed trait GraphOccurrence { - val id: Id - val symbol: Graph.Symbol -} - -object GraphOccurrence { - - /** The definition of a symbol in the aliasing graph. - * - * @param id the identifier of the name in the graph - * @param symbol the text of the name - * @param identifier the identifier of the symbol - * @param externalId the external identifier for the IR node defining - * the symbol - * @param isLazy whether or not the symbol is defined as lazy - */ - sealed case class Def( - override val id: Id, - override val symbol: Graph.Symbol, - identifier: UUID @Identifier, - externalId: Option[UUID @ExternalID], - isLazy: Boolean = false - ) extends GraphOccurrence - - /** A usage of a symbol in the aliasing graph - * - * Name usages _need not_ correspond to name definitions, as dynamic - * symbol resolution means that a name used at runtime _may not_ be - * statically visible in the scope. - * - * @param id the identifier of the name in the graph - * @param symbol the text of the name - * @param identifier the identifier of the symbol - * @param externalId the external identifier for the IR node defining - * the symbol - */ - sealed case class Use( - override val id: Id, - override val symbol: Graph.Symbol, - identifier: UUID @Identifier, - externalId: Option[UUID @ExternalID] - ) extends GraphOccurrence -} From 275a1811ff5fbe4a15b7234ea374c288e7130d10 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 31 Oct 2024 20:26:13 +0100 Subject: [PATCH 3/4] Adjusting AliasAnalysisTest to the Java rewrite --- .../pass/analyse/test/AliasAnalysisTest.scala | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala b/engine/runtime-integration-tests/src/test/scala/org/enso/compiler/pass/analyse/test/AliasAnalysisTest.scala index 526fa34b72fc..b11103ede113 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 @@ -95,19 +95,19 @@ class AliasAnalysisTest extends CompilerTest { val childOfChildOfChild = childOfChild.addChild() val aDefId = graph.nextId() - val aDef = GraphOccurrence.Def(aDefId, "a", genId, None) + val aDef = new GraphOccurrence.Def(aDefId, "a", genId, None) val bDefId = graph.nextId() - val bDef = GraphOccurrence.Def(bDefId, "b", genId, None) + val bDef = new GraphOccurrence.Def(bDefId, "b", genId, None) val aUseId = graph.nextId() - val aUse = GraphOccurrence.Use(aUseId, "a", genId, None) + val aUse = new GraphOccurrence.Use(aUseId, "a", genId, None) val bUseId = graph.nextId() - val bUse = GraphOccurrence.Use(bUseId, "b", genId, None) + val bUse = new GraphOccurrence.Use(bUseId, "b", genId, None) val cUseId = graph.nextId() - val cUse = GraphOccurrence.Use(cUseId, "c", genId, None) + val cUse = new GraphOccurrence.Use(cUseId, "c", genId, None) // Add occurrences to the scopes complexScope.add(aDef) @@ -233,19 +233,19 @@ class AliasAnalysisTest extends CompilerTest { val childScope = rootScope.addChild() val aDefId = graph.nextId() - val aDef = GraphOccurrence.Def(aDefId, "a", genId, None) + val aDef = new GraphOccurrence.Def(aDefId, "a", genId, None) val bDefId = graph.nextId() - val bDef = GraphOccurrence.Def(bDefId, "b", genId, None) + val bDef = new GraphOccurrence.Def(bDefId, "b", genId, None) val aUse1Id = graph.nextId() - val aUse1 = GraphOccurrence.Use(aUse1Id, "a", genId, None) + val aUse1 = new GraphOccurrence.Use(aUse1Id, "a", genId, None) val aUse2Id = graph.nextId() - val aUse2 = GraphOccurrence.Use(aUse2Id, "a", genId, None) + val aUse2 = new GraphOccurrence.Use(aUse2Id, "a", genId, None) val cUseId = graph.nextId() - val cUse = GraphOccurrence.Use(cUseId, "c", genId, None) + val cUse = new GraphOccurrence.Use(cUseId, "c", genId, None) rootScope.add(aDef) rootScope.add(aUse1) @@ -355,28 +355,31 @@ class AliasAnalysisTest extends CompilerTest { val grandChild = child1.addChild() val aDefInRootId = graph.nextId() - val aDefInRoot = GraphOccurrence.Def(aDefInRootId, "a", genId, None) + val aDefInRoot = new GraphOccurrence.Def(aDefInRootId, "a", genId, None) rootScope.add(aDefInRoot) val aDefInChild1Id = graph.nextId() - val aDefInChild1 = GraphOccurrence.Def(aDefInChild1Id, "a", genId, None) + val aDefInChild1 = + new GraphOccurrence.Def(aDefInChild1Id, "a", genId, None) child1.add(aDefInChild1) val aDefInChild2Id = graph.nextId() - val aDefInChild2 = GraphOccurrence.Def(aDefInChild2Id, "a", genId, None) + val aDefInChild2 = + new GraphOccurrence.Def(aDefInChild2Id, "a", genId, None) child2.add(aDefInChild2) val aDefInGrandChildId = graph.nextId() val aDefInGrandChild = - GraphOccurrence.Def(aDefInGrandChildId, "a", genId, None) + new GraphOccurrence.Def(aDefInGrandChildId, "a", genId, None) grandChild.add(aDefInGrandChild) val bDefInRootId = graph.nextId() - val bDefInRoot = GraphOccurrence.Def(bDefInRootId, "b", genId, None) + val bDefInRoot = new GraphOccurrence.Def(bDefInRootId, "b", genId, None) rootScope.add(bDefInRoot) val bDefInChild2Id = graph.nextId() - val bDefInChild2 = GraphOccurrence.Def(bDefInChild2Id, "b", genId, None) + val bDefInChild2 = + new GraphOccurrence.Def(bDefInChild2Id, "b", genId, None) child2.add(bDefInChild2) graph.knownShadowedDefinitions(aDefInGrandChild) shouldEqual Set( From ca01e112d65b258414e11d0f7935759b19cd6427 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 1 Nov 2024 17:15:42 +0100 Subject: [PATCH 4/4] Only change withParent if the parent is different --- .../compiler/pass/analyse/alias/graph/Graph.scala | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala index 1101955ca624..fe7bf907f52b 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 @@ -323,12 +323,12 @@ object Graph { private[Graph] var _allDefinitions: List[GraphOccurrence.Def] = List() ) { - private[Graph] var _parent: Option[Scope] = None + private[Graph] var _parent: Scope = null def childScopes = _childScopes def occurrences = _occurrences def allDefinitions = _allDefinitions - def parent = _parent + def parent = if (this._parent eq null) None else Some(_parent) /** Counts the number of scopes from this scope to the root. * @@ -347,8 +347,10 @@ object Graph { * @return this scope with parent scope set */ final def withParent(parentScope: Scope): this.type = { - org.enso.common.Asserts.assertInJvm(parent.isEmpty) - this._parent = Some(parentScope) + if (this._parent ne parentScope) { + org.enso.common.Asserts.assertInJvm(parent.isEmpty) + this._parent = parentScope + } this } @@ -401,7 +403,7 @@ object Graph { */ final def addChild(): Scope = { val scope = new Scope() - scope._parent = Some(this) + scope._parent = this _childScopes ::= scope scope