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

MetadataStorage is mutable - structural equals in copy isn't enough #11513

Merged
merged 5 commits into from
Nov 8, 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
Expand Up @@ -159,7 +159,7 @@ class MetadataStorageTest extends CompilerTest {
meta1.update(TestPass1, meta)
meta2.update(TestPass1, meta)

meta1 shouldNot equal(meta2)
meta1 shouldEqual meta2
}

def newMetadataStorage(init: Seq[MetadataPair[_]]): MetadataStorage = {
Expand Down Expand Up @@ -201,8 +201,8 @@ class MetadataStorageTest extends CompilerTest {
)
)

meta.duplicate shouldNot equal(meta)
meta.duplicate shouldNot equal(expected)
meta.duplicate shouldEqual meta
meta.duplicate shouldEqual expected
}

"enforce safe construction" in {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.enso.compiler.core;

import java.util.Comparator;
import java.util.UUID;
import java.util.function.Consumer;
import java.util.function.Function;
Expand Down Expand Up @@ -29,24 +28,6 @@
* <p>See also: Note [IR Equality and hashing]
*/
public interface IR {
/**
* Compares IR structure, but not metadata neither diagnostics. A special comparator used by
* <em>Persistance API</em> to perform some rare consistency checks.
*/
public static final Comparator<IR> STRUCTURE_COMPARATOR =
(aIr, bIr) -> {
if (aIr == bIr) {
return 0;
}
var aCopy = aIr.duplicate(true, false, false, true);
var bCopy = bIr.duplicate(true, false, false, true);

if (aCopy.equals(bCopy)) {
return 0;
}
return System.identityHashCode(aIr) - System.identityHashCode(bIr);
};

/**
* Storage for metadata that the node has been tagged with as the result of various compiler
* passes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import java.io.IOException;
import java.util.AbstractList;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.UUID;
import org.enso.compiler.core.ir.expression.Application;
Expand Down Expand Up @@ -445,33 +444,6 @@ protected Seq readObject(Input in) throws IOException, ClassNotFoundException {
}
}

@ServiceProvider(service = Persistance.class)
public static final class PersistMetadataStorage extends Persistance<MetadataStorage> {
public PersistMetadataStorage() {
super(MetadataStorage.class, false, 389);
}

@Override
@SuppressWarnings("unchecked")
protected void writeObject(MetadataStorage obj, Output out) throws IOException {
var map = new LinkedHashMap<ProcessingPass, ProcessingPass.Metadata>();
obj.map(
(processingPass, data) -> {
map.put(processingPass, data);
return null;
});
out.writeInline(java.util.Map.class, map);
}

@Override
@SuppressWarnings("unchecked")
protected MetadataStorage readObject(Input in) throws IOException, ClassNotFoundException {
var map = in.readInline(java.util.Map.class);
var storage = new MetadataStorage(map);
return storage;
}
}

@ServiceProvider(service = Persistance.class)
public static final class PersistDiagnosticStorage extends Persistance<DiagnosticStorage> {
public PersistDiagnosticStorage() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,25 @@
import java.util.function.BiFunction;
import java.util.stream.Collectors;
import org.enso.compiler.core.CompilerStub;
import org.enso.persist.Persistable;
import scala.Option;

/** Stores metadata for the various passes. */
@Persistable(id = 398)
public final class MetadataStorage {
private Map<ProcessingPass, ProcessingPass.Metadata> metadata;

/** Constructs empty, ready to be populated metadata. */
public MetadataStorage() {
this(Collections.emptyMap());
}

public MetadataStorage(Map<ProcessingPass, ProcessingPass.Metadata> init) {
this.metadata = init;
MetadataStorage(Map<ProcessingPass, ProcessingPass.Metadata> metaValues) {
this.metadata = metaValues;
}

final Map<ProcessingPass, ProcessingPass.Metadata> metaValues() {
return this.metadata;
}

/**
Expand Down Expand Up @@ -82,6 +89,15 @@ public Option<ProcessingPass.Metadata> get(ProcessingPass pass) {
return Option.apply(prev);
}

/**
* Creates a shallow copy of `this`. Use when re-assigning metadata from one IR to another.
*
* @return a shallow copy of `this`
*/
public MetadataStorage copy() {
return new MetadataStorage(metadata);
}

/**
* Creates a deep copy of `this`.
*
Expand Down Expand Up @@ -214,7 +230,7 @@ public boolean equals(Object obj) {
return true;
}
if (obj instanceof MetadataStorage other) {
return this.metadata == other.metadata;
return this.metadata.equals(other.metadata);
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ object CallArgument {
name != this.name
|| value != this.value
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ object DefinitionArgument {
|| defaultValue != this.defaultValue
|| suspended != this.suspended
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ sealed case class Empty(
): Empty = {
if (
location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ object Expression {
|| returnValue != this.returnValue
|| suspended != this.suspended
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -231,7 +231,7 @@ object Expression {
name != this.name
|| expression != this.expression
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ object Function {
|| body != this.body
|| location != this.location
|| canBeTCO != this.canBeTCO
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -283,7 +283,7 @@ object Function {
|| isPrivate != this.isPrivate
|| location != this.location
|| canBeTCO != this.canBeTCO
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ object Literal {
base != this.base
|| value != this.value
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -197,7 +197,7 @@ object Literal {
if (
text != this.text
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ final case class Module(
|| bindings != this.bindings
|| isPrivate != this.isPrivate
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ object Name {
typePointer != this.typePointer
|| methodName != this.methodName
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -246,7 +246,7 @@ object Name {
if (
parts != this.parts
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -318,7 +318,7 @@ object Name {
): Blank = {
if (
location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -400,7 +400,7 @@ object Name {
if (
specialName != this.specialName
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -506,7 +506,7 @@ object Name {
|| isMethod != this.isMethod
|| location != this.location
|| originalName != this.originalName
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -616,7 +616,7 @@ object Name {
if (
name != this.name
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -711,7 +711,7 @@ object Name {
name != this.name
|| expression != this.expression
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -822,7 +822,7 @@ object Name {
if (
synthetic != this.synthetic
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -921,7 +921,7 @@ object Name {
): SelfType = {
if (
location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ object Pattern {
if (
name != this.name
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -169,7 +169,7 @@ object Pattern {
constructor != this.constructor
|| fields != this.fields
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -324,7 +324,7 @@ object Pattern {
if (
literal != this.literal
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -429,7 +429,7 @@ object Pattern {
name != this.name
|| tpe != this.tpe
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -549,7 +549,7 @@ object Pattern {
if (
doc != this.doc
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ object Type {
args != this.args
|| result != this.result
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -171,7 +171,7 @@ object Type {
|| signature != this.signature
|| comment != this.comment
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -287,7 +287,7 @@ object Type {
typed != this.typed
|| context != this.context
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down Expand Up @@ -399,7 +399,7 @@ object Type {
typed != this.typed
|| error != this.error
|| location != this.location
|| passData != this.passData
|| (passData ne this.passData)
|| diagnostics != this.diagnostics
|| id != this.id
) {
Expand Down
Loading
Loading