Share Name.Literal strings (#11886)

Fixes #10330 by interning `Name.Literal` strings.
enso-org · Dec 18, 2024 · e4a4bcc · e4a4bcc
1 parent 021fada
commit e4a4bcc
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 12 deletions.
diff --git a/...untime-integration-tests/src/test/java/org/enso/compiler/test/passes/MethodCallsTest.java b/...untime-integration-tests/src/test/java/org/enso/compiler/test/passes/MethodCallsTest.java
@@ -3,14 +3,19 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
 
+import org.enso.compiler.core.ir.MetadataStorage;
 import org.enso.compiler.core.ir.Module;
 import org.enso.compiler.core.ir.Name;
 import org.enso.compiler.core.ir.expression.Application;
 import org.enso.compiler.data.BindingsMap;
 import org.enso.compiler.pass.resolve.MethodCalls$;
 import org.enso.test.utils.ContextUtils;
 import org.junit.Test;
+import scala.Option;
 
 public class MethodCallsTest {
   @Test
@@ -44,4 +49,20 @@ private Application.Prefix findMethodCall(Module ir, String methodName) {
     assertThat(res.isDefined(), is(true));
     return (Application.Prefix) res.get();
   }
+
+  @Test
+  public void interningNameLiteralStrings() {
+    var interned = "same_name";
+    var n1 = new String(interned);
+    var n2 = new String(interned);
+    assertNotSame(n1, n2);
+
+    var meta = new MetadataStorage();
+    var l1 = new Name.Literal(n1, false, null, Option.empty(), meta);
+    var l2 = new Name.Literal(n2, false, null, Option.empty(), meta);
+
+    assertEquals("Literals are structurally equal", l1, l2);
+    assertSame("Literals share the same name string", l1.name(), l2.name());
+    assertSame("It is the interned string", l1.name(), interned);
+  }
 }
diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Name.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/Name.scala
@@ -477,16 +477,34 @@ object Name {
     * @param originalName the name which this literal has replaced, if any
     * @param passData the pass metadata associated with this node
     */
-  sealed case class Literal(
+  sealed case class Literal private (
     override val name: String,
     override val isMethod: Boolean,
     override val identifiedLocation: IdentifiedLocation,
-    originalName: Option[Name]             = None,
-    override val passData: MetadataStorage = new MetadataStorage()
+    private val origName: Name,
+    override val passData: MetadataStorage
   ) extends Name
       with LazyDiagnosticStorage
       with LazyId {
 
+    def this(
+      name: String,
+      isMethod: Boolean,
+      identifiedLocation: IdentifiedLocation,
+      originalName: Option[Name] = None,
+      passData: MetadataStorage  = new MetadataStorage()
+    ) = {
+      this(
+        name.intern(),
+        isMethod,
+        identifiedLocation,
+        originalName.orNull,
+        passData
+      )
+    }
+
+    def originalName: Option[Name] = Option(this.origName)
+
     /** Creates a copy of `this`.
       *
       * @param name        the literal text of the name
@@ -517,7 +535,7 @@ object Name {
         || id != this.id
       ) {
         val res =
-          Literal(name, isMethod, location.orNull, originalName, passData)
+          new Literal(name, isMethod, location.orNull, originalName, passData)
         res.diagnostics = diagnostics
         res.id          = id
         res
@@ -568,6 +586,16 @@ object Name {
     override def showCode(indent: Int): String = name
   }
 
+  object Literal {
+    def apply(
+      name: String,
+      isMethod: Boolean,
+      identifiedLocation: IdentifiedLocation,
+      originalName: Option[Name] = None,
+      passData: MetadataStorage  = new MetadataStorage()
+    ) = new Literal(name, isMethod, identifiedLocation, originalName, passData)
+  }
+
   /** Base trait for annotations. */
   sealed trait Annotation extends Name with module.scope.Definition {
 

diff --git a/lib/java/persistance-dsl/src/main/java/org/enso/persist/impl/PersistableProcessor.java b/lib/java/persistance-dsl/src/main/java/org/enso/persist/impl/PersistableProcessor.java
@@ -139,12 +139,14 @@ && isVisibleFrom(e, orig))
       if (constructors.size() > 1) {
         var snd = (ExecutableElement) constructors.get(1);
         if (richerConstructor.compare(cons, snd) == 0) {
-          processingEnv
-              .getMessager()
-              .printMessage(
-                  Kind.ERROR,
-                  "There should be exactly one 'richest' constructor in " + typeElem,
-                  orig);
+          var sb = new StringBuilder();
+          sb.append("There should be exactly one 'richest' constructor in ")
+              .append(typeElem)
+              .append(". Found:");
+          for (var c : constructors) {
+            sb.append("\n  ").append(c);
+          }
+          processingEnv.getMessager().printMessage(Kind.ERROR, sb.toString(), orig);
           return false;
         }
       }
@@ -288,8 +290,12 @@ private int countInlineRef(List<? extends VariableElement> parameters) {
     var cnt = 0;
     for (var p : parameters) {
       var type = tu.asElement(tu.erasure(p.asType()));
-      if (type != null && type.getSimpleName().toString().equals("Reference")) {
-        cnt++;
+      if (type != null) {
+        switch (type.getSimpleName().toString()) {
+          case "Reference" -> cnt++;
+          case "Option" -> cnt++;
+          default -> {}
+        }
       }
     }
     return cnt;