[Truffle] Completed String#{[], slice}.

jruby · Mar 31, 2015 · be1ad6d · be1ad6d
1 parent fb55c17
commit be1ad6d
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 50 deletions.
diff --git a/spec/truffle/tags/core/string/element_reference_tags.txt b/spec/truffle/tags/core/string/element_reference_tags.txt
diff --git a/spec/truffle/tags/core/string/slice_tags.txt b/spec/truffle/tags/core/string/slice_tags.txt
@@ -1,6 +1,3 @@
-fails:String#slice with Range calls to_int on range arguments
-fails:String#slice with Range works with Range subclasses
-fails:String#slice with Regexp, index returns nil if there is no capture for the given index
 fails:String#slice! with index calls to_int on index
 fails:String#slice! with index returns the character given by the character index
 fails:String#slice! with index, length deletes and returns the substring at idx and the given length
@@ -9,7 +6,6 @@ fails:String#slice! with index, length calls to_int on idx and length
 fails:String#slice! with index, length returns subclass instances
 fails:String#slice! Range calls to_int on range arguments
 fails:String#slice! Range works with Range subclasses
-fails:String#slice! Range returns the substring given by the character offsets of the range
 fails:String#slice! with Regexp deletes and returns the first match from self
 fails:String#slice! with Regexp always taints resulting strings when self or regexp is tainted
 fails:String#slice! with Regexp doesn't taint self when regexp is tainted
@@ -18,7 +14,6 @@ fails:String#slice! with Regexp sets $~ to MatchData when there is a match and n
 fails:String#slice! with Regexp, index deletes and returns the capture for idx from self
 fails:String#slice! with Regexp, index always taints resulting strings when self or regexp is tainted
 fails:String#slice! with Regexp, index doesn't taint self when regexp is tainted
-fails:String#slice! with Regexp, index returns nil if there is no capture for idx
 fails:String#slice! with Regexp, index calls to_int on idx
 fails:String#slice! with Regexp, index returns subclass instances
 fails:String#slice! with String removes and returns the first occurrence of other_str from self

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/StringNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/StringNodes.java
@@ -463,6 +463,7 @@ public abstract static class GetIndexNode extends CoreMethodNode {
         @Child private ToIntNode toIntNode;
         @Child private CallDispatchHeadNode includeNode;
         @Child private CallDispatchHeadNode dupNode;
+        @Child private SizeNode sizeNode;
         @Child private StringPrimitiveNodes.StringSubstringPrimitiveNode substringNode;
 
         private final BranchProfile outOfBounds = BranchProfile.create();
@@ -476,6 +477,7 @@ public GetIndexNode(GetIndexNode prev) {
             toIntNode = prev.toIntNode;
             includeNode = prev.includeNode;
             dupNode = prev.dupNode;
+            sizeNode = prev.sizeNode;
             substringNode = prev.substringNode;
         }
 
@@ -493,72 +495,77 @@ public Object getIndex(RubyString string, int index, UndefinedPlaceholder undefi
 
         @Specialization(guards = { "!isRubyRange(arguments[1])", "!isRubyRegexp(arguments[1])", "!isRubyString(arguments[1])" })
         public Object getIndex(VirtualFrame frame, RubyString string, Object index, UndefinedPlaceholder undefined) {
-            if (toIntNode == null) {
-                CompilerDirectives.transferToInterpreter();
-                toIntNode = insert(ToIntNodeFactory.create(getContext(), getSourceSection(), null));
-            }
+            return getIndex(string, getToIntNode().executeIntegerFixnum(frame, index), undefined);
+        }
+
+        @Specialization
+        public Object sliceIntegerRange(VirtualFrame frame, RubyString string, RubyRange.IntegerFixnumRange range, UndefinedPlaceholder undefined) {
+            return sliceRange(frame, string, range.getBegin(), range.getEnd(), range.doesExcludeEnd());
+        }
 
-            return getIndex(string, toIntNode.executeIntegerFixnum(frame, index), undefined);
+        @Specialization
+        public Object sliceLongRange(VirtualFrame frame, RubyString string, RubyRange.LongFixnumRange range, UndefinedPlaceholder undefined) {
+            // TODO (nirvdrum 31-Mar-15) The begin and end values should be properly lowered, only if possible.
+            return sliceRange(frame, string, (int) range.getBegin(), (int) range.getEnd(), range.doesExcludeEnd());
         }
 
         @Specialization
-        public Object slice(RubyString string, RubyRange.IntegerFixnumRange range, UndefinedPlaceholder undefined) {
-            notDesignedForCompilation();
+        public Object sliceObjectRange(VirtualFrame frame, RubyString string, RubyRange.ObjectRange range, UndefinedPlaceholder undefined) {
+            // TODO (nirvdrum 31-Mar-15) The begin and end values may return Fixnums beyond int boundaries and we should handle that -- Bignums are always errors.
+            final int coercedBegin = getToIntNode().executeIntegerFixnum(frame, range.getBegin());
+            final int coercedEnd = getToIntNode().executeIntegerFixnum(frame, range.getEnd());
 
-            final String javaString = string.toString();
-            final int begin = string.normalizeIndex(range.getBegin());
+            return sliceRange(frame, string, coercedBegin, coercedEnd, range.doesExcludeEnd());
+        }
+
+        private Object sliceRange(VirtualFrame frame, RubyString string, int begin, int end, boolean doesExcludeEnd) {
+            if (sizeNode == null) {
+                CompilerDirectives.transferToInterpreter();
+                sizeNode = insert(StringNodesFactory.SizeNodeFactory.create(getContext(), getSourceSection(), new RubyNode[]{null}));
+            }
 
-            if (begin < 0 || begin > javaString.length()) {
+            final int stringLength = sizeNode.executeIntegerFixnum(frame, string);
+            begin = string.normalizeIndex(stringLength, begin);
+
+            if (begin < 0 || begin > stringLength) {
                 outOfBounds.enter();
                 return nil();
             } else {
-                final int end = string.normalizeIndex(range.getEnd());
-                final int excludingEnd = string.clampExclusiveIndex(range.doesExcludeEnd() ? end : end+1);
 
-                if (begin > excludingEnd) {
-                    return getContext().makeString("");
+                if (begin == stringLength) {
+                    return getContext().makeString(string.getLogicalClass(), "", string.getByteList().getEncoding());
+                }
+
+                end = string.normalizeIndex(stringLength, end);
+                int length = string.clampExclusiveIndex(doesExcludeEnd ? end : end + 1);
+
+                if (length > stringLength) {
+                    length = stringLength;
+                }
+
+                length -= begin;
+
+                if (length < 0) {
+                    length = 0;
                 }
 
-                return getContext().makeString(string.getLogicalClass(),
-                        javaString.substring(begin, excludingEnd),
-                        string.getByteList().getEncoding());
+                return getSubstringNode().execute(frame, string, begin, length);
             }
         }
 
         @Specialization
         public Object slice(VirtualFrame frame, RubyString string, int start, int length) {
-            if (substringNode == null) {
-                CompilerDirectives.transferToInterpreter();
-
-                substringNode = insert(StringPrimitiveNodesFactory.StringSubstringPrimitiveNodeFactory.create(
-                        getContext(), getSourceSection(), new RubyNode[] { null, null, null }));
-            }
-
-            return substringNode.execute(frame, string, start, length);
+            return getSubstringNode().execute(frame, string, start, length);
         }
 
         @Specialization(guards = "!isUndefinedPlaceholder(arguments[2])")
         public Object slice(VirtualFrame frame, RubyString string, int start, Object length) {
-            notDesignedForCompilation();
-
-            if (toIntNode == null) {
-                CompilerDirectives.transferToInterpreter();
-                toIntNode = insert(ToIntNodeFactory.create(getContext(), getSourceSection(), null));
-            }
-
-            return slice(frame, string, start, toIntNode.executeIntegerFixnum(frame, length));
+            return slice(frame, string, start, getToIntNode().executeIntegerFixnum(frame, length));
         }
 
         @Specialization(guards = { "!isRubyRange(arguments[1])", "!isRubyRegexp(arguments[1])", "!isRubyString(arguments[1])", "!isUndefinedPlaceholder(arguments[2])" })
         public Object slice(VirtualFrame frame, RubyString string, Object start, Object length) {
-            notDesignedForCompilation();
-
-            if (toIntNode == null) {
-                CompilerDirectives.transferToInterpreter();
-                toIntNode = insert(ToIntNodeFactory.create(getContext(), getSourceSection(), null));
-            }
-
-            return slice(frame, string, toIntNode.executeIntegerFixnum(frame, start), toIntNode.executeIntegerFixnum(frame, length));
+            return slice(frame, string, getToIntNode().executeIntegerFixnum(frame, start), getToIntNode().executeIntegerFixnum(frame, length));
         }
 
         @Specialization
@@ -568,7 +575,7 @@ public Object slice(VirtualFrame frame, RubyString string, RubyRegexp regexp, Un
 
         @Specialization(guards = "!isUndefinedPlaceholder(arguments[2])")
         public Object slice(VirtualFrame frame, RubyString string, RubyRegexp regexp, Object capture) {
-            // Extract from Rubinius's definition of String#[].
+            // Extracted from Rubinius's definition of String#[].
             return ruby(frame, "match, str = subpattern(index, other); Regexp.last_match = match; str", "index", regexp, "other", capture);
         }
 
@@ -594,6 +601,26 @@ public Object slice(VirtualFrame frame, RubyString string, RubyString matchStr,
 
             return nil();
         }
+
+        private ToIntNode getToIntNode() {
+            if (toIntNode == null) {
+                CompilerDirectives.transferToInterpreter();
+                toIntNode = insert(ToIntNodeFactory.create(getContext(), getSourceSection(), null));
+            }
+
+            return toIntNode;
+        }
+
+        private StringPrimitiveNodes.StringSubstringPrimitiveNode getSubstringNode() {
+            if (substringNode == null) {
+                CompilerDirectives.transferToInterpreter();
+
+                substringNode = insert(StringPrimitiveNodesFactory.StringSubstringPrimitiveNodeFactory.create(
+                        getContext(), getSourceSection(), new RubyNode[] { null, null, null }));
+            }
+
+            return substringNode;
+        }
     }
 
     @CoreMethod(names = "[]=", required = 2, lowerFixnumParameters = 0, raiseIfFrozenSelf = true)

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/core/RubyString.java b/truffle/src/main/java/org/jruby/truffle/runtime/core/RubyString.java
@@ -85,8 +85,12 @@ public int length() {
         }
     }
 
+    public int normalizeIndex(int length, int index) {
+        return RubyArray.normalizeIndex(length, index);
+    }
+
     public int normalizeIndex(int index) {
-        return RubyArray.normalizeIndex(bytes.length(), index);
+        return normalizeIndex(length(), index);
     }
 
     public int clampExclusiveIndex(int index) {