Correct translation from reference to definition key

apple · Sep 26, 2024 · e661ba0 · e661ba0
1 parent a2d5b37
commit e661ba0
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 9 deletions.
diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmObject.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmObject.java
@@ -96,6 +96,18 @@ public final UnmodifiableEconomicMap<Object, ObjectMember> getMembers() {
   /** Tells whether this object has any elements. */
   public abstract boolean hasElements();
 
+  /**
+   * Members in this {@code VmObject} are subscripted with their {@code referenceKey}. Given that
+   * the definition of this object may contain use of {@code delete}, the keys used in said
+   * definition are different from the {@code referenceKey}. In case of element deletions, the
+   * definition key may be higher {@code long} than the {@code referenceKey}. In case of entry or
+   * property deletions, the key is no longer valid for this object.
+   * 
+   * @param referenceKey The key used from outside of this object to dereference a member.
+   * @return {@code null} if the key is deleted on this object, an offset {@code Long} in case of an
+   *         element index with earlier elements being deleted in this object, or the original
+   *         input.
+   */
   public @Nullable Object toDefinitionKey(Object referenceKey) {
     if (!(referenceKey instanceof Long index) || !hasElements()) {
       var member = getMember(referenceKey);
@@ -109,11 +121,15 @@ public final UnmodifiableEconomicMap<Object, ObjectMember> getMembers() {
     if (deletedIndices == null) {
       return referenceKey;
     }
-
-    index += deletedIndices.subSet(0L, index + 1L).size();
-    for (var m = getMember(index); m != null && m.isDelete(); m = getMember(index)) {
-      index++;
-    }
+
+    var lowerBound = 0L;
+    var add = 0L;
+    do {
+      add = deletedIndices.subSet(lowerBound, index + 1L).size();
+      lowerBound = index + 1L;
+      index += add;
+    } while (add > 0L);
+
     return index;
   }
 

diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmUtils.java
@@ -258,6 +258,7 @@ public static Object readMember(
         // The key was not renamed by deletion of other keys.
         return doReadMember(receiver, owner, key, member, checkType, callNode);
       }
+      memberKey = key;
     }
 
     return null;

diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/basic/delete.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/basic/delete.pkl
@@ -13,7 +13,7 @@ facts {
       ["deleteQux"] = delete
     }.toMap() == Map("deleteFoo", 1, "deleteBaz", 42)
   }
-  
+
   ["literal deletion of elements"] {
     (new Dynamic {
       "foo"
@@ -23,7 +23,7 @@ facts {
       [1] = delete
     }.toList() == List("foo", "baz")
   }
-  
+
   ["member predicate deletion"] {
     local source = (new Dynamic {
       "one"
@@ -40,7 +40,7 @@ facts {
     source.toList() == List("two", "four")
     source.toMap() == Map("foo", "six")
   }
-  
+
   ["deleted properties are no longer listed in errors"] {
     local source = (new Dynamic {
       foo = 1
@@ -51,7 +51,7 @@ facts {
     }
     !module.catch(() -> source.qux).contains("foo")
   }
-  
+
   ["element indices are correctly renamed"] {
     local source = (new Dynamic {
       "foo"
@@ -94,3 +94,4 @@ facts {
     source[2] == "eight"
   }
 }
+