Two BindingsMap$ResolvedImport aren't equal and raise exception on the CI
#11171
Assignees
Labels
Comments
The test has been disabled in the release branch only by as the fix was integrated only into https://github.com/enso-org/enso/tree/release/2024.4 branch, the failure still appears in Closing as duplicate. But thanks for the reminder we need a fix for #10985 before next release.
|
|
I continue to see this failure in CI. Latest one where it happened:
|
JaroslavTulach
changed the title
BindingsMap$ResolvedImport aren't equal and raise exception on the CI
|
And another https://github.com/enso-org/enso/actions/runs/11563450462/job/32186772474?pr=11375#step:7:1636
|
|
Jaroslav Tulach reports a new STANDUP for yesterday (2024-11-06): Progress: .
|
|
I tried to re-enable proper check of diff --git engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/FramePointer.java engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/FramePointer.java
index 425af3bf8f..697b769b24 100644
--- engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/FramePointer.java
+++ engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/FramePointer.java
@@ -1,5 +1,6 @@
package org.enso.compiler.pass.analyse;
+import java.util.IdentityHashMap;
import org.enso.compiler.core.CompilerStub;
import org.enso.compiler.core.ir.ProcessingPass;
import org.enso.persist.Persistable;
@@ -10,10 +11,20 @@ import scala.Option;
*/
@Persistable(clazz = FramePointer.class, id = 1283)
public record FramePointer(int parentLevel, int frameSlotIdx) implements FrameAnalysisMeta {
+ public static final IdentityHashMap<FramePointer, Exception> WHEN = new IdentityHashMap<>();
public FramePointer {
assert parentLevel >= 0;
assert frameSlotIdx >= 0;
+ if (parentLevel == 0 && frameSlotIdx == 3) {
+ // System.err.println("now Frame for 0, 3");
+ }
+ // WHEN.put(this, new Exception("Allocated now: " + parentLevel + " " + frameSlotIdx));
+ }
+
+ public Exception when() {
+ var ex = WHEN.get(this);
+ return ex == null ? new Exception("no previous record") : ex;
}
@Override
diff --git engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/FrameVariableNames.java engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/FrameVariableNames.java
index 9d87a57a17..92edc4c9e0 100644
--- engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/FrameVariableNames.java
+++ engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/FrameVariableNames.java
@@ -1,6 +1,7 @@
package org.enso.compiler.pass.analyse;
import java.util.List;
+import java.util.Objects;
import org.enso.compiler.core.CompilerStub;
import org.enso.compiler.core.ir.ProcessingPass;
import org.enso.persist.Persistable;
@@ -11,7 +12,7 @@ import scala.jdk.javaapi.CollectionConverters;
public final class FrameVariableNames implements FrameAnalysisMeta {
private final List<String> names;
- public FrameVariableNames(List<String> variableNames) {
+ FrameVariableNames(List<String> variableNames) {
this.names = variableNames;
}
@@ -42,4 +43,31 @@ public final class FrameVariableNames implements FrameAnalysisMeta {
public Option<ProcessingPass.Metadata> duplicate() {
return Option.apply(new FrameVariableNames(names));
}
+
+ @Override
+ public int hashCode() {
+ int hash = 3;
+ hash = 59 * hash + Objects.hashCode(this.names);
+ return hash;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null) {
+ return false;
+ }
+ if (getClass() != obj.getClass()) {
+ return false;
+ }
+ final FrameVariableNames other = (FrameVariableNames) obj;
+ return Objects.equals(this.names, other.names);
+ }
+
+ @Override
+ public String toString() {
+ return "FrameVariableNames{" + "names=" + names + '}';
+ }
}
diff --git engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala
index 71a1e00b1b..07263105f1 100644
--- engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala
+++ engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala
@@ -38,7 +38,10 @@ case object FramePointerAnalysis extends IRPass {
override val invalidatedPasses: Seq[IRPass] = Seq(this)
override def runModule(ir: Module, moduleContext: ModuleContext): Module = {
+ FramePointer.WHEN.clear()
+ System.err.println("running a module")
ir.bindings.foreach(processBinding)
+ System.err.println("finished a module")
ir
}
@@ -319,16 +322,21 @@ case object FramePointerAnalysis extends IRPass {
newMeta: FrameAnalysisMeta
): Unit = {
ir.passData().get(this) match {
- case None =>
- ir.passData()
- .update(this, newMeta)
case Some(meta) =>
- val ex = new IllegalStateException(
- "Unexpected FrameAnalysisMeta associated with IR " + ir + "\nOld: " + meta + " new " + newMeta
- )
- ex.setStackTrace(ex.getStackTrace().slice(0, 10))
- throw ex
+ if (meta != newMeta) {
+ val ex = new IllegalStateException(
+ "Unexpected FrameAnalysisMeta associated with IR " + ir + "\nOld: " + meta + " new " + newMeta
+ )
+// ex.setStackTrace(ex.getStackTrace().slice(0, 10))
+ if (meta.isInstanceOf[FramePointer]) {
+ ex.printStackTrace()
+ meta.asInstanceOf[FramePointer].when.printStackTrace()
+ }
+ // throw ex
+ }
+ case None =>
}
+ ir.passData().update(this, newMeta)
}
/** Returns the index of the given `defOcc` definition in the given `scope`
diff --git engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/core/ir/MetadataStorageTest.scala engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/core/ir/MetadataStorageTest.scala
index 1f2fdf6558..07a0c187d2 100644
--- engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/core/ir/MetadataStorageTest.scala
+++ engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/core/ir/MetadataStorageTest.scala
@@ -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 = {
@@ -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 {
diff --git engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java
index e0ddd703ae..89f08ccec1 100644
--- engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java
+++ engine/runtime-parser/src/main/java/org/enso/compiler/core/ir/MetadataStorage.java
@@ -3,10 +3,10 @@ package org.enso.compiler.core.ir;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.SortedMap;
import java.util.TreeMap;
import java.util.function.BiFunction;
import java.util.stream.Collectors;
@@ -15,14 +15,17 @@ import scala.Option;
/** Stores metadata for the various passes. */
public final class MetadataStorage {
- private Map<ProcessingPass, ProcessingPass.Metadata> metadata;
+ private SortedMap<ProcessingPass, ProcessingPass.Metadata> metadata;
public MetadataStorage() {
- this(Collections.emptyMap());
+ this(copyMetaMap(null));
}
public MetadataStorage(Map<ProcessingPass, ProcessingPass.Metadata> init) {
- this.metadata = init;
+ this.metadata =
+ init instanceof SortedMap<ProcessingPass, ProcessingPass.Metadata> sorted
+ ? sorted
+ : copyMetaMap(init);
}
/**
@@ -34,7 +37,7 @@ public final class MetadataStorage {
* @tparam K the concrete type of `pass`
*/
public void update(ProcessingPass pass, ProcessingPass.Metadata newMeta) {
- var copy = copyMetaMap();
+ var copy = copyMetaMap(metadata);
copy.put(pass, newMeta);
metadata = copy;
}
@@ -63,7 +66,7 @@ public final class MetadataStorage {
if (prev == null) {
return Option.empty();
} else {
- var copy = copyMetaMap();
+ var copy = copyMetaMap(metadata);
copy.remove(pass);
metadata = copy;
return Option.apply(prev);
@@ -88,7 +91,7 @@ public final class MetadataStorage {
* @return a deep copy of `this`
*/
public MetadataStorage duplicate() {
- var map = new HashMap<ProcessingPass, ProcessingPass.Metadata>();
+ var map = new TreeMap<ProcessingPass, ProcessingPass.Metadata>(COMPARATOR);
for (var entry : this.metadata.entrySet()) {
var key = entry.getKey();
var meta = (ProcessingPass.Metadata) entry.getValue();
@@ -151,24 +154,17 @@ public final class MetadataStorage {
* @return `true` if restoration was successful, `false` otherwise
*/
public boolean restoreFromSerialization(CompilerStub compiler) {
- var ok = new boolean[] {true};
- var newMap =
- metadata.entrySet().stream()
- .collect(
- Collectors.toMap(
- Map.Entry::getKey,
- (en) -> {
- var value = en.getValue();
- var newOption = value.restoreFromSerialization(compiler);
- if (newOption.nonEmpty()) {
- return newOption.get();
- } else {
- ok[0] = false;
- return value;
- }
- }));
+ var newMap = new TreeMap<ProcessingPass, ProcessingPass.Metadata>(COMPARATOR);
+ for (var en : metadata.entrySet()) {
+ var newOption = en.getValue().restoreFromSerialization(compiler);
+ if (newOption.nonEmpty()) {
+ newMap.put(en.getKey(), newOption.get());
+ } else {
+ return false;
+ }
+ }
this.metadata = newMap;
- return ok[0];
+ return true;
}
@Override
@@ -195,9 +191,12 @@ public final class MetadataStorage {
return p1.getClass().getName().compareTo(p2.getClass().getName());
};
- private Map<ProcessingPass, ProcessingPass.Metadata> copyMetaMap() {
+ private static SortedMap<ProcessingPass, ProcessingPass.Metadata> copyMetaMap(
+ Map<ProcessingPass, ProcessingPass.Metadata> data) {
var copy = new TreeMap<ProcessingPass, ProcessingPass.Metadata>(COMPARATOR);
- copy.putAll(metadata);
+ if (data != null) {
+ copy.putAll(data);
+ }
return copy;
}
@@ -214,7 +213,7 @@ public final class MetadataStorage {
return true;
}
if (obj instanceof MetadataStorage other) {
- return this.metadata == other.metadata;
+ return this.metadata.equals(other.metadata);
}
return false;
}but that fails with |
|
Jaroslav Tulach reports a new STANDUP for yesterday (2024-11-07): Progress: .
|
2 tasks
There are problems with the
CCing @radeusgd |
2 tasks
|
Finally found the problem! |
|
Jaroslav Tulach reports a new STANDUP for today (2024-11-08): Progress: .
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I somehow got an impression that this shouldn't happen anymore?
https://github.com/enso-org/enso/actions/runs/11030572774/job/30635424983#step:7:1461
The text was updated successfully, but these errors were encountered: