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

Use GraphBuilder to construct an alias Graph #11491

Merged
merged 3 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package org.enso.compiler.pass.analyse.alias.graph;

/**
* Builder of {@link Graph}. Separates the concerns of building a graph of local symbol definitions
* and their usages from the actual querying of those symbols.
*/
public final class GraphBuilder {
private final Graph graph;
private final Graph.Scope scope;

private GraphBuilder(Graph graph, Graph.Scope scope) {
this.graph = graph;
this.scope = scope;
}

/**
* Creates new empty builder.
*
* @return empty builder
*/
public static GraphBuilder create() {
var topLevel = Graph$.MODULE$.create();
return create(topLevel, topLevel.rootScope());
}

/**
* Creates a builder for given graph and scope.
*
* @param g the graph
* @param s its scope
* @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);
}

/**
* Creates a child scope and returns a builder for it.
*
* @return new builder for newly created scope, but the same graph
*/
public GraphBuilder addChild() {
return new GraphBuilder(graph, scope.addChild());
}

/**
* Adds occurrence to current scope.
*
* @return this builder with modified scope
*/
public GraphBuilder add(GraphOccurrence occ) {
this.scope.add(occ);
return this;
}

/**
* Adds definition to current scope.
*
* @return this builder with modified scope
*/
public GraphBuilder addDefinition(GraphOccurrence.Def def) {
this.scope.addDefinition(def);
return this;
}

/**
* Finds definition ID of provided symbol.
*
* @param name the name of the symbol
* @return -1 if not such symbol found, otherwise ID of the symbol
*/
public int findDef(String name) {
var first = this.scope.occurrences().values().find(occ -> occ.symbol().equals(name));
if (first.nonEmpty() && first.get() instanceof GraphOccurrence.Def def) {
return def.id();
} else {
return -1;
}
}

/** Creates new definition for */
public GraphOccurrence.Def newDef(
String symbol, java.util.UUID identifier, scala.Option<java.util.UUID> externalId) {
return newDef(symbol, identifier, externalId, false);
}

public GraphOccurrence.Def newDef(
String symbol,
java.util.UUID identifier,
scala.Option<java.util.UUID> externalId,
boolean suspended) {
return new GraphOccurrence.Def(graph.nextId(), symbol, identifier, externalId, suspended);
}

/** Factory method to create new [GraphOccurrence.Use]. */
public GraphOccurrence.Use newUse(
String symbol, java.util.UUID identifier, scala.Option<java.util.UUID> externalId) {
return new GraphOccurrence.Use(graph.nextId(), symbol, identifier, externalId);
}

public void resolveLocalUsage(GraphOccurrence.Use use) {
graph.resolveLocalUsage(use);
}

public Graph toGraph() {
return graph;
Copy link
Member

Choose a reason for hiding this comment

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

Should we ensure that toGraph was called just once? Looking at AliasAnalysis, it seems to me that toGraph creates snapshots of a graph, i.e., the builder is reusable?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that, but it is probably not going to work. For example processing arguments is trying to modify a builder that has already been queried for a graph/scope:

org.enso.compiler.pass.analyse.alias.graph.GraphBuilder.assertNotFreezed(GraphBuilder.java:124)
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.alias.graph.GraphBuilder.newDef(GraphBuilder.java:97)
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.AliasAnalysis$.$anonfun$analyseArgumentDefs$1(AliasAnalysis.scala:570)
        at [email protected]/scala.collection.immutable.List.map(List.scala:250)
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.AliasAnalysis$.analyseArgumentDefs(AliasAnalysis.scala:521)
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.AliasAnalysis$.$anonfun$analyseModuleDefinition$1(AliasAnalysis.scala:292)
        at [email protected]/scala.collection.immutable.List.map(List.scala:246)
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.AliasAnalysis$.analyseModuleDefinition(AliasAnalysis.scala:287)
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.AliasAnalysis$.$anonfun$runModule$1(AliasAnalysis.scala:100)
        at [email protected]/scala.collection.immutable.List.map(List.scala:246)
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.AliasAnalysis$.runModule(AliasAnalysis.scala:100)
        at org.enso.runtime/org.enso.compiler.pass.analyse.test.AliasAnalysisTest$AnalyseModule.analyse(AliasAnalysisTest.scala:58)
        at org.enso.runtime/org.enso.compiler.pass.analyse.test.AliasAnalysisTest.$anonfun$new$37(AliasAnalysisTest.scala:414)
        at org.scalatest.SuperEngine.registerNestedBranch(Engine.scala:609)
        at org.scalatest.wordspec.AnyWordSpecLike.org$scalatest$wordspec$AnyWordSpecLike$$registerBranch(AnyWordSpecLike.scala:218)
        at org.scalatest.wordspec.AnyWordSpecLike$$anon$1.apply(AnyWordSpecLike.scala:1158)
        at org.scalatest.verbs.ShouldVerb$StringShouldWrapperForVerb.should(ShouldVerb.scala:194)
        at org.scalatest.verbs.ShouldVerb$StringShouldWrapperForVerb.should$(ShouldVerb.scala:193)
        at org.scalatest.matchers.should.Matchers$StringShouldWrapper.should(Matchers.scala:8242)
        at org.enso.runtime/org.enso.compiler.pass.analyse.test.AliasAnalysisTest.<init>(AliasAnalysisTest.scala:407)
        at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)
        at java.base/java.lang.reflect.ReflectAccess.newInstance(ReflectAccess.java:128)
        at java.base/jdk.internal.reflect.ReflectionFactory.newInstance(ReflectionFactory.java:304)
        at java.base/java.lang.Class.newInstance(Class.java:725)
        at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:454)
        at sbt.ForkMain$Run.lambda$runTest$1(ForkMain.java:414)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
        at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.AssertionError: Freezed here!
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.alias.graph.GraphBuilder.toGraph(GraphBuilder.java:113)
        at org.enso.runtime.compiler/org.enso.compiler.pass.analyse.AliasAnalysis$.$anonfun$analyseArgumentDefs$1(AliasAnalysis.scala:583)
        at [email protected]/scala.collection.immutable.List.map(List.scala:246)

Maybe it doesn't matter. The goal of the GraphBuilder was to identify the code that can modify the Graph.Scope - which has been achieved. All such code needs the GraphBuilder.

}

public Graph.Scope toScope() {
return scope;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import org.enso.compiler.pass.analyse.FrameVariableNames
import org.enso.compiler.pass.analyse.DataflowAnalysis
import org.enso.compiler.pass.analyse.alias.graph.{
GraphOccurrence,
GraphBuilder,
Graph => AliasGraph
}

Expand Down Expand Up @@ -92,7 +93,9 @@ class LocalScope(
*
* @return a child of this scope
*/
def createChild(): LocalScope = createChild(() => scope.addChild())
def createChild(): LocalScope = createChild(() => {
GraphBuilder.create(null, scope).addChild().toScope()
})

/** Creates a child using a known aliasing scope.
*
Expand Down
Loading
Loading