jruby · Mar 23, 2015 · Mar 23, 2015 · Mar 23, 2015 · Mar 23, 2015 · Mar 23, 2015
diff --git a/spec/truffle/tags/core/string/each_byte_tags.txt b/spec/truffle/tags/core/string/each_byte_tags.txt
diff --git a/spec/truffle/tags/core/string/element_set_tags.txt b/spec/truffle/tags/core/string/element_set_tags.txt
@@ -4,7 +4,6 @@ fails:String#[]= with Fixnum index calls to_int on index
 fails:String#[]= with Fixnum index calls #to_int to convert the index
 fails:String#[]= with Fixnum index raises a TypeError if #to_int does not return an Fixnum
 fails:String#[]= with Fixnum index raises an IndexError if #to_int returns a value out of range
-fails:String#[]= with Fixnum index raises an Encoding::CompatibilityError if the replacement encoding is incompatible
 fails:String#[]= with String index replaces fewer characters with more characters
 fails:String#[]= with String index replaces more characters with fewer characters
 fails:String#[]= with String index replaces characters with no characters
@@ -30,7 +29,6 @@ fails:String#[]= with a Regexp index with 3 arguments allows the specified captu
 fails:String#[]= with a Regexp index with 3 arguments checks the match index before calling #to_str to convert the replacement
 fails:String#[]= with a Regexp index with 3 arguments raises IndexError if the specified capture isn't available
 fails:String#[]= with a Regexp index with 3 arguments when the optional capture does not match raises an IndexError before setting the replacement
-fails:String#[]= with a Range index raises an Encoding::CompatibilityError if the replacement encoding is incompatible
 fails:String#[]= with Fixnum index, count starts at idx and overwrites count characters before inserting the rest of other_str
 fails:String#[]= with Fixnum index, count counts negative idx values from end of the string
 fails:String#[]= with Fixnum index, count overwrites and deletes characters if count is more than the length of other_str

diff --git a/spec/truffle/tags/core/string/gsub_tags.txt b/spec/truffle/tags/core/string/gsub_tags.txt
@@ -2,7 +2,5 @@ fails:String#gsub with pattern and replacement treats \+ as an empty string if t
 fails:String#gsub with pattern and block sets $~ for access from the block
 fails:String#gsub with pattern and block restores $~ after leaving the block
 passes:String#gsub with pattern and block converts the block's return value to a string using to_s
-fails:String#gsub! with pattern and block uses the compatible encoding if they are compatible
-fails:String#gsub! with pattern and block replaces the incompatible part properly even if the encodings are not compatible
 fails:String#gsub with pattern and replacement respects $KCODE when the pattern collapses
 fails:String#gsub with pattern and replacement handles pattern collapse without $KCODE
diff --git a/spec/truffle/tags/core/string/insert_tags.txt b/spec/truffle/tags/core/string/insert_tags.txt
diff --git a/spec/truffle/tags/core/string/split_tags.txt b/spec/truffle/tags/core/string/split_tags.txt
@@ -1,6 +1,4 @@
 fails:String#split with String splits on multibyte characters
-fails:String#split with String returns subclass instances based on self
 fails:String#split with String taints the resulting strings if self is tainted
-fails:String#split with Regexp returns subclass instances based on self
 fails:String#split with Regexp taints the resulting strings if self is tainted
 fails:String#split with Regexp returns an ArgumentError if an invalid UTF-8 string is supplied
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
@@ -53,6 +53,7 @@
 import org.jruby.truffle.nodes.coerce.ToStrNode;
 import org.jruby.truffle.nodes.coerce.ToStrNodeFactory;
 import org.jruby.truffle.nodes.dispatch.CallDispatchHeadNode;
+import org.jruby.truffle.nodes.dispatch.DispatchHeadNode;
 import org.jruby.truffle.nodes.dispatch.DispatchHeadNodeFactory;
 import org.jruby.truffle.nodes.objects.IsFrozenNode;
 import org.jruby.truffle.nodes.objects.IsFrozenNodeFactory;
@@ -636,25 +637,8 @@ public ElementSetNode(ElementSetNode prev) {
 
         @Specialization
         public RubyString elementSet(VirtualFrame frame, RubyString string, int index, Object replacement) {
-            notDesignedForCompilation();
-
-            if (index < 0) {
-                if (-index > string.length()) {
-                    CompilerDirectives.transferToInterpreter();
-
-                    throw new RaiseException(getContext().getCoreLibrary().indexError(String.format("index %d out of string", index), this));
-                }
-
-                index = index + string.length();
-
-            } else if (index > string.length()) {
-                CompilerDirectives.transferToInterpreter();
-
-                throw new RaiseException(getContext().getCoreLibrary().indexError(String.format("index %d out of string", index), this));
-            }
-
             final RubyString coerced = toStrNode.executeRubyString(frame, replacement);
-            StringSupport.replaceInternal19(index, 1, string, coerced);
+            StringNodesHelper.replaceInternal(string, StringNodesHelper.checkIndex(string, index, this), 1, coerced);
 
             return coerced;
         }
@@ -699,7 +683,7 @@ public RubyString elementSet(VirtualFrame frame, RubyString string, RubyRange.In
             }
 
             final RubyString coerced = toStrNode.executeRubyString(frame, replacement);
-            StringSupport.replaceInternal19(begin, length, string, coerced);
+            StringNodesHelper.replaceInternal(string, StringNodesHelper.checkIndex(string, begin, this), length, coerced);
 
             return coerced;
         }
@@ -1423,61 +1407,51 @@ public Object initializeCopy(RubyString self, RubyString from) {
 
     }
 
-    @CoreMethod(names = "insert", required = 2, lowerFixnumParameters = 0, raiseIfFrozenSelf = true, taintFromParameters = 1)
-    public abstract static class InsertNode extends CoreMethodNode {
+    @CoreMethod(names = "insert", required = 2, lowerFixnumParameters = 0, raiseIfFrozenSelf = true)
+    @NodeChildren({
+        @NodeChild(value = "string"),
+        @NodeChild(value = "index"),
+        @NodeChild(value = "otherString")
+    })
+    public abstract static class InsertNode extends RubyNode {
 
-        @Child private ConcatNode concatNode;
-        @Child private GetIndexNode getIndexNode;
+        @Child private CallDispatchHeadNode concatNode;
+        @Child private TaintResultNode taintResultNode;
 
         public InsertNode(RubyContext context, SourceSection sourceSection) {
             super(context, sourceSection);
-            concatNode = StringNodesFactory.ConcatNodeFactory.create(context, sourceSection, null, null);
-            getIndexNode = StringNodesFactory.GetIndexNodeFactory.create(context, sourceSection, new RubyNode[]{});
+            concatNode = DispatchHeadNodeFactory.createMethodCall(context);
+            taintResultNode = new TaintResultNode(context, sourceSection, false, new int[] {});
         }
 
         public InsertNode(InsertNode prev) {
             super(prev);
             concatNode = prev.concatNode;
-            getIndexNode = prev.getIndexNode;
+            taintResultNode = prev.taintResultNode;
         }
 
-        @Specialization
-        public RubyString insert(VirtualFrame frame, RubyString string, int index, RubyString otherString) {
-            notDesignedForCompilation();
+        @CreateCast("index") public RubyNode coerceIndexToInt(RubyNode index) {
+            return ToIntNodeFactory.create(getContext(), getSourceSection(), index);
+        }
 
-            if (index == -1) {
-                concatNode.concat(string, otherString);
+        @CreateCast("otherString") public RubyNode coerceOtherToString(RubyNode other) {
+            return ToStrNodeFactory.create(getContext(), getSourceSection(), other);
+        }
 
-                return string;
+        @Specialization
+        public Object insert(VirtualFrame frame, RubyString string, int index, RubyString otherString) {
+            if (index == -1) {
+                return concatNode.call(frame, string, "<<", null, otherString);
 
             } else if (index < 0) {
                 // Incrementing first seems weird, but MRI does it and it's significant because it uses the modified
                 // index value in its error messages.  This seems wrong, but we should be compatible.
                 index++;
-
-                if (-index > string.length()) {
-                    CompilerDirectives.transferToInterpreter();
-
-                    throw new RaiseException(getContext().getCoreLibrary().indexError(String.format("index %d out of string", index), this));
-                }
-
-                index = index + string.length();
-
-            } else if (index > string.length()) {
-                CompilerDirectives.transferToInterpreter();
-
-                throw new RaiseException(getContext().getCoreLibrary().indexError(String.format("index %d out of string", index), this));
             }
 
-            // TODO (Kevin): using node directly and cast
-            RubyString firstPart = (RubyString) getIndexNode.slice(frame, string, 0, index);
-            RubyString secondPart = (RubyString) getIndexNode.slice(frame, string, index, string.length());
+            StringNodesHelper.replaceInternal(string, StringNodesHelper.checkIndex(string, index, this), 0, otherString);
 
-            RubyString concatenated = concatNode.concat(concatNode.concat(firstPart, otherString), secondPart);
-
-            string.set(concatenated.getBytes());
-
-            return string;
+            return taintResultNode.maybeTaint(otherString, string);
         }
     }
 
@@ -2415,6 +2389,33 @@ public static ByteList swapcase(RubyString string) {
 
             return byteListString;
         }
+
+        public static int checkIndex(RubyString string, int index, RubyNode node) {
+            if (index > string.length()) {
+                CompilerDirectives.transferToInterpreter();
+
+                throw new RaiseException(
+                        node.getContext().getCoreLibrary().indexError(String.format("index %d out of string", index), node));
+            }
+
+            if (index < 0) {
+                if (-index > string.length()) {
+                    CompilerDirectives.transferToInterpreter();
+
+                    throw new RaiseException(
+                            node.getContext().getCoreLibrary().indexError(String.format("index %d out of string", index), node));
+                }
+
+                index += string.length();
+            }
+
+            return index;
+        }
+
+        @TruffleBoundary
+        public static void replaceInternal(RubyString string, int start, int length, RubyString replacement) {
+            StringSupport.replaceInternal19(start, length, string, replacement);
+        }
     }
 
 }
diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/RubyContext.java b/truffle/src/main/java/org/jruby/truffle/runtime/RubyContext.java
@@ -423,6 +423,8 @@ public RubyException toTruffle(org.jruby.RubyException jrubyException, RubyNode
         switch (jrubyException.getMetaClass().getName()) {
             case "ArgumentError":
                 return getCoreLibrary().argumentError(jrubyException.getMessage().toString(), currentNode);
+            case "Encoding::CompatibilityError":
+                return getCoreLibrary().encodingCompatibilityError(jrubyException.getMessage().toString(), currentNode);
             case "TypeError":
                 return getCoreLibrary().typeError(jrubyException.getMessage().toString(), currentNode);
         }

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
@@ -197,13 +197,24 @@ public final void modify(int length) {
     }
 
     @Override
+    @TruffleBoundary
     public Encoding checkEncoding(CodeRangeable other) {
-        // TODO (nirvdrum 16-Mar-15) This will return null for bad cases and we really don't want to propagate that.  Need a way to raise an appropriate exception while adhering to the interface.
-        return StringSupport.areCompatible(this, other);
+        final Encoding encoding = StringSupport.areCompatible(this, other);
+
+        // TODO (nirvdrum 23-Mar-15) We need to raise a proper Truffle+JRuby exception here, rather than a non-Truffle JRuby exception.
+        if (encoding == null) {
+            throw getContext().getRuntime().newEncodingCompatibilityError(
+                    String.format("incompatible character encodings: %s and %s",
+                            getByteList().getEncoding().toString(),
+                            other.getByteList().getEncoding().toString()));
+        }
+
+        return encoding;
     }
 
+    @TruffleBoundary
     public Encoding checkEncoding(CodeRangeable other, Node node) {
-        final Encoding encoding = checkEncoding(other);
+        final Encoding encoding = StringSupport.areCompatible(this, other);
 
         if (encoding == null) {
             throw new RaiseException(