[Truffle] Fix Array#min and #max specialisations to fallback correctly.

jruby · Dec 7, 2015 · 9b7b419 · 9b7b419
1 parent a13820c
commit 9b7b419
Showing 4 changed files with 47 additions and 16 deletions.
diff --git a/spec/truffle/tags/truffle/array/max_tags.txt b/spec/truffle/tags/truffle/array/max_tags.txt
diff --git a/spec/truffle/tags/truffle/array/min_tags.txt b/spec/truffle/tags/truffle/array/min_tags.txt
diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/array/ArrayNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/array/ArrayNodes.java
@@ -2113,7 +2113,7 @@ private Object write(VirtualFrame frame, DynamicObject array, int index, Object
 
     // TODO: move into Enumerable?
 
-    @CoreMethod(names = "max")
+    @CoreMethod(names = "max", needsBlock = true)
     public abstract static class MaxNode extends ArrayCoreMethodNode {
 
         @Child private CallDispatchHeadNode eachNode;
@@ -2126,7 +2126,7 @@ public MaxNode(RubyContext context, SourceSection sourceSection) {
         }
 
         @Specialization
-        public Object max(VirtualFrame frame, DynamicObject array) {
+        public Object max(VirtualFrame frame, DynamicObject array, NotProvided blockNotProvided) {
             // TODO: can we just write to the frame instead of having this indirect object?
 
             final Memo<Object> maximum = new Memo<>();
@@ -2149,6 +2149,11 @@ public Object max(VirtualFrame frame, DynamicObject array) {
             }
         }
 
+        @Specialization
+        public Object max(VirtualFrame frame, DynamicObject array, DynamicObject block) {
+            return ruby(frame, "array.max_internal(&block)", "array", array, "block", block);
+        }
+
     }
 
     public abstract static class MaxBlockNode extends CoreMethodArrayArgumentsNode {
@@ -2165,12 +2170,22 @@ public DynamicObject max(VirtualFrame frame, Object maximumObject, Object value)
             @SuppressWarnings("unchecked")
             final Memo<Object> maximum = (Memo<Object>) maximumObject;
 
-            // TODO(CS): cast
-
             final Object current = maximum.get();
 
-            if (current == null || (int) compareNode.call(frame, value, "<=>", null, current) > 0) {
+            if (current == null) {
                 maximum.set(value);
+            } else {
+                final Object compared = compareNode.call(frame, value, "<=>", null, current);
+
+                if (compared instanceof Integer) {
+                    if ((int) compared > 0) {
+                        maximum.set(value);
+                    }
+                } else {
+                    CompilerDirectives.transferToInterpreter();
+                    // Should be the actual type and object in this string - but this method should go away soon
+                    throw new RaiseException(getContext().getCoreLibrary().argumentError("comparison of X with Y failed", this));
+                }
             }
 
             return nil();
@@ -2218,7 +2233,7 @@ public CallTarget getCallTarget() {
         }
     }
 
-    @CoreMethod(names = "min")
+    @CoreMethod(names = "min", needsBlock = true)
     public abstract static class MinNode extends ArrayCoreMethodNode {
 
         @Child private CallDispatchHeadNode eachNode;
@@ -2231,7 +2246,7 @@ public MinNode(RubyContext context, SourceSection sourceSection) {
         }
 
         @Specialization
-        public Object min(VirtualFrame frame, DynamicObject array) {
+        public Object min(VirtualFrame frame, DynamicObject array, NotProvided blockNotProvided) {
             // TODO: can we just write to the frame instead of having this indirect object?
 
             final Memo<Object> minimum = new Memo<>();
@@ -2254,6 +2269,11 @@ public Object min(VirtualFrame frame, DynamicObject array) {
             }
         }
 
+        @Specialization
+        public Object min(VirtualFrame frame, DynamicObject array, DynamicObject block) {
+            return ruby(frame, "array.min_internal(&block)", "array", array, "block", block);
+        }
+
     }
 
     public abstract static class MinBlockNode extends CoreMethodArrayArgumentsNode {
@@ -2270,12 +2290,22 @@ public DynamicObject min(VirtualFrame frame, Object minimumObject, Object value)
             @SuppressWarnings("unchecked")
             final Memo<Object> minimum = (Memo<Object>) minimumObject;
 
-            // TODO(CS): cast
-
             final Object current = minimum.get();
 
-            if (current == null || (int) compareNode.call(frame, value, "<=>", null, current) < 0) {
+            if (current == null) {
                 minimum.set(value);
+            } else {
+                final Object compared = compareNode.call(frame, value, "<=>", null, current);
+
+                if (compared instanceof Integer) {
+                    if ((int) compared < 0) {
+                        minimum.set(value);
+                    }
+                } else {
+                    CompilerDirectives.transferToInterpreter();
+                    // Should be the actual type and object in this string - but this method should go away soon
+                    throw new RaiseException(getContext().getCoreLibrary().argumentError("comparison of X with Y failed", this));
+                }
             }
 
             return nil();

diff --git a/truffle/src/main/ruby/core/shims.rb b/truffle/src/main/ruby/core/shims.rb
@@ -251,6 +251,13 @@ def inplace_mode=(ext)
 
 end
 
+module Enumerable
+
+  alias_method :min_internal, :min
+  alias_method :max_internal, :max
+
+end
+
 # JRuby uses this for example to make proxy settings visible to stdlib/uri/common.rb
 
 ENV_JAVA = {}