Fix regressions after chr fixes. uptoCommon was changing our single byte

encodings because they were shared String buts setEncoding was changing their encoding. This seems like a tiny fix but it has a large change in it. If we setEncoding() on a shared String it will actually stop sharing the underlying bytelist. In practice, this could mean some extra bytelist creation behind the scenes. What it fixes is n strings sharing the same bytelist suddenly getting a new encoding because one decided to have force_encoding called on it. Making that tiny change also forced a couple of other changes because flags for sharing are fragile due to order dependency when constructing Ruby Strings. Also tweaked ordering of a conditional which wrankled my delicate sensibilities.
jruby · Feb 14, 2018 · f72c1b9 · f72c1b9
1 parent f4df68b
commit f72c1b9
Showing 2 changed files with 5 additions and 5 deletions.
diff --git a/core/src/main/java/org/jruby/RubyInteger.java b/core/src/main/java/org/jruby/RubyInteger.java
@@ -328,7 +328,7 @@ public RubyString chr(ThreadContext context) {
 
         Encoding enc;
 
-        if (0xff < i) {
+        if (i > 0xff) {
             enc = runtime.getDefaultInternalEncoding();
             if (enc == null) {
                 throw runtime.newRangeError(toString() + " out of char range");

diff --git a/core/src/main/java/org/jruby/RubyString.java b/core/src/main/java/org/jruby/RubyString.java
@@ -167,6 +167,7 @@ public Encoding getEncoding() {
 
     @Override
     public void setEncoding(Encoding encoding) {
+        modify();
         value.setEncoding(encoding);
     }
 
@@ -384,8 +385,8 @@ public RubyString(Ruby runtime, RubyClass rubyClass, ByteList value, Encoding en
 
     protected RubyString(Ruby runtime, RubyClass rubyClass, ByteList value, Encoding enc, int cr) {
         this(runtime, rubyClass, value);
-        value.setEncoding(enc);
         flags |= cr;
+        value.setEncoding(enc);
     }
 
     protected RubyString(Ruby runtime, RubyClass rubyClass, ByteList value, Encoding enc) {
@@ -3275,9 +3276,8 @@ final IRubyObject uptoCommon(ThreadContext context, IRubyObject arg, boolean exc
             byte e = end.value.getUnsafeBytes()[end.value.getBegin()];
             if (c > e || (excl && c == e)) return this;
             while (true) {
-                RubyString s = new RubyString(runtime, runtime.getString(), RubyInteger.SINGLE_CHAR_BYTELISTS[c & 0xff],
-                                                                            enc, CR_7BIT);
-                s.shareLevel = SHARE_LEVEL_BYTELIST;
+                RubyString s = newStringShared(runtime, RubyInteger.SINGLE_CHAR_BYTELISTS[c & 0xff], CR_7BIT);
+                if (!s.getEncoding().equals(enc)) s.setEncoding(enc);
                 block.yield(context, asSymbol ? runtime.newSymbol(s.toString()) : s);
 
                 if (!excl && c == e) break;