Use Kernel#dup and Kernel#clone for Range

* Add RubyIntOrLongRange superclass to deduplicate a bit.

Author: eregon

src/main/java/org/truffleruby/core/encoding/RubyEncoding.java

@@ -23,8 +23,8 @@
 import org.truffleruby.core.rope.RopeConstants;
 import org.truffleruby.core.string.FrozenStringLiterals;
 import org.truffleruby.core.string.ImmutableRubyString;
-import org.truffleruby.language.ImmutableRubyObject;
 
+import org.truffleruby.language.ImmutableRubyObjectNotCopyable;
 import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.language.objects.ObjectGraph;
 import org.truffleruby.language.objects.ObjectGraphNode;
@@ -33,7 +33,7 @@
 import java.util.Set;
 
 @ExportLibrary(InteropLibrary.class)
-public class RubyEncoding extends ImmutableRubyObject implements ObjectGraphNode, Comparable<RubyEncoding> {
+public class RubyEncoding extends ImmutableRubyObjectNotCopyable implements ObjectGraphNode, Comparable<RubyEncoding> {
 
     public final Encoding jcoding;
     public final ImmutableRubyString name;

src/main/java/org/truffleruby/core/kernel/KernelNodes.java

@@ -71,6 +71,8 @@
 import org.truffleruby.core.proc.ProcNodes.ProcNewNode;
 import org.truffleruby.core.proc.ProcOperations;
 import org.truffleruby.core.proc.RubyProc;
+import org.truffleruby.core.range.RubyIntOrLongRange;
+import org.truffleruby.core.range.RubyObjectRange;
 import org.truffleruby.core.rope.CodeRange;
 import org.truffleruby.core.rope.Rope;
 import org.truffleruby.core.rope.RopeNodes;
@@ -125,7 +127,8 @@
 import org.truffleruby.language.objects.AllocationTracing;
 import org.truffleruby.language.objects.CheckIVarNameNode;
 import org.truffleruby.language.objects.IsANode;
-import org.truffleruby.language.objects.IsImmutableObjectNode;
+import org.truffleruby.language.objects.IsCopyableObjectNode;
+import org.truffleruby.language.objects.IsCopyableObjectNodeGen;
 import org.truffleruby.language.objects.LogicalClassNode;
 import org.truffleruby.language.objects.MetaClassNode;
 import org.truffleruby.language.objects.ShapeCachingGuards;
@@ -522,7 +525,7 @@ protected RubyDynamicObject copyRubyDynamicObject(RubyDynamicObject self,
         }
 
         @Specialization
-        protected RubyClass copyRubyClass(RubyClass self,
+        protected RubyDynamicObject copyRubyClass(RubyClass self,
                 @Cached CopyInstanceVariablesNode copyInstanceVariablesNode) {
             var newClass = new RubyClass(coreLibrary().classClass, getLanguage(), getEncapsulatingSourceSection(),
                     null, null, false, null, self.superclass);
@@ -531,33 +534,40 @@ protected RubyClass copyRubyClass(RubyClass self,
         }
 
         @Specialization
-        protected RubyString copyImmutableString(ImmutableRubyString string,
+        protected RubyDynamicObject copy(ImmutableRubyString string,
                 @Cached DispatchNode allocateStringNode) {
             return (RubyString) allocateStringNode.call(coreLibrary().stringClass, "__allocate__");
         }
+
+        @Specialization
+        protected RubyDynamicObject copy(RubyIntOrLongRange range,
+                @Cached DispatchNode allocateRangeNode) {
+            return (RubyObjectRange) allocateRangeNode.call(coreLibrary().rangeClass, "__allocate__");
+        }
     }
 
-    @Primitive(name = "object_clone")
+    @Primitive(name = "object_clone") // "clone"
     @NodeChild(value = "object", type = RubyNode.class)
     @NodeChild(value = "freeze", type = RubyBaseNodeWithExecute.class)
     public abstract static class CloneNode extends PrimitiveNode {
 
-        @Child private SingletonClassNode singletonClassNode;
+        @Child IsCopyableObjectNode isCopyableObjectNode = IsCopyableObjectNodeGen.create();
+        @Child SingletonClassNode singletonClassNode;
         private final BranchProfile cantUnfreezeErrorProfile = BranchProfile.create();
 
-        @Specialization(limit = "getRubyLibraryCacheLimit()")
-        protected RubyDynamicObject clone(RubyDynamicObject object, Object freeze,
+        @Specialization(guards = "isCopyableObjectNode.execute(object)", limit = "getRubyLibraryCacheLimit()")
+        protected RubyDynamicObject copyable(Object object, Object freeze,
+                @Cached MetaClassNode metaClassNode,
                 @Cached CopyNode copyNode,
                 @Cached DispatchNode initializeCloneNode,
                 @Cached ConditionProfile isSingletonProfile,
-                @Cached ConditionProfile isRubyClass,
                 @Cached HashingNodes.ToHashByHashCode hashNode,
                 @CachedLibrary("object") RubyLibrary rubyLibrary,
                 @CachedLibrary(limit = "getRubyLibraryCacheLimit()") RubyLibrary rubyLibraryFreeze) {
             final RubyDynamicObject newObject = copyNode.executeCopy(object);
 
             // Copy the singleton class if any.
-            final RubyClass selfMetaClass = object.getMetaClass();
+            final RubyClass selfMetaClass = metaClassNode.execute(object);
             if (isSingletonProfile.profile(selfMetaClass.isSingleton)) {
                 final RubyClass newObjectMetaClass = executeSingletonClass(newObject);
                 newObjectMetaClass.fields.initCopy(selfMetaClass);
@@ -581,58 +591,14 @@ protected RubyDynamicObject clone(RubyDynamicObject object, Object freeze,
             return newObject;
         }
 
-        @Specialization
-        protected boolean cloneBoolean(boolean object, Object freeze) {
-            if (forceNotFrozen(freeze)) {
-                raiseCantUnfreezeError(object);
-            }
-            return object;
-        }
-
-        @Specialization
-        protected int cloneInteger(int object, Object freeze) {
-            if (forceNotFrozen(freeze)) {
-                raiseCantUnfreezeError(object);
-            }
-            return object;
-        }
-
-        @Specialization
-        protected long cloneLong(long object, Object freeze) {
+        @Specialization(guards = "!isCopyableObjectNode.execute(object)")
+        protected Object notCopyable(Object object, Object freeze) {
             if (forceNotFrozen(freeze)) {
                 raiseCantUnfreezeError(object);
             }
             return object;
         }
 
-        @Specialization
-        protected double cloneFloat(double object, Object freeze) {
-            if (forceNotFrozen(freeze)) {
-                raiseCantUnfreezeError(object);
-            }
-            return object;
-        }
-
-        @Specialization(guards = "!isImmutableRubyString(object)")
-        protected ImmutableRubyObject cloneImmutableObject(ImmutableRubyObject object, Object freeze) {
-            if (forceNotFrozen(freeze)) {
-                raiseCantUnfreezeError(object);
-            }
-            return object;
-        }
-
-        @Specialization
-        protected RubyDynamicObject cloneImmutableRubyString(ImmutableRubyString object, Object freeze,
-                @CachedLibrary(limit = "getRubyLibraryCacheLimit()") RubyLibrary rubyLibraryFreeze,
-                @Cached MakeStringNode makeStringNode) {
-            final RubyDynamicObject newObject = makeStringNode.fromRope(object.rope, object.encoding);
-            if (!forceNotFrozen(freeze)) {
-                rubyLibraryFreeze.freeze(newObject);
-            }
-
-            return newObject;
-        }
-
         private RubyHash createFreezeBooleanHash(boolean freeze, HashingNodes.ToHashByHashCode hashNode) {
             final RubySymbol key = coreSymbols().FREEZE;
 
@@ -677,20 +643,19 @@ private RubyClass executeSingletonClass(RubyDynamicObject newObject) {
     public abstract static class DupNode extends AlwaysInlinedMethodNode {
         @Specialization
         protected Object dup(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget target,
-                @Cached IsImmutableObjectNode isImmutableObjectNode,
-                @Cached ConditionProfile immutableProfile,
+                @Cached IsCopyableObjectNode isCopyableObjectNode,
+                @Cached ConditionProfile isCopyableProfile,
                 @Cached CopyNode copyNode,
                 @Cached DispatchNode initializeDupNode) {
-            if (immutableProfile
-                    .profile(!(self instanceof ImmutableRubyString) && isImmutableObjectNode.execute(self))) {
-                return self;
-            }
+            if (isCopyableProfile.profile(isCopyableObjectNode.execute(self))) {
+                final RubyDynamicObject copy = copyNode.executeCopy(self);
 
-            final RubyDynamicObject newObject = copyNode.executeCopy(self);
+                initializeDupNode.call(copy, "initialize_dup", self);
 
-            initializeDupNode.call(newObject, "initialize_dup", self);
-
-            return newObject;
+                return copy;
+            } else {
+                return self;
+            }
         }
     }
 

src/main/java/org/truffleruby/core/numeric/RubyBignum.java

@@ -16,12 +16,12 @@
 import com.oracle.truffle.api.library.ExportMessage;
 import org.truffleruby.RubyContext;
 import org.truffleruby.core.klass.RubyClass;
-import org.truffleruby.language.ImmutableRubyObject;
+import org.truffleruby.language.ImmutableRubyObjectNotCopyable;
 
 import java.math.BigInteger;
 
 @ExportLibrary(InteropLibrary.class)
-public class RubyBignum extends ImmutableRubyObject {
+public class RubyBignum extends ImmutableRubyObjectNotCopyable {
 
     public final BigInteger value;
 

src/main/java/org/truffleruby/core/range/RangeNodes.java

@@ -161,12 +161,7 @@ protected boolean excludeEnd(RubyObjectRange range) {
         }
 
         @Specialization
-        protected boolean excludeEnd(RubyIntRange range) {
-            return range.excludedEnd;
-        }
-
-        @Specialization
-        protected boolean excludeEnd(RubyLongRange range) {
+        protected boolean excludeEnd(RubyIntOrLongRange range) {
             return range.excludedEnd;
         }
 
@@ -192,86 +187,6 @@ protected Object eachObject(RubyObjectRange range) {
 
     }
 
-    @CoreMethod(names = "dup")
-    public abstract static class DupNode extends UnaryCoreMethodNode {
-
-        // NOTE(norswap): This is a hack, as it doesn't copy the ivars.
-        //   We do copy the logical class (but not the singleton class, to be MRI compatible).
-
-        @Specialization
-        protected RubyObjectRange dupIntRange(RubyIntRange range) {
-            // RubyIntRange means this isn't a Range subclass (cf. NewNode), we can use the shape directly.
-            final RubyObjectRange copy = new RubyObjectRange(
-                    coreLibrary().rangeClass,
-                    getLanguage().objectRangeShape,
-                    range.excludedEnd,
-                    range.begin,
-                    range.end,
-                    false);
-            AllocationTracing.trace(copy, this);
-            return copy;
-        }
-
-        @Specialization
-        protected RubyObjectRange dupLongRange(RubyLongRange range) {
-            // RubyLongRange means this isn't a Range subclass (cf. NewNode), we can use the shape directly.
-            final RubyObjectRange copy = new RubyObjectRange(
-                    coreLibrary().rangeClass,
-                    getLanguage().objectRangeShape,
-                    range.excludedEnd,
-                    range.begin,
-                    range.end,
-                    false);
-            AllocationTracing.trace(copy, this);
-            return copy;
-        }
-
-        @Specialization
-        protected RubyObjectRange dup(RubyObjectRange range) {
-            final RubyClass logicalClass = range.getLogicalClass();
-            final RubyObjectRange copy = new RubyObjectRange(
-                    logicalClass,
-                    getLanguage().objectRangeShape,
-                    range.excludedEnd,
-                    range.begin,
-                    range.end,
-                    false);
-            AllocationTracing.trace(copy, this);
-            return copy;
-        }
-    }
-
-    @CoreMethod(names = "clone")
-    public abstract static class CloneNode extends UnaryCoreMethodNode {
-
-        // NOTE(norswap): This is a hack, as it doesn't copy the ivars.
-        //   We do copy the logical class (but not the singleton class, to be MRI compatible).
-
-        @Specialization
-        protected RubyIntRange cloneIntRange(RubyIntRange range) {
-            return new RubyIntRange(range);
-        }
-
-        @Specialization
-        protected RubyLongRange dupLongRange(RubyLongRange range) {
-            return new RubyLongRange(range);
-        }
-
-        @Specialization
-        protected RubyObjectRange dup(RubyObjectRange range) {
-            final RubyClass logicalClass = range.getLogicalClass();
-            final RubyObjectRange copy = new RubyObjectRange(
-                    logicalClass,
-                    getLanguage().objectRangeShape,
-                    range.excludedEnd,
-                    range.begin,
-                    range.end,
-                    range.frozen);
-            AllocationTracing.trace(copy, this);
-            return copy;
-        }
-    }
-
     @CoreMethod(names = "end")
     public abstract static class EndNode extends CoreMethodArrayArgumentsNode {
 
@@ -517,6 +432,34 @@ protected RubyObjectRange allocate(RubyClass rubyClass) {
         }
     }
 
+    @CoreMethod(names = "initialize_copy", required = 1, raiseIfFrozenSelf = true)
+    public abstract static class InitializeCopyNode extends CoreMethodArrayArgumentsNode {
+
+        @Specialization
+        protected RubyObjectRange initializeCopy(RubyObjectRange self, RubyIntRange from) {
+            self.begin = from.begin;
+            self.end = from.end;
+            self.excludedEnd = from.excludedEnd;
+            return self;
+        }
+
+        @Specialization
+        protected RubyObjectRange initializeCopy(RubyObjectRange self, RubyLongRange from) {
+            self.begin = from.begin;
+            self.end = from.end;
+            self.excludedEnd = from.excludedEnd;
+            return self;
+        }
+
+        @Specialization
+        protected RubyObjectRange initializeCopy(RubyObjectRange self, RubyObjectRange from) {
+            self.begin = from.begin;
+            self.end = from.end;
+            self.excludedEnd = from.excludedEnd;
+            return self;
+        }
+    }
+
     /** Returns an array containing normalized int range parameters {@code [start, length]}, such that both are 32-bits
      * java ints (if conversion is impossible, an error is raised). The method attempts to make the values positive, by
      * adding {@code size} to them if they are negative. They may still be negative after the operation however, as

src/main/java/org/truffleruby/core/range/RubyIntOrLongRange.java

@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2022 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 2.0, or
+ * GNU General Public License version 2, or
+ * GNU Lesser General Public License version 2.1.
+ */
+package org.truffleruby.core.range;
+
+import org.truffleruby.RubyContext;
+import org.truffleruby.language.ImmutableRubyObjectCopyable;
+import org.truffleruby.language.dispatch.DispatchNode;
+
+import com.oracle.truffle.api.dsl.Cached;
+import com.oracle.truffle.api.dsl.Cached.Exclusive;
+import com.oracle.truffle.api.interop.InteropLibrary;
+import com.oracle.truffle.api.library.CachedLibrary;
+import com.oracle.truffle.api.library.ExportLibrary;
+import com.oracle.truffle.api.library.ExportMessage;
+
+@ExportLibrary(InteropLibrary.class)
+public abstract class RubyIntOrLongRange extends ImmutableRubyObjectCopyable {
+
+    public final boolean excludedEnd;
+
+    public RubyIntOrLongRange(boolean excludedEnd) {
+        this.excludedEnd = excludedEnd;
+    }
+
+    @ExportMessage
+    public boolean hasIterator() {
+        return true;
+    }
+
+    @ExportMessage
+    public Object getIterator(
+            @CachedLibrary("this") InteropLibrary node,
+            @Exclusive @Cached DispatchNode dispatchNode) {
+        final RubyContext context = RubyContext.get(node);
+        return dispatchNode.call(context.getCoreLibrary().truffleInteropOperationsModule, "get_iterator", this);
+    }
+
+}