Make StrNode have a frozen bit on it. This pushes some burden more to

generating frozen bit during parse instead of during build but it will make it trivial to implement the Ruby 2.3 frozen string literal pragma.
jruby · Dec 14, 2015 · eca6860 · eca6860
1 parent 0d35995
commit eca6860
Showing 5 changed files with 35 additions and 24 deletions.
diff --git a/core/src/main/java/org/jruby/ast/StrNode.java b/core/src/main/java/org/jruby/ast/StrNode.java
@@ -45,6 +45,7 @@
 public class StrNode extends Node implements ILiteralNode, SideEffectFree {
     private final ByteList value;
     private final int codeRange;
+    private boolean frozen;
 
     public StrNode(ISourcePosition position, ByteList value) {
         this(position, value, StringSupport.codeRangeScan(value.getEncoding(), value));
@@ -103,4 +104,12 @@ public int getCodeRange() {
     public List<Node> childNodes() {
         return EMPTY_LIST;
     }
+
+    public boolean isFrozen() {
+        return frozen;
+    }
+
+    public void setFrozen(boolean frozen) {
+        this.frozen = frozen;
+    }
 }
diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -2532,14 +2532,7 @@ public Operand buildHash(HashNode hashNode) {
                 addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, splat}));
                 continue;
             } else {
-                // TODO: This isn't super pretty. If AST were aware of literal hash string keys being "special"
-                // it could have an appropriate AST node for frozen string and this code would just go away.
-                if (key instanceof StrNode) {
-                    StrNode strKey = (StrNode)key;
-                    keyOperand = new FrozenString(strKey.getValue(), strKey.getCodeRange());
-                } else {
-                    keyOperand = buildWithOrder(key, hasAssignments);
-                }
+                keyOperand = buildWithOrder(key, hasAssignments);
             }
 
             args.add(new KeyValuePair<>(keyOperand, buildWithOrder(pair.getValue(), hasAssignments)));
@@ -3315,10 +3308,13 @@ public Operand buildSplat(SplatNode splatNode) {
     }
 
     public Operand buildStr(StrNode strNode) {
-        if (strNode instanceof FileNode) {
-            return new Filename();
-        }
-        return copyAndReturnValue(new StringLiteral(strNode.getValue(), strNode.getCodeRange()));
+        if (strNode instanceof FileNode) return new Filename();
+
+        Operand literal = strNode.isFrozen() ?
+                new FrozenString(strNode.getValue(), strNode.getCodeRange()) :
+                new StringLiteral(strNode.getValue(), strNode.getCodeRange());
+
+        return copyAndReturnValue(literal);
     }
 
     private Operand buildSuperInstr(Operand block, Operand[] args) {

diff --git a/core/src/main/java/org/jruby/parser/ParserSupport.java b/core/src/main/java/org/jruby/parser/ParserSupport.java
@@ -56,6 +56,7 @@
 import org.jruby.runtime.DynamicScope;
 import org.jruby.runtime.Signature;
 import org.jruby.util.ByteList;
+import org.jruby.util.KeyValuePair;
 import org.jruby.util.RegexpOptions;
 import org.jruby.util.StringSupport;
 import org.jruby.util.cli.Options;
@@ -881,6 +882,12 @@ public DStrNode createDStrNode(ISourcePosition position) {
         return new DStrNode(position, lexer.getEncoding());
     }
 
+    public KeyValuePair<Node, Node> createKeyValue(Node key, Node value) {
+        if (key != null && key instanceof StrNode) ((StrNode) key).setFrozen(true);
+
+        return new KeyValuePair<>(key, value);
+    }
+
     public Node asSymbol(ISourcePosition position, String value) {
         return new SymbolNode(position, value, lexer.getEncoding(), lexer.getTokenCR());
     }

diff --git a/core/src/main/java/org/jruby/parser/RubyParser.java b/core/src/main/java/org/jruby/parser/RubyParser.java
@@ -5236,14 +5236,14 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
 };
 states[610] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
-                    yyVal = new KeyValuePair<Node,Node>(((Node)yyVals[-2+yyTop]), ((Node)yyVals[0+yyTop]));
+                    yyVal = support.createKeyValue(((Node)yyVals[-2+yyTop]), ((Node)yyVals[0+yyTop]));
     return yyVal;
   }
 };
 states[611] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
                     Node label = support.asSymbol(support.getPosition(((Node)yyVals[0+yyTop])), ((String)yyVals[-1+yyTop]));
-                    yyVal = new KeyValuePair<Node,Node>(label, ((Node)yyVals[0+yyTop]));
+                    yyVal = support.createKeyValue(label, ((Node)yyVals[0+yyTop]));
     return yyVal;
   }
 };
@@ -5252,9 +5252,9 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
                     if (((Node)yyVals[-2+yyTop]) instanceof StrNode) {
                         DStrNode dnode = new DStrNode(support.getPosition(((Node)yyVals[-2+yyTop])), lexer.getEncoding());
                         dnode.add(((Node)yyVals[-2+yyTop]));
-                        yyVal = new KeyValuePair<Node,Node>(new DSymbolNode(support.getPosition(((Node)yyVals[-2+yyTop])), dnode), ((Node)yyVals[0+yyTop]));
+                        yyVal = support.createKeyValue(new DSymbolNode(support.getPosition(((Node)yyVals[-2+yyTop])), dnode), ((Node)yyVals[0+yyTop]));
                     } else if (((Node)yyVals[-2+yyTop]) instanceof DStrNode) {
-                        yyVal = new KeyValuePair<Node,Node>(new DSymbolNode(support.getPosition(((Node)yyVals[-2+yyTop])), ((DStrNode)yyVals[-2+yyTop])), ((Node)yyVals[0+yyTop]));
+                        yyVal = support.createKeyValue(new DSymbolNode(support.getPosition(((Node)yyVals[-2+yyTop])), ((DStrNode)yyVals[-2+yyTop])), ((Node)yyVals[0+yyTop]));
                     } else {
                         support.compile_error("Uknown type for assoc in strings: " + ((Node)yyVals[-2+yyTop]));
                     }
@@ -5264,7 +5264,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
 };
 states[613] = new ParserState() {
   @Override public Object execute(ParserSupport support, RubyLexer lexer, Object yyVal, Object[] yyVals, int yyTop) {
-                    yyVal = new KeyValuePair<Node,Node>(null, ((Node)yyVals[0+yyTop]));
+                    yyVal = support.createKeyValue(null, ((Node)yyVals[0+yyTop]));
     return yyVal;
   }
 };
@@ -5293,7 +5293,7 @@ public Object yyparse (RubyLexer yyLex) throws java.io.IOException {
   }
 };
 }
-					// line 2525 "RubyParser.y"
+					// line 2524 "RubyParser.y"
 
     /** The parse method use an lexer stream and parse it to an AST node 
      * structure

diff --git a/core/src/main/java/org/jruby/parser/RubyParser.y b/core/src/main/java/org/jruby/parser/RubyParser.y
@@ -2470,27 +2470,26 @@ assocs          : assoc {
 
 // Cons: [!null]
 assoc           : arg_value tASSOC arg_value {
-                    $$ = new KeyValuePair<Node,Node>($1, $3);
+                    $$ = support.createKeyValue($1, $3);
                 }
                 | tLABEL arg_value {
                     Node label = support.asSymbol(support.getPosition($2), $1);
-                    $$ = new KeyValuePair<Node,Node>(label, $2);
+                    $$ = support.createKeyValue(label, $2);
                 }
                 | tSTRING_BEG string_contents tLABEL_END arg_value {
                     if ($2 instanceof StrNode) {
                         DStrNode dnode = new DStrNode(support.getPosition($2), lexer.getEncoding());
                         dnode.add($2);
-                        $$ = new KeyValuePair<Node,Node>(new DSymbolNode(support.getPosition($2), dnode), $4);
+                        $$ = support.createKeyValue(new DSymbolNode(support.getPosition($2), dnode), $4);
                     } else if ($2 instanceof DStrNode) {
-                        $$ = new KeyValuePair<Node,Node>(new DSymbolNode(support.getPosition($2), $<DStrNode>2), $4);
+                        $$ = support.createKeyValue(new DSymbolNode(support.getPosition($2), $<DStrNode>2), $4);
                     } else {
                         support.compile_error("Uknown type for assoc in strings: " + $2);
                     }
 
                 }
-
                 | tDSTAR arg_value {
-                    $$ = new KeyValuePair<Node,Node>(null, $2);
+                    $$ = support.createKeyValue(null, $2);
                 }
 
 operation       : tIDENTIFIER | tCONSTANT | tFID