Merge pull request jruby#8375 from headius/chilled_revisited

Revisit chilled strings and pass all specs
headius · Oct 14, 2024 · 9d4d157 · 9d4d157
2 parents b9d8001 + 6ff1e2f
commit 9d4d157
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 15 deletions.
diff --git a/core/src/main/java/org/jruby/RubyBasicObject.java b/core/src/main/java/org/jruby/RubyBasicObject.java
@@ -1606,13 +1606,8 @@ public void copyInstanceVariablesInto(final InstanceVariables other) {
      * including information about whether this object is frozen.
      * Will throw a suitable exception in that case.
      */
-    public final void ensureInstanceVariablesSettable() {
-        if (!isFrozen()) {
-            return;
-        } else if (this instanceof RubyString string) {
-            // We put this second to reduce overhead since most objects will not be frozen and we do not want this instanceof all the time.
-            string.frozenCheck();
-        } else {
+    public void ensureInstanceVariablesSettable() {
+        if (isFrozen()) {
             raiseFrozenError();
         }
     }

diff --git a/core/src/main/java/org/jruby/RubyString.java b/core/src/main/java/org/jruby/RubyString.java
@@ -451,6 +451,11 @@ protected RubyString(Ruby runtime, RubyClass rubyClass, ByteList value, int cr)
         flags |= cr;
     }
 
+    protected RubyString(Ruby runtime, RubyClass rubyClass, ByteList value, int cr, boolean objectspace) {
+        this(runtime, rubyClass, value, objectspace);
+        flags |= cr;
+    }
+
     // Deprecated String construction routines
 
     @Deprecated
@@ -536,7 +541,7 @@ public static RubyString newString(Ruby runtime, ByteList bytes, int coderange)
     }
 
     public static RubyString newChilledString(Ruby runtime, ByteList bytes, int coderange) {
-        return newString(runtime, bytes, coderange).chill();
+        return newStringShared(runtime, bytes, coderange).chill();
     }
 
     public static RubyString newString(Ruby runtime, ByteList bytes, Encoding encoding) {
@@ -984,9 +989,18 @@ protected void frozenCheck() {
         }
     }
 
+    @Override
+    public void checkFrozen() {
+        frozenCheck();
+    }
+
+    @Override
+    public final void ensureInstanceVariablesSettable() {
+        frozenCheck();
+    }
+
     private void mutateChilledString() {
         getRuntime().getWarnings().warn("literal string will be frozen in the future");
-        setFrozen(false);
         flags &= ~CHILLED_F;
     }
 
@@ -1086,7 +1100,7 @@ static class DebugFrozenString extends RubyString {
         private final int line;
 
         protected DebugFrozenString(Ruby runtime, RubyClass rubyClass, ByteList value, int cr, String file, int line) {
-            super(runtime, rubyClass, value, cr);
+            super(runtime, rubyClass, value, cr, false);
 
             this.file = file;
             this.line = line;
@@ -1123,7 +1137,7 @@ public static class FString extends RubyString {
         private IRubyObject converted;
 
         protected FString(Ruby runtime, RubyClass rubyClass, ByteList value, int cr) {
-            super(runtime, rubyClass, value, cr);
+            super(runtime, rubyClass, value, cr, false);
 
             // set flag for code that does not use isFrozen
             setFrozen(true);
@@ -1314,7 +1328,7 @@ public final IRubyObject minus_at(ThreadContext context) {
 
     @JRubyMethod(name = "+@") // +'foo' returns modifiable string
     public final IRubyObject plus_at() {
-        return isFrozen() ? this.dup() : this;
+        return isFrozen() | isChilled() ? this.dup() : this;
     }
 
     @Deprecated
@@ -6797,7 +6811,6 @@ public IRubyObject freeze(ThreadContext context) {
 
     public RubyString chill() {
         flags |= CHILLED_F;
-        setFrozen(true);
         return this;
     }
 

diff --git a/core/src/main/java/org/jruby/ir/persistence/IRDumper.java b/core/src/main/java/org/jruby/ir/persistence/IRDumper.java
@@ -18,6 +18,7 @@
 import org.jruby.ir.operands.Array;
 import org.jruby.ir.operands.Bignum;
 import org.jruby.ir.operands.BuiltinClass;
+import org.jruby.ir.operands.ChilledString;
 import org.jruby.ir.operands.ClosureLocalVariable;
 import org.jruby.ir.operands.Complex;
 import org.jruby.ir.operands.CurrentScope;
@@ -309,6 +310,7 @@ public void Array(Array array) {
     public void Boolean(org.jruby.ir.operands.Boolean bool) { print(bool.isTrue() ? "t" : "f"); }
     public void BuiltinClass(BuiltinClass builtinClass) { } // FIXME: need to print enum
     public void UnboxedBoolean(UnboxedBoolean bool) { print(bool.isTrue() ? "t" : "f"); }
+    public void ChilledString(ChilledString chilled) { print(chilled.getByteList()); }
     public void ClosureLocalVariable(ClosureLocalVariable closurelocalvariable) { LocalVariable(closurelocalvariable); }
     public void CurrentScope(CurrentScope currentscope) { }
     public void Complex(Complex complex) { visit(complex.getNumber()); }

diff --git a/core/src/main/java/org/jruby/ir/targets/indy/IndyValueCompiler.java b/core/src/main/java/org/jruby/ir/targets/indy/IndyValueCompiler.java
@@ -89,7 +89,7 @@ public void pushString(ByteList bl, int cr) {
 
     public void pushChilledString(ByteList bl, int cr) {
         compiler.loadContext();
-        compiler.adapter.invokedynamic("string", sig(RubyString.class, ThreadContext.class), StringBootstrap.CSTRING_BOOTSTRAP, RubyEncoding.decodeRaw(bl), bl.getEncoding().toString(), cr);
+        compiler.adapter.invokedynamic("chilled", sig(RubyString.class, ThreadContext.class), StringBootstrap.CSTRING_BOOTSTRAP, RubyEncoding.decodeRaw(bl), bl.getEncoding().toString(), cr);
     }
 
     public void pushFrozenString(ByteList bl, int cr, String file, int line) {

diff --git a/core/src/main/java/org/jruby/ir/targets/indy/StringBootstrap.java b/core/src/main/java/org/jruby/ir/targets/indy/StringBootstrap.java
@@ -80,7 +80,7 @@ public class StringBootstrap {
     private static final MethodHandle CSTRING_HANDLE =
             Binder
                     .from(RubyString.class, ThreadContext.class, ByteList.class, int.class)
-                    .invokeStaticQuiet(LOOKUP, StringBootstrap.class, "string");
+                    .invokeStaticQuiet(LOOKUP, StringBootstrap.class, "chilledString");
 
     private static final MethodHandle FSTRING_HANDLE =
             Binder
@@ -136,6 +136,10 @@ public static RubyString string(ThreadContext context, ByteList value, int cr) {
         return RubyString.newStringShared(context.runtime, value, cr);
     }
 
+    public static RubyString chilledString(ThreadContext context, ByteList value, int cr) {
+        return RubyString.newChilledString(context.runtime, value, cr);
+    }
+
     public static RubyString bufferString(ThreadContext context, Encoding encoding, int size, int cr) {
         return RubyString.newString(context.runtime, new ByteList(size, encoding), cr);
     }