Merge pull request #8226 from enebo/chilled

Implement "chilled" Strings
jruby · May 8, 2024 · ac513f5 · ac513f5
2 parents 3728d65 + e825cab
commit ac513f5
Show file tree

Hide file tree

Showing 39 changed files with 330 additions and 109 deletions.
diff --git a/core/src/main/java/org/jruby/ObjectFlags.java b/core/src/main/java/org/jruby/ObjectFlags.java
@@ -24,6 +24,7 @@ public interface ObjectFlags {
     int FSTRING      = registry.newFlag(RubyString.class);
     int CR_7BIT_F    = registry.newFlag(RubyString.class);
     int CR_VALID_F   = registry.newFlag(RubyString.class);
+    int CHILLED_F    = registry.newFlag(RubyString.class);
 
     int MATCH_BUSY = registry.newFlag(RubyMatchData.class);
 

diff --git a/core/src/main/java/org/jruby/RubyBasicObject.java b/core/src/main/java/org/jruby/RubyBasicObject.java
@@ -918,7 +918,11 @@ protected RubyBasicObject cloneSetup(ThreadContext context, RubyBasicObject clon
 
         if (freeze == context.nil) {
             sites(context).initialize_clone.call(context, clone, clone, this);
-            if (isFrozen()) clone.setFrozen(true);
+            if (this instanceof RubyString str && str.isChilled()) {
+                ((RubyString) clone).chill();
+            } else if (isFrozen()) {
+                clone.setFrozen(true);
+            }
         } else { // will always be true or false (MRI has bulletproofing to catch odd values (rb_bug explodes).
             // FIXME: MRI uses C module variables to make a single hash ever for this setup.  We build every time.
             RubyHash opts = RubyHash.newHash(context.runtime, getRuntime().newSymbol("freeze"), freeze);
@@ -1590,8 +1594,12 @@ public void copyInstanceVariablesInto(final InstanceVariables other) {
     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 {
+            raiseFrozenError();
         }
-        raiseFrozenError();
     }
 
     private void raiseFrozenError() throws RaiseException {

diff --git a/core/src/main/java/org/jruby/RubyInstanceConfig.java b/core/src/main/java/org/jruby/RubyInstanceConfig.java
@@ -1452,7 +1452,7 @@ public void setProfilingService( String service )  {
         this.profilingService = service;
     }
 
-    public boolean isFrozenStringLiteral() {
+    public Boolean isFrozenStringLiteral() {
         return frozenStringLiteral;
     }
 
@@ -1570,7 +1570,7 @@ public ClassLoader getCurrentThreadClassLoader() {
     private boolean updateNativeENVEnabled = true;
     private boolean kernelGsubDefined;
     private boolean hasScriptArgv = false;
-    private boolean frozenStringLiteral = false;
+    private Boolean frozenStringLiteral = null;
     private boolean debuggingFrozenStringLiteral = false;
     private final boolean interruptibleRegexps = Options.REGEXP_INTERRUPTIBLE.load();
     private String jrubyHome;

diff --git a/core/src/main/java/org/jruby/RubyString.java b/core/src/main/java/org/jruby/RubyString.java
@@ -87,6 +87,7 @@
 import java.util.Locale;
 import java.util.function.Function;
 
+import static org.jruby.ObjectFlags.CHILLED_F;
 import static org.jruby.RubyComparable.invcmp;
 import static org.jruby.RubyEnumerator.SizeFn;
 import static org.jruby.RubyEnumerator.enumeratorize;
@@ -515,6 +516,10 @@ public static RubyString newString(Ruby runtime, ByteList bytes, int coderange)
         return new RubyString(runtime, runtime.getString(), bytes, coderange);
     }
 
+    public static RubyString newChilledString(Ruby runtime, ByteList bytes, int coderange) {
+        return newString(runtime, bytes, coderange).chill();
+    }
+
     public static RubyString newString(Ruby runtime, ByteList bytes, Encoding encoding) {
         return new RubyString(runtime, runtime.getString(), bytes, encoding);
     }
@@ -953,11 +958,23 @@ private void modifyCheck(byte[] b, int len, Encoding enc) {
     }
 
     protected void frozenCheck() {
-        if (isFrozen()) {
+        if (isChilled()) {
+            mutateChilledString();
+        } else if (isFrozen()) {
             throw getRuntime().newFrozenError("String", this);
         }
     }
 
+    private void mutateChilledString() {
+        getRuntime().getWarnings().warn("literal string will be frozen in the future");
+        setFrozen(false);
+        flags &= ~CHILLED_F;
+    }
+
+    protected boolean isChilled() {
+        return (flags & CHILLED_F) != 0;
+    }
+
     /** rb_str_modify
      *
      */
@@ -5002,6 +5019,17 @@ private static IRubyObject getPatternQuoted(ThreadContext context, IRubyObject p
         return pat; // String
     }
 
+    @Override
+    public RubyClass getSingletonClass() {
+        if (isChilled()) {
+            mutateChilledString();
+        } else if (isFrozen()) {
+            throw typeError(getRuntime().getCurrentContext(), "can't define singleton");
+        }
+
+        return super.getSingletonClass();
+    }
+
     // MRI: get_pat_quoted (scan error checking portion)
     private static Object getScanPatternQuoted(ThreadContext context, IRubyObject pat) {
         pat = getPatternQuoted(context, pat);
@@ -6758,11 +6786,18 @@ public IRubyObject scrub_bang(ThreadContext context, IRubyObject repl, Block blo
 
     @JRubyMethod
     public IRubyObject freeze(ThreadContext context) {
+        if (isChilled()) flags &= ~CHILLED_F;
         if (isFrozen()) return this;
         resize(size());
         return super.freeze(context);
     }
 
+    public RubyString chill() {
+        flags |= CHILLED_F;
+        setFrozen(true);
+        return this;
+    }
+
     /**
      * Mutator for internal string representation.
      *

diff --git a/core/src/main/java/org/jruby/ast/DStrNode.java b/core/src/main/java/org/jruby/ast/DStrNode.java
@@ -36,12 +36,13 @@
 import org.jcodings.Encoding;
 import org.jruby.ast.types.ILiteralNode;
 import org.jruby.ast.visitor.NodeVisitor;
+import org.jruby.ir.builder.StringStyle;
 
 /**
  * A string which contains some dynamic elements which needs to be evaluated (introduced by #).
  */
 public class DStrNode extends DNode implements ILiteralNode {
-    private boolean frozen;
+    private StringStyle stringStyle = StringStyle.Mutable;
 
     public DStrNode(int line, Encoding encoding) {
         super(line, encoding);
@@ -61,11 +62,11 @@ public <T> T accept(NodeVisitor<T> iVisitor) {
         return iVisitor.visitDStrNode(this);
     }
 
-    public boolean isFrozen() {
-        return frozen;
+    public StringStyle getStringStyle() {
+        return stringStyle;
     }
 
-    public void setFrozen(boolean frozen) {
-        this.frozen = frozen;
+    public void setStringStyle(StringStyle frozen) {
+        this.stringStyle = frozen;
     }
 }
diff --git a/core/src/main/java/org/jruby/ast/StrNode.java b/core/src/main/java/org/jruby/ast/StrNode.java
@@ -39,27 +39,31 @@
 import org.jruby.Ruby;
 import org.jruby.ast.types.ILiteralNode;
 import org.jruby.ast.visitor.NodeVisitor;
+import org.jruby.ir.builder.StringStyle;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.jruby.util.ByteList;
 import org.jruby.util.StringSupport;
 
+import static org.jruby.ir.builder.StringStyle.*;
+
 /** 
  * Representing a simple String literal.
  */
 public class StrNode extends Node implements ILiteralNode, LiteralValue, SideEffectFree {
     private final ByteList value;
     private final int codeRange;
-    private boolean frozen;
+    private StringStyle stringStyle;
 
     public StrNode(int line, ByteList value) {
-        this(line, value, StringSupport.codeRangeScan(value.getEncoding(), value));
+        this(line, value, StringSupport.codeRangeScan(value.getEncoding(), value), Frozen);
     }
 
-    public StrNode(int line, ByteList value, int codeRange) {
+    public StrNode(int line, ByteList value, int codeRange, StringStyle stringStyle) {
         super(line, false);
 
         this.value = value;
         this.codeRange = codeRange;
+        this.stringStyle = stringStyle;
     }
 
     public StrNode(int line, StrNode head, StrNode tail) {
@@ -73,7 +77,20 @@ public StrNode(int line, StrNode head, StrNode tail) {
         myValue.append(headBL);
         myValue.append(tailBL);
 
-        frozen = head.isFrozen() && tail.isFrozen();
+        // Convoluted logic and maybe not totally needed.  Frozen loses to anything else.
+        // once we know neither is Frozen than a mismatch means we just use chilled.
+        if (head.stringStyle != tail.stringStyle) {
+            if (head.stringStyle == Frozen) {
+                stringStyle = tail.stringStyle;
+            } else if (tail.stringStyle == Frozen) {
+                stringStyle = head.stringStyle;
+            } else {
+                stringStyle = Chilled;
+            }
+        } else {
+            stringStyle = head.stringStyle;
+        }
+
         value = myValue;
         codeRange = StringSupport.codeRangeScan(value.getEncoding(), value);
     }
@@ -123,12 +140,12 @@ public List<Node> childNodes() {
         return EMPTY_LIST;
     }
 
-    public boolean isFrozen() {
-        return frozen;
+    public StringStyle getStringStyle() {
+        return stringStyle;
     }
 
-    public void setFrozen(boolean frozen) {
-        this.frozen = frozen;
+    public void setStringStyle(StringStyle stringStyle) {
+        this.stringStyle = stringStyle;
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/ir/IRVisitor.java b/core/src/main/java/org/jruby/ir/IRVisitor.java
@@ -171,6 +171,7 @@ private void error(Object object) {
     public void Boolean(Boolean bool) { error(bool); }
     public void BuiltinClass(BuiltinClass builtinClass) { error(builtinClass); }
     public void UnboxedBoolean(UnboxedBoolean bool) { error(bool); }
+    public void ChilledString(ChilledString chilledString) { error(chilledString); }
     public void ClosureLocalVariable(ClosureLocalVariable closurelocalvariable) { error(closurelocalvariable); }
     public void Complex(Complex complex) { error(complex); }
     public void CurrentScope(CurrentScope currentscope) { error(currentscope); }

diff --git a/core/src/main/java/org/jruby/ir/builder/IRBuilder.java b/core/src/main/java/org/jruby/ir/builder/IRBuilder.java
@@ -54,6 +54,8 @@
 import java.util.function.Consumer;
 
 import static org.jruby.ir.IRFlags.*;
+import static org.jruby.ir.builder.StringStyle.Frozen;
+import static org.jruby.ir.builder.StringStyle.Mutable;
 import static org.jruby.ir.instructions.Instr.EMPTY_OPERANDS;
 import static org.jruby.ir.instructions.IntegerMathInstr.Op.ADD;
 import static org.jruby.ir.instructions.IntegerMathInstr.Op.SUBTRACT;
@@ -326,7 +328,7 @@ protected void maybeGenerateIsNotEmptyErrorString(Variable errorString, Operand
         label("empty", (empty) ->
                 cond(empty, result, tru(), ()->
                         addInstr(new BuildCompoundStringInstr(errorString, new Operand[] {value, new FrozenString(" is not empty")},
-                                UTF8Encoding.INSTANCE, 13, true, getFileName(), lastProcessedLineNum))));
+                                UTF8Encoding.INSTANCE, 13, Frozen, getFileName(), lastProcessedLineNum))));
     }
     protected RubySymbol methodNameFor() {
         IRScope method = scope.getNearestMethod();
@@ -1276,6 +1278,8 @@ protected Operand buildDRegex(Variable result, U[] children, RegexpOptions optio
         return result;
     }
 
+    // Pre-Ruby 3.4 path for JRuby 9.4.x.
+    @Deprecated
     protected Operand buildDStr(Variable result, U[] nodePieces, Encoding encoding, boolean isFrozen, int line) {
         if (result == null) result = temp();
 
@@ -1286,7 +1290,22 @@ protected Operand buildDStr(Variable result, U[] nodePieces, Encoding encoding,
             estimatedSize += dynamicPiece(pieces, i, nodePieces[i], null);
         }
 
-        addInstr(new BuildCompoundStringInstr(result, pieces, encoding, estimatedSize, isFrozen, getFileName(), line));
+        addInstr(new BuildCompoundStringInstr(result, pieces, encoding, estimatedSize, isFrozen ? Frozen : Mutable, getFileName(), line));
+
+        return result;
+    }
+
+    protected Operand buildDStr(Variable result, U[] nodePieces, Encoding encoding, StringStyle stringStyle, int line) {
+        if (result == null) result = temp();
+
+        Operand[] pieces = new Operand[nodePieces.length];
+        int estimatedSize = 0;
+
+        for (int i = 0; i < pieces.length; i++) {
+            estimatedSize += dynamicPiece(pieces, i, nodePieces[i], null);
+        }
+
+        addInstr(new BuildCompoundStringInstr(result, pieces, encoding, estimatedSize, stringStyle, getFileName(), line));
 
         return result;
     }

diff --git a/core/src/main/java/org/jruby/ir/builder/IRBuilderAST.java b/core/src/main/java/org/jruby/ir/builder/IRBuilderAST.java
@@ -27,6 +27,7 @@
 import org.jruby.ir.interpreter.InterpreterContext;
 import org.jruby.ir.operands.Array;
 import org.jruby.ir.operands.Bignum;
+import org.jruby.ir.operands.ChilledString;
 import org.jruby.ir.operands.Complex;
 import org.jruby.ir.operands.Filename;
 import org.jruby.ir.operands.Float;
@@ -67,6 +68,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static org.jruby.ir.builder.StringStyle.Frozen;
 import static org.jruby.ir.instructions.RuntimeHelperCall.Methods.*;
 
 import static org.jruby.ir.operands.ScopeModule.*;
@@ -916,7 +918,7 @@ public Operand buildCall(Variable aResult, CallNode callNode, Label lazyLabel, L
                 (arg0 = argsAry.get(0)) instanceof StrNode &&
 
                 // not pre-frozen (which can just go through normal call path)
-                !((StrNode) arg0).isFrozen() &&
+                ((StrNode) arg0).getStringStyle() != Frozen &&
 
                 // obj#[] definitely not refined
                 !scope.maybeUsingRefinements() &&
@@ -1065,7 +1067,7 @@ protected boolean containsVariableAssignment(Node node) {
 
     @Override
     protected Operand frozen_string(Node node) {
-        ((StrNode) node).setFrozen(true);
+        ((StrNode) node).setStringStyle(Frozen);
         return buildStrRaw((StrNode) node);
     }
 
@@ -2134,7 +2136,7 @@ public Operand buildDRegexp(Variable result, DRegexpNode node) {
     }
 
     public Operand buildDStr(Variable result, DStrNode node) {
-        return buildDStr(result, node.children(), node.getEncoding(), node.isFrozen(), node.getLine());
+        return buildDStr(result, node.children(), node.getEncoding(), node.getStringStyle(), node.getLine());
     }
 
     public Operand buildDSymbol(Variable result, DSymbolNode node) {
@@ -2615,9 +2617,11 @@ public Operand buildStrRaw(StrNode strNode) {
 
         int line = strNode.getLine();
 
-        if (strNode.isFrozen()) return new FrozenString(strNode.getValue(), strNode.getCodeRange(), scope.getFile(), line);
-
-        return new MutableString(strNode.getValue(), strNode.getCodeRange(), scope.getFile(), line);
+        switch(strNode.getStringStyle()) {
+            case Frozen -> { return new FrozenString(strNode.getValue(), strNode.getCodeRange(), scope.getFile(), line); }
+            case Mutable -> { return new MutableString(strNode.getValue(), strNode.getCodeRange(), scope.getFile(), line); }
+            default -> { return new ChilledString(strNode.getValue(), strNode.getCodeRange(), scope.getFile(), line); }
+        }
     }
 
     public Operand buildSuper(Variable result, SuperNode node) {

diff --git a/core/src/main/java/org/jruby/ir/builder/StringStyle.java b/core/src/main/java/org/jruby/ir/builder/StringStyle.java
@@ -0,0 +1,7 @@
+package org.jruby.ir.builder;
+
+public enum StringStyle {
+    Frozen, // Explicitly frozen from option or pragma
+    Mutable, // Explicitly mutable from option or pragma
+    Chilled, // New String instances which warn if you mutate but say they are frozen
+}