Preliminary support for debug-frozen-string-literal.

This was done in easiest way possible in that IR just does a put_field onto the string literal with its location into @debug_created_info. MRI also stores into an ivar but it is a numeric slot so it is not user visible. Without actually modifying IR this seemed the closest mechanism. It also has an interesting benefit that you can write Ruby to access it and make your own tooling. I put a FIXME: about whether we should make this user-visible or not. If we decide this is a nice user-visible feature we should make a RFE on redmine. The other slightly oddity of this impl is a dyndispatch to "freeze" in IR since we have no instr for freezing. I think though we could possibly augment put_field to have a boolean which allows us to by-pass freeze and then we could make a frozen literal and still put field on it. One other mechanism I thought about would be if we could make an instr for dataGetStruct() for true non-ruby-visible storage. This seems like it would lead to a lot of new instrs but if we want to manipulate native data maybe we need something in IR for that? All-in-all this is a debugging feature and I do not think we should worry too much about implementation so long as it gives the expected output... New options: --debug={list} --debug-frozen-string-literal.
jruby · Jan 28, 2016 · bfd0b76 · bfd0b76
1 parent ac3849a
commit bfd0b76
Showing 4 changed files with 73 additions and 16 deletions.
diff --git a/core/src/main/java/org/jruby/RubyInstanceConfig.java b/core/src/main/java/org/jruby/RubyInstanceConfig.java
@@ -1462,6 +1462,14 @@ public void setFrozenStringLiteral(boolean frozenStringLiteral) {
         this.frozenStringLiteral = frozenStringLiteral;
     }
 
+    public boolean isDebuggingFrozenStringLiteral() {
+        return debuggingFrozenStringLiteral;
+    }
+
+    public void setDebuggingFrozenStringLiteral(boolean debuggingFrozenStringLiteral) {
+        this.debuggingFrozenStringLiteral = debuggingFrozenStringLiteral;
+    }
+
     public static ClassLoader defaultClassLoader() {
         ClassLoader loader = RubyInstanceConfig.class.getClassLoader();
 
@@ -1563,6 +1571,7 @@ public ClassLoader getCurrentThreadClassLoader() {
     private boolean hasScriptArgv = false;
     private boolean preferIPv4 = Options.PREFER_IPV4.load();
     private boolean frozenStringLiteral = false;
+    private boolean debuggingFrozenStringLiteral = false;
     private String jrubyHome;
 
     /**

diff --git a/core/src/main/java/org/jruby/RubyString.java b/core/src/main/java/org/jruby/RubyString.java
@@ -113,6 +113,7 @@
  */
 @JRubyClass(name="String", include={"Enumerable", "Comparable"})
 public class RubyString extends RubyObject implements EncodingCapable, MarshalEncoding, CodeRangeable {
+    public static final String DEBUG_INFO_FIELD = "@debug_created_info";
 
     private static final ASCIIEncoding ASCII = ASCIIEncoding.INSTANCE;
     private static final UTF8Encoding UTF8 = UTF8Encoding.INSTANCE;
@@ -886,7 +887,21 @@ private void frozenCheck() {
     }
 
     private void frozenCheck(boolean runtimeError) {
-        if (isFrozen()) throw getRuntime().newFrozenError("string", runtimeError);
+        if (isFrozen()) {
+            if (getRuntime().getInstanceConfig().isDebuggingFrozenStringLiteral()) {
+                IRubyObject obj = getInstanceVariable(DEBUG_INFO_FIELD);
+
+                if (obj != null && obj instanceof RubyArray) {
+                    RubyArray info = (RubyArray) obj;
+                    if (info.getLength() == 2) {
+                        throw getRuntime().newRaiseException(getRuntime().getRuntimeError(),
+                                "can't modify frozen String, created at " + info.eltInternal(0) + ":" + info.eltInternal(1));
+                    }
+                }
+            }
+
+            throw getRuntime().newFrozenError("string", runtimeError);
+        }
     }
 
     /** rb_str_modify

diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -4,6 +4,7 @@
 import org.jcodings.specific.USASCIIEncoding;
 import org.jruby.Ruby;
 import org.jruby.RubyInstanceConfig;
+import org.jruby.RubyString;
 import org.jruby.ast.*;
 import org.jruby.ast.types.INameNode;
 import org.jruby.compiler.NotCompilableException;
@@ -3378,11 +3379,29 @@ public Operand buildSplat(SplatNode splatNode) {
     public Operand buildStr(StrNode strNode) {
         if (strNode instanceof FileNode) return new Filename();
 
-        Operand literal = strNode.isFrozen() ?
+        Operand literal = strNode.isFrozen() && !manager.getInstanceConfig().isDebuggingFrozenStringLiteral() ?
                 new FrozenString(strNode.getValue(), strNode.getCodeRange()) :
                 new StringLiteral(strNode.getValue(), strNode.getCodeRange());
 
-        return copyAndReturnValue(literal);
+        Operand result;
+
+        // This logic might be slightly different than MRI in cases.  This is defined as any frozen string literal
+        // we be able to be debugged.  In MRI, they only do this is the iseq mode is set to frozen-string-literal.
+        // So we might actually debug more instances of frozen string but this way seems more logical.
+        if (strNode.isFrozen() && manager.getInstanceConfig().isDebuggingFrozenStringLiteral()) {
+            Variable stringReference = createTemporaryVariable();
+            addInstr(new CopyInstr(stringReference, literal));
+            // FIXME: I think we want to hide this and not make user accessible?  Seems like it could have utility to be user-exposed though.
+            // FIXME: Also we are manually dyn-dispatching to freeze and not calling it internally. Maybe also ok?
+            Operand[] pair = new Operand[] { new FrozenString(strNode.getPosition().getFile()), new Fixnum(strNode.getPosition().getLine()) };
+            addInstr(new PutFieldInstr(stringReference, RubyString.DEBUG_INFO_FIELD, new Array(pair)));
+            result = createTemporaryVariable();
+            addInstr(CallInstr.create(scope, (Variable) result, "freeze", stringReference, NO_ARGS, null));
+        } else {
+            result = copyAndReturnValue(literal);
+        }
+
+        return result;
     }
 
     private Operand buildSuperInstr(Operand block, Operand[] args) {

diff --git a/core/src/main/java/org/jruby/util/cli/ArgumentProcessor.java b/core/src/main/java/org/jruby/util/cli/ArgumentProcessor.java
@@ -426,6 +426,18 @@ private void processArgument() {
                         Options.DEBUG_FULLTRACE.force("true");
                         RubyInstanceConfig.FULL_TRACE_ENABLED = true;
                         config.setCompileMode(RubyInstanceConfig.CompileMode.OFF);
+                        config.setDebuggingFrozenStringLiteral(true);
+                        break FOR;
+                    } else if (argument.startsWith("--debug=")) {
+                        for (String debug : valueListFor(argument, "debug")) {
+                            boolean all = debug.equals("all");
+                            if (debug.equals("frozen-string-literal") || all) {
+                                config.setDebuggingFrozenStringLiteral(true);
+                                continue;
+                            }
+
+                            config.getError().println("warning: unknown argument for --debug: `" + debug + "'");
+                        }
                         break FOR;
                     } else if (argument.equals("--jdb")) {
                         config.setDebug(true);
@@ -491,6 +503,9 @@ private void processArgument() {
                     } else if (VERSION_FLAG.matcher(argument).matches()) {
                         config.getError().println("warning: " + argument + " ignored");
                         break FOR;
+                    } else if (argument.equals("--debug-frozen-string-literal")) {
+                        config.setDebuggingFrozenStringLiteral(true);
+                        break FOR;
                     } else if (argument.equals("--disable-gems")) {
                         config.setDisableGems(true);
                         break FOR;
@@ -500,12 +515,7 @@ private void processArgument() {
                         config.setFrozenStringLiteral(true);
                         break FOR;
                     } else if (argument.startsWith("--disable=")) {
-                        String disablesStr = argument.substring("--disable=".length());
-                        String[] disables = disablesStr.split(",");
-
-                        if (disables.length == 0) errorMissingEquals("disable");
-
-                        for (String disable : disables) {
+                        for (String disable : valueListFor(argument, "disable")) {
                             boolean all = disable.equals("all");
                             if (disable.equals("gems") || all) {
                                 config.setDisableGems(true);
@@ -529,14 +539,9 @@ private void processArgument() {
                         config.setFrozenStringLiteral(true);
                         break FOR;
                     } else if (argument.startsWith("--enable=")) {
-                        String enablesString = argument.substring("--enable=".length());
-                        String[] enables = enablesString.split(",");
-
-                        if (enables.length == 0) errorMissingEquals("enable");
-
-                        for (String enable : enables) {
+                        for (String enable : valueListFor(argument, "enable")) {
                             boolean all = enable.equals("all");
-                            if (enable.equals("frozen-string-literal") || enable.equals("frozen-string-literal") || all) {
+                            if (enable.equals("frozen-string-literal") || enable.equals("frozen_string_literal") || all) {
                                 config.setFrozenStringLiteral(true);
                                 continue;
                             }
@@ -604,6 +609,15 @@ private void processArgument() {
         }
     }
 
+    private String[] valueListFor(String argument, String key) {
+        int length = key.length() + 3; // 3 is from -- and = (e.g. --disable=)
+        String[] values = argument.substring(length).split(",");
+
+        if (values.length == 0) errorMissingEquals(key);
+
+        return values;
+    }
+
     private void disallowedInRubyOpts(String option) {
         if (rubyOpts) {
             throw new MainExitException(1, "jruby: invalid switch in RUBYOPT: " + option + " (RuntimeError)");