Merge pull request oracle#451 in G/truffleruby from clone-freeze to m…

…aster

* commit 'b420558731a11bb8111f589cf45d076edf43edca':
  Document keywordAsOptional
  Exception message
  Implement Kernel#clone(freeze:)
  Add support for turning one keyword argument into a trailing optional in the DSL
  Rename MISSING to NOT_PROVIDED to match change ages ago
  Dead code

CHANGELOG.md

@@ -17,6 +17,7 @@ New features
 * `Mutex` and `ConditionVariable` have a new fast path for acquiring locks
   that are unlocked.
 * `Queue` and `SizedQueue`, `#close` and `#closed?`, have been implemented.
+* `Kernel#clone(freeze)` has been implemented (#1454).
 
 Changes:
 

spec/tags/core/kernel/clone_tags.txt

@@ -3,4 +3,3 @@ fails:Kernel#clone returns true for TrueClass
 fails:Kernel#clone returns false for FalseClass
 fails:Kernel#clone returns the same Integer for Integer
 fails:Kernel#clone returns the same Symbol for Symbol
-fails:Kernel#clone takes an option to copy freeze state or not

src/annotations/java/org/truffleruby/builtins/CoreMethod.java

@@ -48,6 +48,12 @@
 
     int optional() default 0;
 
+    /**
+     * Declares that a keyword argument with this name will be passed into the method as if it was an
+     * extra trailing positional optional argument.
+     */
+    String keywordAsOptional() default "";
+
     boolean rest() default false;
 
     boolean needsBlock() default false;

src/main/java/org/truffleruby/builtins/CoreMethodNodeManager.java

@@ -31,8 +31,10 @@
 import org.truffleruby.language.RubyRootNode;
 import org.truffleruby.language.Visibility;
 import org.truffleruby.language.arguments.MissingArgumentBehavior;
+import org.truffleruby.language.arguments.NotProvidedNode;
 import org.truffleruby.language.arguments.ProfileArgumentNodeGen;
 import org.truffleruby.language.arguments.ReadBlockNode;
+import org.truffleruby.language.arguments.ReadKeywordArgumentNode;
 import org.truffleruby.language.arguments.ReadPreArgumentNode;
 import org.truffleruby.language.arguments.ReadRemainingArgumentsNode;
 import org.truffleruby.language.arguments.ReadSelfNode;
@@ -138,23 +140,23 @@ private void addCoreMethod(DynamicObject module, MethodDetails methodDetails) {
         verifyUsage(module, methodDetails, method, visibility);
 
         final SharedMethodInfo sharedMethodInfo = makeSharedMethodInfo(context, module,
-                method.required(), method.optional(), method.rest(), names[0]);
+                method.required(), method.optional(), method.rest(), method.keywordAsOptional().isEmpty() ? null : method.keywordAsOptional(), names[0]);
         final CallTarget callTarget = makeGenericMethod(context, methodDetails.getNodeFactory(), methodDetails.getMethodAnnotation(), sharedMethodInfo);
 
         final boolean onSingleton = method.onSingleton() || method.constructor();
         addMethods(module, method.isModuleFunction(), onSingleton, names, visibility, sharedMethodInfo, callTarget);
     }
 
     public void addLazyCoreMethod(String nodeFactoryName, String moduleName, Visibility visibility,
-            boolean isModuleFunction, boolean onSingleton, int required, int optional, boolean rest, String... names) {
+            boolean isModuleFunction, boolean onSingleton, int required, int optional, boolean rest, String keywordAsOptional, String... names) {
         final DynamicObject module = getModule(moduleName);
 
-        final SharedMethodInfo sharedMethodInfo = makeSharedMethodInfo(context, module, required, optional, rest, names[0]);
+        final SharedMethodInfo sharedMethodInfo = makeSharedMethodInfo(context, module, required, optional, rest, keywordAsOptional, names[0]);
 
         final RubyNode methodNode = new LazyRubyNode(() -> {
             final NodeFactory<? extends RubyNode> nodeFactory = loadNodeFactory(nodeFactoryName);
             final CoreMethod methodAnnotation = nodeFactory.getNodeClass().getAnnotation(CoreMethod.class);
-            return createCoreMethodNode(context, nodeFactory, methodAnnotation, sharedMethodInfo);
+            return createCoreMethodNode(nodeFactory, methodAnnotation, sharedMethodInfo);
         });
 
         final CallTarget callTarget = createCallTarget(context, sharedMethodInfo, methodNode);
@@ -216,13 +218,21 @@ private static void addMethod(RubyContext context, DynamicObject module, SharedM
     }
 
     private static SharedMethodInfo makeSharedMethodInfo(RubyContext context, DynamicObject module,
-            int required, int optional, boolean rest, String primaryName) {
+            int required, int optional, boolean rest,  String keywordAsOptional, String primaryName) {
         final LexicalScope lexicalScope = new LexicalScope(context.getRootLexicalScope(), module);
 
+        final Arity arity;
+
+        if (keywordAsOptional == null) {
+            arity = new Arity(required, optional, rest);
+        } else {
+            arity = new Arity(required, optional, rest, 0, new String[]{keywordAsOptional}, false);
+        }
+
         return new SharedMethodInfo(
                 context.getCoreLibrary().getSourceSection(),
                 lexicalScope,
-                new Arity(required, optional, rest),
+                arity,
                 module,
                 primaryName,
                 0,
@@ -234,9 +244,9 @@ private static SharedMethodInfo makeSharedMethodInfo(RubyContext context, Dynami
     private static CallTarget makeGenericMethod(RubyContext context, NodeFactory<? extends RubyNode> nodeFactory, CoreMethod method, SharedMethodInfo sharedMethodInfo) {
         final RubyNode methodNode;
         if (!TruffleOptions.AOT && context.getOptions().LAZY_CORE_METHOD_NODES) {
-            methodNode = new LazyRubyNode(() -> createCoreMethodNode(context, nodeFactory, method, sharedMethodInfo));
+            methodNode = new LazyRubyNode(() -> createCoreMethodNode(nodeFactory, method, sharedMethodInfo));
         } else {
-            methodNode = createCoreMethodNode(context, nodeFactory, method, sharedMethodInfo);
+            methodNode = createCoreMethodNode(nodeFactory, method, sharedMethodInfo);
         }
 
         return createCallTarget(context, sharedMethodInfo, methodNode);
@@ -247,7 +257,7 @@ private static CallTarget createCallTarget(RubyContext context, SharedMethodInfo
         return Truffle.getRuntime().createCallTarget(rootNode);
     }
 
-    public static RubyNode createCoreMethodNode(RubyContext context, NodeFactory<? extends RubyNode> nodeFactory, CoreMethod method, SharedMethodInfo sharedMethodInfo) {
+    public static RubyNode createCoreMethodNode(NodeFactory<? extends RubyNode> nodeFactory, CoreMethod method, SharedMethodInfo sharedMethodInfo) {
         final List<RubyNode> argumentsNodes = new ArrayList<>();
 
         final boolean needsSelf = needsSelf(method);
@@ -267,7 +277,7 @@ public static RubyNode createCoreMethodNode(RubyContext context, NodeFactory<? e
         }
 
         for (int n = 0; n < nArgs; n++) {
-            RubyNode readArgumentNode = ProfileArgumentNodeGen.create(new ReadPreArgumentNode(n, MissingArgumentBehavior.UNDEFINED));
+            RubyNode readArgumentNode = ProfileArgumentNodeGen.create(new ReadPreArgumentNode(n, MissingArgumentBehavior.NOT_PROVIDED));
             argumentsNodes.add(transformArgument(method, readArgumentNode, n + 1));
         }
 
@@ -279,17 +289,25 @@ public static RubyNode createCoreMethodNode(RubyContext context, NodeFactory<? e
             argumentsNodes.add(new ReadBlockNode(NotProvided.INSTANCE));
         }
 
-        RubyNode node = createNodeFromFactory(context, nodeFactory, argumentsNodes);
+        if (!method.keywordAsOptional().isEmpty()) {
+            if (optional > 0) {
+                throw new UnsupportedOperationException("core method has been declared with both optional arguments and a keyword-as-optional argument");
+            }
+
+            argumentsNodes.add(new ReadKeywordArgumentNode(required, method.keywordAsOptional(), new NotProvidedNode()));
+        }
+
+        RubyNode node = createNodeFromFactory(nodeFactory, argumentsNodes);
 
         final RubyNode checkArity = Translator.createCheckArityNode(sharedMethodInfo.getArity());
 
-        node = transformResult(context, method, node);
+        node = transformResult(method, node);
         node = Translator.sequence(null, Arrays.asList(checkArity, node));
 
         return new ExceptionTranslatingNode(node, method.unsupportedOperationBehavior());
     }
 
-    public static RubyNode createNodeFromFactory(RubyContext context, NodeFactory<? extends RubyNode> nodeFactory, List<RubyNode> argumentsNodes) {
+    public static RubyNode createNodeFromFactory(NodeFactory<? extends RubyNode> nodeFactory, List<RubyNode> argumentsNodes) {
         final List<List<Class<?>>> signatures = nodeFactory.getNodeSignatures();
 
         assert signatures.size() == 1;
@@ -327,7 +345,7 @@ private static RubyNode transformArgument(CoreMethod method, RubyNode argument,
         return argument;
     }
 
-    private static RubyNode transformResult(RubyContext context, CoreMethod method, RubyNode node) {
+    private static RubyNode transformResult(CoreMethod method, RubyNode node) {
         if (!method.enumeratorSize().isEmpty()) {
             assert !method.returnsEnumeratorIfNoBlock() : "Only one of enumeratorSize or returnsEnumeratorIfNoBlock can be specified";
             // TODO BF 6-27-2015 Handle multiple method names correctly

src/main/java/org/truffleruby/builtins/PrimitiveNodeConstructor.java

@@ -54,11 +54,11 @@ public RubyNode createCallPrimitiveNode(RubyContext context, Source source, Sour
         }
 
         for (int n = 0; n < argumentsCount; n++) {
-            RubyNode readArgumentNode = ProfileArgumentNodeGen.create(new ReadPreArgumentNode(n, MissingArgumentBehavior.UNDEFINED));
+            RubyNode readArgumentNode = ProfileArgumentNodeGen.create(new ReadPreArgumentNode(n, MissingArgumentBehavior.NOT_PROVIDED));
             arguments.add(transformArgument(readArgumentNode, n + 1));
         }
 
-        final RubyNode primitiveNode = CoreMethodNodeManager.createNodeFromFactory(context, factory, arguments);
+        final RubyNode primitiveNode = CoreMethodNodeManager.createNodeFromFactory(factory, arguments);
 
         return Translator.withSourceSection(sourceSection, new CallPrimitiveNode(primitiveNode, fallback));
     }

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

@@ -431,8 +431,12 @@ protected int getCacheLimit() {
 
     }
 
-    @CoreMethod(names = "clone")
-    public abstract static class CloneNode extends CoreMethodArrayArgumentsNode {
+    @CoreMethod(names = "clone", keywordAsOptional = "freeze")
+    @NodeChildren({
+            @NodeChild(type = RubyNode.class, value = "self"),
+            @NodeChild(type = RubyNode.class, value = "freeze")
+    })
+    public abstract static class CloneNode extends CoreMethodNode {
 
         @Child private CopyNode copyNode = CopyNodeFactory.create(null);
         @Child private CallDispatchHeadNode initializeCloneNode = CallDispatchHeadNode.createPrivate();
@@ -441,9 +445,15 @@ public abstract static class CloneNode extends CoreMethodArrayArgumentsNode {
         @Child private PropagateTaintNode propagateTaintNode = PropagateTaintNode.create();
         @Child private SingletonClassNode singletonClassNode;
 
+        @CreateCast("freeze")
+        public RubyNode coerceToBoolean(RubyNode freeze) {
+            return BooleanCastWithDefaultNodeGen.create(true, freeze);
+        }
+
         @Specialization
-        public DynamicObject clone(VirtualFrame frame, DynamicObject self,
+        public DynamicObject clone(VirtualFrame frame, DynamicObject self, boolean freeze,
                 @Cached("createBinaryProfile()") ConditionProfile isSingletonProfile,
+                @Cached("createBinaryProfile()") ConditionProfile freezeProfile,
                 @Cached("createBinaryProfile()") ConditionProfile isFrozenProfile,
                 @Cached("createBinaryProfile()") ConditionProfile isRubyClass) {
             final DynamicObject newObject = copyNode.executeCopy(frame, self);
@@ -459,7 +469,7 @@ public DynamicObject clone(VirtualFrame frame, DynamicObject self,
 
             propagateTaintNode.propagate(self, newObject);
 
-            if (isFrozenProfile.profile(isFrozenNode.executeIsFrozen(self))) {
+            if (freezeProfile.profile(freeze) && isFrozenProfile.profile(isFrozenNode.executeIsFrozen(self))) {
                 if (freezeNode == null) {
                     CompilerDirectives.transferToInterpreterAndInvalidate();
                     freezeNode = insert(FreezeNode.create());

src/main/java/org/truffleruby/language/arguments/MissingArgumentBehavior.java

@@ -10,5 +10,5 @@
 package org.truffleruby.language.arguments;
 
 public enum MissingArgumentBehavior {
-    RUNTIME_ERROR, UNDEFINED, NIL;
+    RUNTIME_ERROR, NOT_PROVIDED, NIL;
 }

src/main/java/org/truffleruby/language/arguments/NotProvidedNode.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2018 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.
+ */
+package org.truffleruby.language.arguments;
+
+import com.oracle.truffle.api.frame.VirtualFrame;
+import org.truffleruby.language.NotProvided;
+import org.truffleruby.language.RubyNode;
+
+public class NotProvidedNode extends RubyNode {
+
+    @Override
+    public Object execute(VirtualFrame frame) {
+        return NotProvided.INSTANCE;
+    }
+
+}

src/main/java/org/truffleruby/language/arguments/ReadPreArgumentNode.java

@@ -40,7 +40,7 @@ public Object execute(VirtualFrame frame) {
             case RUNTIME_ERROR:
                 throw new IndexOutOfBoundsException();
 
-            case UNDEFINED:
+            case NOT_PROVIDED:
                 return NotProvided.INSTANCE;
 
             case NIL:

src/main/java/org/truffleruby/language/methods/Arity.java

@@ -34,6 +34,7 @@ public Arity(int preRequired, int optional, boolean hasRest) {
         this(preRequired, optional, hasRest, 0, NO_KEYWORDS, false);
     }
 
+
     public Arity(int preRequired, int optional, boolean hasRest, int postRequired, String[] keywordArguments, boolean hasKeywordsRest) {
         this.preRequired = preRequired;
         this.optional = optional;

src/processor/java/org/truffleruby/processor/CoreClassProcessor.java

@@ -109,6 +109,7 @@ private void processCoreMethod(TypeElement element) throws IOException {
                                 method.required() + ", " +
                                 method.optional() + ", " +
                                 method.rest() + ", " +
+                                (method.keywordAsOptional().isEmpty() ? "null" : quote(method.keywordAsOptional())) + ", " +
                                 names + ");");
                     }