Update Hash#merge and Hash#merge! to be 2.6 compatible

aardvark179 · chrisseaton · commit b544bea22f99 · 2019-03-10T01:27:39.000Z
diff --git a/spec/tags/core/hash/merge_tags.txt b/spec/tags/core/hash/merge_tags.txt
diff --git a/src/main/java/org/truffleruby/core/CoreLibrary.java b/src/main/java/org/truffleruby/core/CoreLibrary.java
@@ -1422,6 +1422,7 @@ public DynamicObject getWarningModule() {
             "/core/truffle/debug.rb",
             "/core/truffle/warnings.rb",
             "/core/truffle/exception_operations.rb",
+            "/core/truffle/hash_operations.rb",
             "/core/truffle/numeric_operations.rb",
             "/core/truffle/proc_operations.rb",
             "/core/truffle/range_operations.rb",
diff --git a/src/main/java/org/truffleruby/core/hash/HashNodes.java b/src/main/java/org/truffleruby/core/hash/HashNodes.java
@@ -34,7 +34,6 @@
 import org.truffleruby.core.hash.HashNodesFactory.HashLookupOrExecuteDefaultNodeGen;
 import org.truffleruby.core.hash.HashNodesFactory.InitializeCopyNodeFactory;
 import org.truffleruby.core.hash.HashNodesFactory.InternalRehashNodeGen;
-import org.truffleruby.language.NotOptimizedWarningNode;
 import org.truffleruby.language.NotProvided;
 import org.truffleruby.language.RubyBaseNode;
 import org.truffleruby.language.RubyGuards;
@@ -683,8 +682,8 @@ private Object yieldPair(DynamicObject block, Object key, Object value) {
     }
 
     @ImportStatic(HashGuards.class)
-    @CoreMethod(names = "merge", required = 1, needsBlock = true)
-    public abstract static class MergeNode extends YieldingCoreMethodNode {
+    @Primitive(name = "hash_merge", needsSelf = false)
+    public abstract static class MergeNode extends PrimitiveArrayArgumentsNode {
 
         @Child private LookupEntryNode lookupEntryNode;
         @Child private SetNode setNode = SetNode.create();
@@ -694,38 +693,38 @@ public abstract static class MergeNode extends YieldingCoreMethodNode {
         // Merge with an empty hash, without a block
 
         @Specialization(guards = {"isNullHash(hash)", "isRubyHash(other)", "isNullHash(other)"})
-        public DynamicObject mergeEmptyEmpty(DynamicObject hash, DynamicObject other, NotProvided block) {
+        public DynamicObject mergeEmptyEmpty(DynamicObject hash, DynamicObject other) {
             return newHash(hash, null, 0, Layouts.HASH.getCompareByIdentity(hash));
         }
 
         @Specialization(guards = {"isEmptyHash(hash)", "isRubyHash(other)", "isPackedHash(other)"})
-        public DynamicObject mergeEmptyPacked(DynamicObject hash, DynamicObject other, NotProvided block) {
+        public DynamicObject mergeEmptyPacked(DynamicObject hash, DynamicObject other) {
             final Object[] store = (Object[]) Layouts.HASH.getStore(other);
             final Object[] copy = PackedArrayStrategy.copyStore(getContext(), store);
             return newHash(hash, copy, Layouts.HASH.getSize(other), Layouts.HASH.getCompareByIdentity(hash));
         }
 
         @Specialization(guards = {"isPackedHash(hash)", "isRubyHash(other)", "isEmptyHash(other)"})
-        public DynamicObject mergePackedEmpty(DynamicObject hash, DynamicObject other, NotProvided block) {
-            return mergeEmptyPacked(other, hash, block);
+        public DynamicObject mergePackedEmpty(DynamicObject hash, DynamicObject other) {
+            return mergeEmptyPacked(other, hash);
         }
 
         @Specialization(guards = {"isEmptyHash(hash)", "isRubyHash(other)", "isBucketHash(other)"})
-        public DynamicObject mergeEmptyBuckets(DynamicObject hash, DynamicObject other, NotProvided block) {
+        public DynamicObject mergeEmptyBuckets(DynamicObject hash, DynamicObject other) {
             final DynamicObject merged = newHash(hash, null, 0, Layouts.HASH.getCompareByIdentity(hash));
             BucketsStrategy.copyInto(getContext(), other, merged);
             return merged;
         }
 
         @Specialization(guards = {"isBucketHash(hash)", "isRubyHash(other)", "isEmptyHash(other)"})
-        public DynamicObject mergeBucketsEmpty(DynamicObject hash, DynamicObject other, NotProvided block) {
-            return mergeEmptyBuckets(other, hash, block);
+        public DynamicObject mergeBucketsEmpty(DynamicObject hash, DynamicObject other) {
+            return mergeEmptyBuckets(other, hash);
         }
 
         // Merge non-empty packed with non-empty packed, without a block
 
         @Specialization(guards = {"isPackedHash(hash)", "!isEmptyHash(hash)", "isRubyHash(other)", "isPackedHash(other)", "!isEmptyHash(other)"})
-        public DynamicObject mergePackedPacked(DynamicObject hash, DynamicObject other, NotProvided block,
+        public DynamicObject mergePackedPacked(DynamicObject hash, DynamicObject other,
                         @Cached("createBinaryProfile()") ConditionProfile byIdentityProfile,
                         @Cached("createBinaryProfile()") ConditionProfile nothingFromFirstProfile,
                         @Cached("createBinaryProfile()") ConditionProfile resultIsSmallProfile) {
@@ -834,7 +833,7 @@ private int mergedPackedHashes(boolean compareByIdentity, Object[] storeA, int s
         // Merge non-empty buckets with non-empty buckets, without a block
 
         @Specialization(guards = {"isBucketHash(hash)", "!isEmptyHash(hash)", "isRubyHash(other)", "isBucketHash(other)", "!isEmptyHash(other)"})
-        public DynamicObject mergeBucketsBuckets(DynamicObject hash, DynamicObject other, NotProvided block) {
+        public DynamicObject mergeBucketsBuckets(DynamicObject hash, DynamicObject other) {
             final boolean compareByIdentity = Layouts.HASH.getCompareByIdentity(hash);
             final Entry[] newStore = new Entry[BucketsStrategy.capacityGreaterThan(Layouts.HASH.getSize(hash) + Layouts.HASH.getSize(other))];
             final DynamicObject merged = newHash(hash, newStore, 0, compareByIdentity);
@@ -854,7 +853,7 @@ public DynamicObject mergeBucketsBuckets(DynamicObject hash, DynamicObject other
         // Merge combinations of packed and buckets, without a block
 
         @Specialization(guards = {"isPackedHash(hash)", "!isEmptyHash(hash)", "isRubyHash(other)", "isBucketHash(other)", "!isEmptyHash(other)"})
-        public DynamicObject mergePackedBuckets(DynamicObject hash, DynamicObject other, NotProvided block) {
+        public DynamicObject mergePackedBuckets(DynamicObject hash, DynamicObject other) {
             final boolean compareByIdentity = Layouts.HASH.getCompareByIdentity(hash);
             final Entry[] newStore = new Entry[BucketsStrategy.capacityGreaterThan(Layouts.HASH.getSize(hash) + Layouts.HASH.getSize(other))];
             final DynamicObject merged = newHash(hash, newStore, 0, compareByIdentity);
@@ -877,7 +876,7 @@ public DynamicObject mergePackedBuckets(DynamicObject hash, DynamicObject other,
         }
 
         @Specialization(guards = {"isBucketHash(hash)", "!isEmptyHash(hash)", "isRubyHash(other)", "isPackedHash(other)", "!isEmptyHash(other)"})
-        public DynamicObject mergeBucketsPacked(DynamicObject hash, DynamicObject other, NotProvided block) {
+        public DynamicObject mergeBucketsPacked(DynamicObject hash, DynamicObject other) {
             final boolean compareByIdentity = Layouts.HASH.getCompareByIdentity(hash);
             final Entry[] newStore = new Entry[BucketsStrategy.capacityGreaterThan(Layouts.HASH.getSize(hash) + Layouts.HASH.getSize(other))];
             final DynamicObject merged = newHash(hash, newStore, 0, compareByIdentity);
@@ -899,66 +898,6 @@ public DynamicObject mergeBucketsPacked(DynamicObject hash, DynamicObject other,
             return merged;
         }
 
-        // Merge using a block
-
-        @Specialization(guards = "isRubyHash(other)")
-        public DynamicObject merge(DynamicObject hash, DynamicObject other, DynamicObject block,
-                                   @Cached("new()") NotOptimizedWarningNode notOptimizedWarningNode) {
-            notOptimizedWarningNode.warn("Hash#merge with a block is not yet optimized");
-
-            final boolean compareByIdentity = Layouts.HASH.getCompareByIdentity(hash);
-
-            final int capacity = BucketsStrategy.capacityGreaterThan(Layouts.HASH.getSize(hash) + Layouts.HASH.getSize(other));
-            final DynamicObject merged = newHash(hash, new Entry[capacity], 0, compareByIdentity);
-
-            int size = 0;
-
-            for (KeyValue keyValue : HashOperations.iterableKeyValues(hash)) {
-                setNode.executeSet(merged, keyValue.getKey(), keyValue.getValue(), false);
-                size++;
-            }
-
-            if (lookupEntryNode == null) {
-                CompilerDirectives.transferToInterpreterAndInvalidate();
-                lookupEntryNode = insert(new LookupEntryNode());
-            }
-
-            for (KeyValue keyValue : HashOperations.iterableKeyValues(other)) {
-                final HashLookupResult searchResult = lookupEntryNode.lookup(merged, keyValue.getKey());
-
-                if (searchResult.getEntry() == null) {
-                    setNode.executeSet(merged, keyValue.getKey(), keyValue.getValue(), false);
-                    size++;
-                } else {
-                    final Object oldValue = searchResult.getEntry().getValue();
-                    final Object newValue = keyValue.getValue();
-                    final Object mergedValue = yield(block, keyValue.getKey(), oldValue, newValue);
-
-                    setNode.executeSet(merged, keyValue.getKey(), mergedValue, false);
-                }
-            }
-
-            Layouts.HASH.setSize(merged, size);
-
-            assert HashOperations.verifyStore(getContext(), hash);
-            return merged;
-        }
-
-        // Merge with something that wasn't a hash
-
-        @Specialization(guards = "!isRubyHash(other)")
-        public Object merge(DynamicObject hash, Object other, Object maybeBlock,
-                @Cached("createPrivate()") CallDispatchHeadNode fallbackCallNode) {
-            final DynamicObject block;
-            if (maybeBlock == NotProvided.INSTANCE) {
-                block = null;
-            } else {
-                block = (DynamicObject) maybeBlock;
-            }
-
-            return fallbackCallNode.callWithBlock(hash, "merge_fallback", block, other);
-        }
-
         private DynamicObject newHash(DynamicObject hash, Object[] store, int size, boolean compareByIdentity) {
             return allocateObjectNode.allocate(Layouts.BASIC_OBJECT.getLogicalClass(hash),
                     Layouts.HASH.build(store, size, null, null, Layouts.HASH.getDefaultBlock(hash), Layouts.HASH.getDefaultValue(hash), compareByIdentity));
diff --git a/src/main/ruby/core/hash.rb b/src/main/ruby/core/hash.rb
@@ -242,6 +242,29 @@ def keep_if
     self
   end
 
+  def merge(*others)
+    unless block_given?
+      current = self
+      others.each do |other|
+        other = Truffle::Type.coerce_to(other, Hash, :to_hash)
+        current = Truffle::HashOperations.hash_merge(current, other)
+      end
+    else
+      current = self.dup
+      others.each do |other|
+        other.each do |k, v|
+          other = Truffle::Type.coerce_to(other, Hash, :to_hash)
+          if current.include?(k)
+            current[k] = yield(k, current[k], v)
+          else
+            current[k] = v
+          end
+        end
+      end
+    end
+    current
+  end
+      
   def merge!(*others)
     Truffle.check_frozen
 
@@ -467,10 +490,6 @@ def to_proc
     end
   end
 
-  def merge_fallback(other, &block)
-    merge(Truffle::Type.coerce_to other, Hash, :to_hash, &block)
-  end
-
   def transform_values
     return to_enum(:transform_values) { size } unless block_given?
 
diff --git a/src/main/ruby/core/truffle/hash_operations.rb b/src/main/ruby/core/truffle/hash_operations.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+# Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. This
+# code is released under a tri EPL/GPL/LGPL license. You can use it,
+# redistribute it and/or modify it under the terms of the:
+#
+# Eclipse Public License version 1.0, or
+# GNU General Public License version 2, or
+# GNU Lesser General Public License version 2.1.
+
+module Truffle
+  module HashOperations
+    def self.hash_merge(current, other)
+      Truffle.invoke_primitive(:hash_merge, current, other)
+    end
+  end
+end
+
