Removing useless GraphOccurrence.Global & some encapsulation (#11419)

Work on #11365 made me realize that `GraphOccurrence.Global` isn't really used anywhere. Simplifying the code by removing it and associated methods.
This commit is contained in:
Jaroslav Tulach 2024-10-29 11:23:22 +01:00 committed by GitHub
parent 78d9e34840
commit 71acb83e7f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 86 additions and 166 deletions

View File

@ -75,8 +75,8 @@ public class InlineCompilerBenchmark {
@Benchmark
public void longExpression(Blackhole blackhole) throws IOException {
try (InlineContextResource resource = inlineSource.inlineContextFactory().create()) {
var tuppleOpt = compiler.runInline(longExpression, resource.inlineContext());
try (var inlineCtx = inlineSource.inlineContextFactory().create()) {
var tuppleOpt = compiler.runInline(longExpression, inlineCtx);
if (tuppleOpt.isEmpty()) {
throw new AssertionError("Unexpected: inline compilation should succeed");
}

View File

@ -80,8 +80,8 @@ public class InlineCompilerErrorBenchmark {
@Benchmark
public void expressionWithErrors(Blackhole blackhole) throws IOException {
try (InlineContextResource resource = mainInlineContextResourceFactory.create()) {
var tuppleOpt = compiler.runInline(expressionWithErrors, resource.inlineContext());
try (var inlineCtx = mainInlineContextResourceFactory.create()) {
var tuppleOpt = compiler.runInline(expressionWithErrors, inlineCtx);
blackhole.consume(tuppleOpt);
}
}

View File

@ -1,23 +0,0 @@
package org.enso.compiler.benchmarks.inline;
import java.io.IOException;
import org.enso.compiler.context.InlineContext;
/**
* InlineContextResource ensures that the underlying InlineContext is properly cleaned up after
* usage.
*
* @param inlineContext InlineContext for the main method
*/
public record InlineContextResource(InlineContext inlineContext) implements AutoCloseable {
@Override
public void close() throws IOException {
inlineContext
.localScope()
.foreach(
s -> {
s.scope().removeScopeFromParent();
return null;
});
}
}

View File

@ -13,16 +13,15 @@ public record InlineContextResourceFactory(
EnsoContext ensoCtx,
PackageRepository pkgRepository) {
public InlineContextResource create() {
public InlineContext create() {
var mainFunc = moduleScope.getMethodForType(assocTypeReceiver, "main");
var mainFuncRootNode = (MethodRootNode) mainFunc.getCallTarget().getRootNode();
var mainLocalScope = mainFuncRootNode.getLocalScope();
return new InlineContextResource(
InlineContext.fromJava(
mainLocalScope.createChild(),
moduleScope.getModule().asCompilerModule(),
scala.Option.apply(false),
ensoCtx.getCompilerConfig(),
scala.Option.apply(pkgRepository)));
return InlineContext.fromJava(
mainLocalScope.createChild(),
moduleScope.getModule().asCompilerModule(),
scala.Option.apply(false),
ensoCtx.getCompilerConfig(),
scala.Option.apply(pkgRepository));
}
}

View File

@ -4,7 +4,7 @@ import java.util.Set;
record InlineSource(
String source,
// InlineContextResource for the main method
// InlineContext for the main method
InlineContextResourceFactory inlineContextFactory,
// Local variables in main method
Set<String> localVarNames) {}

View File

@ -143,7 +143,6 @@ public final class PassPersistance {
var nextIdCounter = in.readInt();
var g = new Graph(rootScope, nextIdCounter, links);
g.freeze();
return g;
}

View File

@ -25,9 +25,15 @@ case class InlineContext(
freshNameSupply: Option[FreshNameSupply] = None,
passConfiguration: Option[PassConfiguration] = None,
pkgRepo: Option[PackageRepository] = None
) {
) extends AutoCloseable {
def bindingsAnalysis(): BindingsMap = module.bindingsAnalysis()
def getModule() = module.module
def close(): Unit = {
this.localScope
.foreach(_.scope.removeScopeFromParent())
}
}
object InlineContext {

View File

@ -784,8 +784,6 @@ case object AliasAnalysis extends IRPass {
parentScope.add(occurrence)
if (!isConstructorNameInPatternContext && !name.isMethod) {
graph.resolveLocalUsage(occurrence)
} else {
graph.resolveGlobalUsage(occurrence)
}
}
name.updateMetadata(

View File

@ -1,4 +1,6 @@
package org.enso.compiler.pass.analyse.alias.graph
package org.enso.compiler
package pass.analyse
package alias.graph
import org.enso.compiler.core.CompilerError
import org.enso.compiler.debug.Debug
@ -7,68 +9,53 @@ import org.enso.compiler.pass.analyse.alias.graph.Graph.Scope
import scala.collection.immutable.HashMap
import scala.collection.mutable
import scala.reflect.ClassTag
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(),
private var _nextIdCounter: Int = 0,
private var links: Set[Graph.Link] = Set()
) extends Serializable {
) {
private var sourceLinks: Map[Graph.Id, Set[Graph.Link]] = new HashMap()
private var targetLinks: Map[Graph.Id, Set[Graph.Link]] = new HashMap()
private var frozen: Boolean = false
{
links.foreach(addSourceTargetLink)
}
private var globalSymbols: Map[Graph.Symbol, GraphOccurrence.Global] =
Map()
/** @return the next counter value
*/
def nextIdCounter: Int = _nextIdCounter
/** @return a deep structural copy of `this` */
def deepCopy(
final def deepCopy(
scope_mapping: mutable.Map[Scope, Scope] = mutable.Map()
): Graph = {
val copy = new Graph(
this.rootScope.deepCopy(scope_mapping),
this.nextIdCounter
this._nextIdCounter
)
copy.links = this.links
copy.sourceLinks = this.sourceLinks
copy.targetLinks = this.targetLinks
copy.globalSymbols = this.globalSymbols
copy.links = this.links
copy.sourceLinks = this.sourceLinks
copy.targetLinks = this.targetLinks
copy
}
def getLinks(): Set[Graph.Link] = links
private[analyse] def getLinks(): Set[Graph.Link] = links
def freeze(): Unit = {
frozen = true
}
/** Registers a requested global symbol in the aliasing scope.
*
* @param sym the symbol occurrence
*/
def addGlobalSymbol(sym: GraphOccurrence.Global): Unit = {
org.enso.common.Asserts.assertInJvm(!frozen)
if (!globalSymbols.contains(sym.symbol)) {
globalSymbols = globalSymbols + (sym.symbol -> sym)
}
final def freeze(): Unit = {
_nextIdCounter = -1
}
/** Creates a deep copy of the aliasing graph structure.
*
* @return a copy of the graph structure
*/
def copy: Graph = {
private[analyse] def copy: Graph = {
val graph = new Graph(
rootScope.deepCopy(mutable.Map()),
nextIdCounter
_nextIdCounter
)
graph.links = links
graph.sourceLinks = sourceLinks
@ -93,8 +80,11 @@ sealed class Graph(
*
* @return a unique identifier for this graph
*/
def nextId(): Graph.Id = {
final def nextId(): Graph.Id = {
val nextId = _nextIdCounter
if (nextId < 0) {
throw new IllegalStateException("Cannot emit new IDs. Frozen!")
}
_nextIdCounter += 1
nextId
}
@ -105,7 +95,7 @@ sealed class Graph(
* @param occurrence the symbol usage
* @return the link, if it exists
*/
def resolveLocalUsage(
final def resolveLocalUsage(
occurrence: GraphOccurrence.Use
): Option[Graph.Link] = {
scopeFor(occurrence.id).flatMap(_.resolveUsage(occurrence).map { link =>
@ -126,24 +116,6 @@ sealed class Graph(
)
}
/** Resolves any links for the given usage of a symbol, assuming the symbol
* is global (i.e. method, constructor etc.)
*
* @param occurrence the symbol usage
* @return the link, if it exists
*/
def resolveGlobalUsage(
occurrence: GraphOccurrence.Use
): Option[Graph.Link] = {
scopeFor(occurrence.id) match {
case Some(scope) =>
globalSymbols
.get(occurrence.symbol)
.map(g => Graph.Link(occurrence.id, scope.scopesToRoot + 1, g.id))
case None => None
}
}
/** Returns a string representation of the graph.
*
* @return a string representation of `this`
@ -155,7 +127,7 @@ sealed class Graph(
*
* @return a pretty-printed string representation of the graph
*/
def pprint: String = {
@unused private def pretty: String = {
val original = toString
Debug.pretty(original)
}
@ -165,7 +137,7 @@ sealed class Graph(
* @param id the identifier for the symbol
* @return a list of links in which `id` occurs
*/
def linksFor(id: Graph.Id): Set[Graph.Link] = {
final def linksFor(id: Graph.Id): Set[Graph.Link] = {
sourceLinks.getOrElse(id, Set.empty[Graph.Link]) ++ targetLinks.getOrElse(
id,
Set()
@ -179,7 +151,7 @@ sealed class Graph(
* @tparam T the role in which `symbol` should occur
* @return a set of all links in which `symbol` occurs with role `T`
*/
def linksFor[T <: GraphOccurrence: ClassTag](
private[analyse] def linksFor[T <: GraphOccurrence: ClassTag](
symbol: Graph.Symbol
): Set[Graph.Link] = {
val idsForSym = rootScope.symbolToIds[T](symbol)
@ -195,7 +167,7 @@ sealed class Graph(
* @param id the occurrence identifier
* @return the occurrence for `id`, if it exists
*/
def getOccurrence(id: Graph.Id): Option[GraphOccurrence] =
final def getOccurrence(id: Graph.Id): Option[GraphOccurrence] =
scopeFor(id).flatMap(_.getOccurrence(id))
/** Gets the link from an id to the definition of the symbol it represents.
@ -203,7 +175,7 @@ sealed class Graph(
* @param id the identifier to find the definition link for
* @return the definition link for `id` if it exists
*/
def defLinkFor(id: Graph.Id): Option[Graph.Link] = {
final def defLinkFor(id: Graph.Id): Option[Graph.Link] = {
linksFor(id).find { edge =>
val occ = getOccurrence(edge.target)
occ match {
@ -218,7 +190,7 @@ sealed class Graph(
* @param id the id to find the scope for
* @return the scope where `id` occurs
*/
def scopeFor(id: Graph.Id): Option[Graph.Scope] = {
final def scopeFor(id: Graph.Id): Option[Graph.Scope] = {
rootScope.scopeFor(id)
}
@ -228,7 +200,7 @@ sealed class Graph(
* @tparam T the role in which `symbol` occurs
* @return all the scopes where `symbol` occurs with role `T`
*/
def scopesFor[T <: GraphOccurrence: ClassTag](
private[analyse] def scopesFor[T <: GraphOccurrence: ClassTag](
symbol: Graph.Symbol
): List[Graph.Scope] = {
rootScope.scopesForSymbol[T](symbol)
@ -239,7 +211,7 @@ sealed class Graph(
* @return the number of scopes that are either this scope or children of
* it
*/
def numScopes: Int = {
private[analyse] def numScopes: Int = {
rootScope.scopeCount
}
@ -247,7 +219,7 @@ sealed class Graph(
*
* @return the maximum nesting depth of scopes through this scope.
*/
def nesting: Int = {
private[analyse] def nesting: Int = {
rootScope.maxNesting
}
@ -256,7 +228,7 @@ sealed class Graph(
* @param id the occurrence identifier
* @return `true` if `id` shadows other bindings, otherwise `false`
*/
def canShadow(id: Graph.Id): Boolean = {
private[analyse] def canShadow(id: Graph.Id): Boolean = {
scopeFor(id)
.flatMap(
_.getOccurrence(id).flatMap {
@ -277,15 +249,14 @@ sealed class Graph(
* @param definition the definition to find the 'shadowees' of
* @return the bindings shadowed by `definition`
*/
def knownShadowedDefinitions(
final def knownShadowedDefinitions(
definition: GraphOccurrence
): Set[GraphOccurrence] = {
def getShadowedIds(
scope: Graph.Scope
): Set[GraphOccurrence] = {
scope.occurrences.values.collect {
case d: GraphOccurrence.Def if d.symbol == definition.symbol => d
case g: GraphOccurrence.Global if g.symbol == definition.symbol => g
case d: GraphOccurrence.Def if d.symbol == definition.symbol => d
} ++ scope.parent.map(getShadowedIds).getOrElse(Set())
}.toSet
@ -295,27 +266,15 @@ sealed class Graph(
case Some(scope) => getShadowedIds(scope) // + globals
case None => Set()
}
case _: GraphOccurrence.Global => Set()
case _: GraphOccurrence.Use => Set()
case _: GraphOccurrence.Use => Set()
}
}
/** Determines if the provided id is linked to a binding that shadows
* another binding.
*
* @param id the identifier to check
* @return `true` if the definition of the symbol for `id` shadows another
* binding for the same symbol, `false`, otherwise
*/
def linkedToShadowingBinding(id: Graph.Id): Boolean = {
defLinkFor(id).isDefined
}
/** Gets all symbols defined in the graph.
*
* @return the set of symbols defined in this graph
*/
def symbols: Set[Graph.Symbol] = {
private[analyse] def symbols: Set[Graph.Symbol] = {
rootScope.symbols
}
@ -326,7 +285,7 @@ sealed class Graph(
* @tparam T the role in which `symbol` should occur
* @return a list of identifiers for that symbol
*/
def symbolToIds[T <: GraphOccurrence: ClassTag](
private[analyse] def symbolToIds[T <: GraphOccurrence: ClassTag](
symbol: Graph.Symbol
): List[Graph.Id] = {
rootScope.symbolToIds[T](symbol)
@ -337,7 +296,7 @@ sealed class Graph(
* @param id the identifier of an occurrence
* @return the symbol associated with `id`, if it exists
*/
def idToSymbol(
private[analyse] def idToSymbol(
id: Graph.Id
): Option[Graph.Symbol] = {
rootScope.idToSymbol(id)
@ -362,7 +321,7 @@ object Graph {
var childScopes: List[Scope] = List(),
var occurrences: Map[Id, GraphOccurrence] = HashMap(),
var allDefinitions: List[GraphOccurrence.Def] = List()
) extends Serializable {
) {
var parent: Option[Scope] = None
@ -372,7 +331,7 @@ object Graph {
*
* @return the number of scopes from this scope to the root
*/
def scopesToRoot: Int = {
private[analyse] def scopesToRoot: Int = {
parent.flatMap(scope => Some(scope.scopesToRoot + 1)).getOrElse(0)
}
@ -382,7 +341,7 @@ object Graph {
*
* @return this scope with parent scope set
*/
def withParent(parentScope: Scope): this.type = {
final def withParent(parentScope: Scope): this.type = {
org.enso.common.Asserts.assertInJvm(parent.isEmpty)
this.parent = Some(parentScope)
this
@ -393,7 +352,7 @@ object Graph {
*
* @return a copy of `this`
*/
def deepCopy(
final def deepCopy(
mapping: mutable.Map[Scope, Scope] = mutable.Map()
): Scope = {
mapping.get(this) match {
@ -435,7 +394,7 @@ object Graph {
*
* @return a scope that is a child of `this`
*/
def addChild(): Scope = {
final def addChild(): Scope = {
val scope = new Scope()
scope.parent = Some(this)
childScopes ::= scope
@ -447,7 +406,7 @@ object Graph {
*
* @param occurrence the occurrence to add
*/
def add(occurrence: GraphOccurrence): Unit = {
final def add(occurrence: GraphOccurrence): Unit = {
if (occurrences.contains(occurrence.id)) {
throw new CompilerError(
s"Multiple occurrences found for ID ${occurrence.id}."
@ -462,7 +421,7 @@ object Graph {
*
* @param definition The definition to add.
*/
def addDefinition(definition: GraphOccurrence.Def): Unit = {
final def addDefinition(definition: GraphOccurrence.Def): Unit = {
allDefinitions = allDefinitions ++ List(definition)
}
@ -472,7 +431,9 @@ object Graph {
* @param id the occurrence identifier
* @return the occurrence for `id`, if it exists
*/
def getOccurrence(id: Graph.Id): Option[GraphOccurrence] = {
private[analyse] def getOccurrence(
id: Graph.Id
): Option[GraphOccurrence] = {
occurrences.get(id)
}
@ -483,7 +444,7 @@ object Graph {
* @tparam T the role for the symbol
* @return the occurrences for `name`, if they exist
*/
def getOccurrences[T <: GraphOccurrence: ClassTag](
private[analyse] def getOccurrences[T <: GraphOccurrence: ClassTag](
symbol: Graph.Symbol
): Set[GraphOccurrence] = {
occurrences.values.collect {
@ -491,18 +452,6 @@ object Graph {
}.toSet
}
/** Unsafely gets the occurrence for the provided ID in the current scope.
*
* Please note that this will crash if the ID is not defined in this
* scope.
*
* @param id the occurrence identifier
* @return the occurrence for `id`
*/
def unsafeGetOccurrence(id: Graph.Id): GraphOccurrence = {
getOccurrence(id).get
}
/** Checks whether a symbol occurs in a given role in the current scope.
*
* @param symbol the symbol to check for
@ -510,7 +459,7 @@ object Graph {
* @return `true` if `symbol` occurs in role `T` in this scope, `false`
* otherwise
*/
def hasSymbolOccurrenceAs[T <: GraphOccurrence: ClassTag](
final def hasSymbolOccurrenceAs[T <: GraphOccurrence: ClassTag](
symbol: Graph.Symbol
): Boolean = {
occurrences.values.collectFirst {
@ -526,7 +475,7 @@ object Graph {
* @return the link from `occurrence` to the definition of that symbol, if it
* exists
*/
def resolveUsage(
private[analyse] def resolveUsage(
occurrence: GraphOccurrence.Use,
parentCounter: Int = 0
): Option[Graph.Link] = {
@ -556,7 +505,7 @@ object Graph {
* @return the number of scopes that are either this scope or children of
* it
*/
def scopeCount: Int = {
private[analyse] def scopeCount: Int = {
childScopes.map(_.scopeCount).sum + 1
}
@ -564,7 +513,7 @@ object Graph {
*
* @return the maximum nesting depth of scopes through this scope.
*/
def maxNesting: Int = {
private[analyse] def maxNesting: Int = {
childScopes.map(_.maxNesting).foldLeft(0)(Math.max) + 1
}
@ -573,7 +522,7 @@ object Graph {
* @param id the id to find the scope for
* @return the scope where `id` occurs
*/
def scopeFor(id: Graph.Id): Option[Scope] = {
private[analyse] def scopeFor(id: Graph.Id): Option[Scope] = {
if (!occurrences.contains(id)) {
if (childScopes.isEmpty) {
None
@ -611,7 +560,7 @@ object Graph {
* @param n the number of scopes to walk up
* @return the n-th parent of `this` scope, if present
*/
def nThParent(n: Int): Option[Scope] = {
private[analyse] def nThParent(n: Int): Option[Scope] = {
if (n == 0) Some(this) else this.parent.flatMap(_.nThParent(n - 1))
}
@ -624,7 +573,7 @@ object Graph {
* @tparam T the role in which `name` occurs
* @return all the scopes where `name` occurs with role `T`
*/
def scopesForSymbol[T <: GraphOccurrence: ClassTag](
private[analyse] def scopesForSymbol[T <: GraphOccurrence: ClassTag](
symbol: Graph.Symbol
): List[Scope] = {
val occursInThisScope = hasSymbolOccurrenceAs[T](symbol)
@ -643,7 +592,7 @@ object Graph {
*
* @return the set of symbols
*/
def symbols: Set[Graph.Symbol] = {
private[analyse] def symbols: Set[Graph.Symbol] = {
val symbolsInThis = occurrences.values.map(_.symbol).toSet
val symbolsInChildScopes = childScopes.flatMap(_.symbols)
@ -657,7 +606,7 @@ object Graph {
* @tparam T the role in which `symbol` should occur
* @return a list of identifiers for that symbol
*/
def symbolToIds[T <: GraphOccurrence: ClassTag](
private[analyse] def symbolToIds[T <: GraphOccurrence: ClassTag](
symbol: Graph.Symbol
): List[Graph.Id] = {
val scopes =
@ -670,7 +619,7 @@ object Graph {
* @param id the identifier of an occurrence
* @return the symbol associated with `id`, if it exists
*/
def idToSymbol(
private[analyse] def idToSymbol(
id: Graph.Id
): Option[Graph.Symbol] = {
scopeFor(id).flatMap(_.getOccurrence(id)).map(_.symbol)
@ -681,7 +630,7 @@ object Graph {
* @param scope the potential parent scope
* @return `true` if `this` is a child of `scope`, otherwise `false`
*/
def isChildOf(scope: Scope): Boolean = {
private[analyse] def isChildOf(scope: Scope): Boolean = {
val isDirectChildOf = scope.childScopes.contains(this)
val isChildOfChildren = scope.childScopes
@ -697,7 +646,7 @@ object Graph {
/** Disassociates this Scope from its parent.
*/
def removeScopeFromParent(): Unit = {
private[compiler] def removeScopeFromParent(): Unit = {
org.enso.common.Asserts.assertInJvm(this.parent.nonEmpty)
this.parent.foreach(_.removeScopeFromParent(this))
}
@ -712,6 +661,9 @@ object Graph {
* @param scopeCount the number of scopes that the link traverses
* @param target the target ID of the link in the graph
*/
sealed case class Link(source: Id, scopeCount: Int, target: Id)
extends Serializable
sealed private[analyse] case class Link(
source: Id,
scopeCount: Int,
target: Id
) {}
}

View File

@ -9,7 +9,7 @@ import java.util.UUID
* 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 extends Serializable {
sealed trait GraphOccurrence {
val id: Id
val symbol: Graph.Symbol
}
@ -51,15 +51,4 @@ object GraphOccurrence {
identifier: UUID @Identifier,
externalId: Option[UUID @ExternalID]
) extends GraphOccurrence
// TODO [AA] At some point the analysis should make use of these.
/** Represents a global symbol that has been _asked for_ in the program.
*
* @param id the identifier of the name in the graph
* @param symbol the text of the name
*/
sealed case class Global(
override val id: Id,
override val symbol: Graph.Symbol
) extends GraphOccurrence
}

View File

@ -1,4 +1,4 @@
package org.enso.compiler.test.pass.analyse
package org.enso.compiler.pass.analyse.test
import org.enso.compiler.Passes
import org.enso.compiler.context.{FreshNameSupply, InlineContext, ModuleContext}

View File

@ -1,4 +1,4 @@
package org.enso.compiler.test.pass.analyse
package org.enso.compiler.pass.analyse.test
import org.enso.compiler.Passes
import org.enso.compiler.pass.analyse.FramePointer