[GR-19220] Replace boundary on StringCharacterIndexPrimitiveNode (#2383)

PullRequest: truffleruby/2782

Author: eregon

spec/ruby/core/string/gsub_spec.rb

@@ -594,6 +594,14 @@ def obj.to_s() "ok" end
   end
 end
 
+describe "String#gsub with a string pattern" do
+  it "handles multibyte characters" do
+    "é".gsub("é", "â").should == "â"
+    "aé".gsub("é", "â").should == "aâ"
+    "éa".gsub("é", "â").should == "âa"
+  end
+end
+
 describe "String#gsub! with pattern and replacement" do
   it "modifies self in place and returns self" do
     a = "hello"

spec/ruby/core/string/index_spec.rb

@@ -146,6 +146,7 @@
 
   it "returns the character index after offset" do
     "われわれ".index("わ", 1).should == 2
+    "ありがとうありがとう".index("が", 3).should == 7
   end
 
   it "returns the character index after a partial first match" do

src/main/java/org/truffleruby/core/string/StringNodes.java

@@ -4398,49 +4398,91 @@ protected int notValidUtf8(Object string, int byteIndex,
         }
     }
 
+    /** Search pattern in string starting after offset characters, and return a character index or nil */
     @Primitive(name = "string_character_index", lowerFixnum = 2)
-    public abstract static class StringCharacterIndexPrimitiveNode extends PrimitiveArrayArgumentsNode {
+    @NodeChild(value = "string", type = RubyNode.class)
+    @NodeChild(value = "pattern", type = RubyNode.class)
+    @NodeChild(value = "offset", type = RubyNode.class)
+    public abstract static class StringCharacterIndexNode extends PrimitiveNode {
 
-        @TruffleBoundary
-        @Specialization
-        protected Object stringCharacterIndex(Object string, Object pattern, int offset,
-                @CachedLibrary(limit = "2") RubyStringLibrary stringLibrary,
-                @CachedLibrary(limit = "2") RubyStringLibrary patternLibrary,
-                @Cached RopeNodes.CalculateCharacterLengthNode calculateCharacterLengthNode) {
-            if (offset < 0) {
-                return nil;
-            }
+        @Child SingleByteOptimizableNode singleByteOptimizableNode = SingleByteOptimizableNode.create();
 
-            final Rope stringRope = stringLibrary.getRope(string);
-            final Rope patternRope = patternLibrary.getRope(pattern);
+        @CreateCast("string")
+        protected RubyNode coerceStringToRope(RubyNode string) {
+            return ToRopeNodeGen.create(string);
+        }
 
-            final int total = stringRope.byteLength();
-            int p = 0;
-            final int e = p + total;
+        @CreateCast("pattern")
+        protected RubyNode coercePatternToRope(RubyNode pattern) {
+            return ToRopeNodeGen.create(pattern);
+        }
+
+        @Specialization(
+                guards = {
+                        "offset >= 0",
+                        "singleByteOptimizableNode.execute(stringRope)",
+                        "!patternFits(stringRope, patternRope, offset)" })
+        protected Object patternTooLarge(Rope stringRope, Rope patternRope, int offset) {
+            return nil;
+        }
+
+        @Specialization(
+                guards = {
+                        "offset >= 0",
+                        "singleByteOptimizableNode.execute(stringRope)",
+                        "patternFits(stringRope, patternRope, offset)" })
+        protected Object singleByteOptimizable(Rope stringRope, Rope patternRope, int offset,
+                @Cached RopeNodes.BytesNode stringBytesNode,
+                @Cached RopeNodes.BytesNode patternBytesNode,
+                @Cached LoopConditionProfile loopProfile,
+                @Cached("createCountingProfile()") ConditionProfile matchProfile) {
+
+            int p = offset;
+            final int e = stringRope.byteLength();
             final int pe = patternRope.byteLength();
             final int l = e - pe + 1;
 
-            final byte[] stringBytes = stringRope.getBytes();
-            final byte[] patternBytes = patternRope.getBytes();
+            final byte[] stringBytes = stringBytesNode.execute(stringRope);
+            final byte[] patternBytes = patternBytesNode.execute(patternRope);
 
-            if (stringRope.isSingleByteOptimizable()) {
-                for (p += offset; p < l; p++) {
-                    if (ArrayUtils.memcmp(stringBytes, p, patternBytes, 0, pe) == 0) {
+            try {
+                for (; loopProfile.profile(p < l); p++) {
+                    if (matchProfile.profile(ArrayUtils.memcmp(stringBytes, p, patternBytes, 0, pe) == 0)) {
                         return p;
                     }
                 }
-
-                return nil;
+            } finally {
+                LoopNode.reportLoopCount(this, p - offset);
             }
 
+            return nil;
+        }
+
+        @TruffleBoundary
+        @Specialization(
+                guards = {
+                        "offset >= 0",
+                        "!singleByteOptimizableNode.execute(stringRope)" })
+        protected Object multiByte(Rope stringRope, Rope patternRope, int offset,
+                @Cached RopeNodes.CalculateCharacterLengthNode calculateCharacterLengthNode,
+                @Cached RopeNodes.BytesNode stringBytesNode,
+                @Cached RopeNodes.BytesNode patternBytesNode) {
+
+            int p = 0;
+            final int e = stringRope.byteLength();
+            final int pe = patternRope.byteLength();
+            final int l = e - pe + 1;
+
+            final byte[] stringBytes = stringBytesNode.execute(stringRope);
+            final byte[] patternBytes = patternBytesNode.execute(patternRope);
+
             final Encoding enc = stringRope.getEncoding();
             final CodeRange cr = stringRope.getCodeRange();
-            int index = 0;
             int c = 0;
+            int index = 0;
 
             while (p < e && index < offset) {
                 c = calculateCharacterLengthNode.characterLength(enc, cr, Bytes.fromRange(stringBytes, p, e));
-
                 if (StringSupport.MBCLEN_CHARFOUND_P(c)) {
                     p += c;
                     index++;
@@ -4461,13 +4503,18 @@ protected Object stringCharacterIndex(Object string, Object pattern, int offset,
 
             return nil;
         }
+
+        protected boolean patternFits(Rope stringRope, Rope patternRope, int offset) {
+            return offset + patternRope.byteLength() <= stringRope.byteLength();
+        }
     }
 
+    /** Search pattern in string starting after offset bytes, and return a byte index or nil */
     @Primitive(name = "string_byte_index", lowerFixnum = 2)
     @NodeChild(value = "string", type = RubyNode.class)
     @NodeChild(value = "pattern", type = RubyNode.class)
     @NodeChild(value = "offset", type = RubyNode.class)
-    public abstract static class StringByteIndexPrimitiveNode extends PrimitiveNode {
+    public abstract static class StringByteIndexNode extends PrimitiveNode {
 
         @Child SingleByteOptimizableNode singleByteOptimizableNode = SingleByteOptimizableNode.create();
 
@@ -4481,26 +4528,17 @@ protected RubyNode coercePatternToRope(RubyNode pattern) {
             return ToRopeNodeGen.create(pattern);
         }
 
-        @Specialization(guards = "offset < 0")
-        protected Object stringByteIndexNegativeOffset(Rope stringRope, Rope patternRope, int offset) {
-            return nil;
-        }
-
-        @Specialization(
-                guards = {
-                        "offset >= 0",
-                        "singleByteOptimizableNode.execute(stringRope)",
-                        "patternRope.byteLength() > stringRope.byteLength()" })
-        protected Object stringByteIndexPatternTooLarge(Rope stringRope, Rope patternRope, int offset) {
+        @Specialization(guards = { "offset >= 0", "!patternFits(stringRope, patternRope, offset)" })
+        protected Object patternTooLarge(Rope stringRope, Rope patternRope, int offset) {
             return nil;
         }
 
         @Specialization(
                 guards = {
                         "offset >= 0",
                         "singleByteOptimizableNode.execute(stringRope)",
-                        "patternRope.byteLength() <= stringRope.byteLength()" })
-        protected Object stringCharacterIndexSingleByteOptimizable(Rope stringRope, Rope patternRope, int offset,
+                        "patternFits(stringRope, patternRope, offset)" })
+        protected Object singleByteOptimizable(Rope stringRope, Rope patternRope, int offset,
                 @Cached RopeNodes.BytesNode stringBytesNode,
                 @Cached RopeNodes.BytesNode patternBytesNode,
                 @Cached LoopConditionProfile loopProfile,
@@ -4532,8 +4570,8 @@ protected Object stringCharacterIndexSingleByteOptimizable(Rope stringRope, Rope
                 guards = {
                         "offset >= 0",
                         "!singleByteOptimizableNode.execute(stringRope)",
-                        "patternRope.byteLength() <= stringRope.byteLength()" })
-        protected Object stringCharacterIndex(Rope stringRope, Rope patternRope, int offset,
+                        "patternFits(stringRope, patternRope, offset)" })
+        protected Object multiByte(Rope stringRope, Rope patternRope, int offset,
                 @Cached RopeNodes.CalculateCharacterLengthNode calculateCharacterLengthNode,
                 @Cached RopeNodes.BytesNode stringBytesNode,
                 @Cached RopeNodes.BytesNode patternBytesNode) {
@@ -4548,7 +4586,7 @@ protected Object stringCharacterIndex(Rope stringRope, Rope patternRope, int off
 
             final Encoding enc = stringRope.getEncoding();
             final CodeRange cr = stringRope.getCodeRange();
-            int c = 0;
+            int c;
 
             for (; p < l; p += c) {
                 c = calculateCharacterLengthNode.characterLength(enc, cr, Bytes.fromRange(stringBytes, p, e));
@@ -4562,6 +4600,10 @@ protected Object stringCharacterIndex(Rope stringRope, Rope patternRope, int off
 
             return nil;
         }
+
+        protected boolean patternFits(Rope stringRope, Rope patternRope, int offset) {
+            return offset + patternRope.byteLength() <= stringRope.byteLength();
+        }
     }
 
     /** Calculates the byte offset of a character, indicated by a character index, starting from a provided byte offset

tool/jt.rb

@@ -2319,7 +2319,7 @@ def command_format(changed_java_files = nil)
     format_specializations_check
   end
 
-  private def check_filename_length
+  def check_filename_length
     # For eCryptfs, see https://bugs.launchpad.net/ecryptfs/+bug/344878
     max_length = 143