Fix encoding for mapping with local members (#1152)

Fixes an issue where local members are included in
the mapping entry count.

Also: avoid re-computing Mapping.length

pkl-core/src/main/java/org/pkl/core/runtime/VmMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
+ * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@ package org.pkl.core.runtime;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.frame.MaterializedFrame;
+import java.util.HashSet;
 import java.util.Map;
 import javax.annotation.concurrent.GuardedBy;
 import org.graalvm.collections.UnmodifiableEconomicMap;
@@ -25,10 +26,10 @@ import org.pkl.core.ast.member.ObjectMember;
 import org.pkl.core.util.CollectionUtils;
 import org.pkl.core.util.EconomicMaps;
 import org.pkl.core.util.LateInit;
+import org.pkl.core.util.MutableLong;
 
 public final class VmMapping extends VmListingOrMapping {
-
+  private long cachedLength = -1;
-  private int cachedEntryCount = -1;
 
   @GuardedBy("this")
   private @LateInit VmSet __allKeys;
@@ -124,7 +125,7 @@ public final class VmMapping extends VmListingOrMapping {
     // could use shallow force, but deep force is cached
     force(false);
     other.force(false);
-    if (getEntryCount() != other.getEntryCount()) return false;
+    if (getLength() != other.getLength()) return false;
 
     var cursor = cachedValues.getEntries();
     while (cursor.advance()) {
@@ -162,16 +163,21 @@ public final class VmMapping extends VmListingOrMapping {
     return result;
   }
 
-  // assumes mapping has been forced
+  @TruffleBoundary
-  public int getEntryCount() {
+  public long getLength() {
-    if (cachedEntryCount != -1) return cachedEntryCount;
+    if (cachedLength != -1) return cachedLength;
-
+    var count = new MutableLong(0);
-    var result = 0;
+    var visited = new HashSet<>();
-    for (var key : cachedValues.getKeys()) {
+    iterateMembers(
-      if (key instanceof Identifier) continue;
+        (key, member) -> {
-      result += 1;
+          var alreadyVisited = !visited.add(key);
-    }
+          // important to record hidden member as visited before skipping it
-    cachedEntryCount = result;
+          // because any overriding member won't carry a `hidden` identifier
-    return result;
+          if (alreadyVisited || member.isLocalOrExternalOrHidden()) return true;
+          count.getAndIncrement();
+          return true;
+        });
+    cachedLength = count.get();
+    return cachedLength;
   }
 }

pkl-core/src/main/java/org/pkl/core/stdlib/base/MappingNodes.java

@@ -18,7 +18,6 @@ package org.pkl.core.stdlib.base;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.nodes.IndirectCallNode;
-import java.util.HashSet;
 import org.pkl.core.ast.lambda.ApplyVmFunction1Node;
 import org.pkl.core.ast.lambda.ApplyVmFunction2Node;
 import org.pkl.core.ast.lambda.ApplyVmFunction2NodeGen;
@@ -31,7 +30,6 @@ import org.pkl.core.stdlib.ExternalMethod2Node;
 import org.pkl.core.stdlib.ExternalPropertyNode;
 import org.pkl.core.util.EconomicMaps;
 import org.pkl.core.util.MutableBoolean;
-import org.pkl.core.util.MutableLong;
 import org.pkl.core.util.MutableReference;
 
 public final class MappingNodes {
@@ -53,20 +51,8 @@ public final class MappingNodes {
 
   public abstract static class length extends ExternalPropertyNode {
     @Specialization
-    @TruffleBoundary
     protected long eval(VmMapping self) {
-      var count = new MutableLong(0);
+      return self.getLength();
-      var visited = new HashSet<>();
-      self.iterateMembers(
-          (key, member) -> {
-            var alreadyVisited = !visited.add(key);
-            // important to record hidden member as visited before skipping it
-            // because any overriding member won't carry a `hidden` identifier
-            if (alreadyVisited || member.isLocalOrExternalOrHidden()) return true;
-            count.getAndIncrement();
-            return true;
-          });
-      return count.get();
     }
   }
 

pkl-server/src/main/kotlin/org/pkl/server/BinaryEvaluator.kt

@@ -221,7 +221,7 @@ internal class BinaryEvaluator(
     override fun visitMapping(value: VmMapping) {
       packer.packArrayHeader(2)
       packer.packInt(CODE_MAPPING.toInt())
-      packer.packMapHeader(value.entryCount)
+      packer.packMapHeader(value.length.toInt())
       value.iterateAlreadyForcedMemberValues { key, _, memberValue ->
         visit(key)
         visit(memberValue)

pkl-server/src/test/files/SnippetTests/input/map.pkl

@@ -13,3 +13,9 @@ res4: Mapping = new {
     ["bar"] = 2
   }
 }
+// https://github.com/apple/pkl/issues/1151
+res5: Mapping = new {
+  local self = this
+  ["foo"] = new Dynamic { name = "foo" }
+  ["bar"] = new Dynamic { name = self["foo"].name + "bar" }
+}

pkl-server/src/test/files/SnippetTests/output/map.yaml

@@ -41,4 +41,28 @@
         : 
           - 3
           - 
-            bar: 2
+            bar: 2
+  - 
+    - 16
+    - res5
+    - 
+      - 3
+      - 
+        foo: 
+          - 1
+          - Dynamic
+          - pkl:base
+          - 
+            - 
+              - 16
+              - name
+              - foo
+        bar: 
+          - 1
+          - Dynamic
+          - pkl:base
+          - 
+            - 
+              - 16
+              - name
+              - foobar