[GR-19220] Make interpolated strings frozen (#2304)

PullRequest: truffleruby/2523
oracle · Mar 25, 2021 · 2f6b265 · 2f6b265
2 parents eeb345f + ee46926
commit 2f6b265
Show file tree

Hide file tree

Showing 13 changed files with 97 additions and 50 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ Bug fixes:
 
 Compatibility:
 
+* Make interpolated strings frozen for compatibility with Ruby 2.7 (#2304, @kirs).
 
 Performance:
 

diff --git a/spec/ruby/language/string_spec.rb b/spec/ruby/language/string_spec.rb
@@ -308,4 +308,14 @@ def long_string_literals
       eval(code).should_not.frozen?
     end
   end
+
+  ruby_version_is ""..."3.0" do
+    it "creates a frozen String when # frozen-string-literal: true is used" do
+      code = <<~'RUBY'
+      # frozen-string-literal: true
+      "a#{6*7}c"
+      RUBY
+      eval(code).should.frozen?
+    end
+  end
 end
diff --git a/src/main/java/org/truffleruby/core/string/InterpolatedStringNode.java b/src/main/java/org/truffleruby/core/string/InterpolatedStringNode.java
@@ -28,10 +28,13 @@ public final class InterpolatedStringNode extends RubyContextSourceNode {
 
     private final Rope emptyRope;
 
-    public InterpolatedStringNode(ToSNode[] children, Encoding encoding) {
+    private final boolean frozen;
+
+    public InterpolatedStringNode(ToSNode[] children, Encoding encoding, boolean frozen) {
         assert children.length > 0;
         this.children = children;
         this.emptyRope = RopeOperations.emptyRope(encoding);
+        this.frozen = frozen;
     }
 
     @ExplodeLoop
@@ -47,6 +50,9 @@ public Object execute(VirtualFrame frame) {
             builder = executeStringAppend(builder, toInterpolate);
         }
 
+        if (frozen) {
+            builder.freeze();
+        }
 
         return builder;
     }

diff --git a/src/main/java/org/truffleruby/language/RubyInlineParsingRequestNode.java b/src/main/java/org/truffleruby/language/RubyInlineParsingRequestNode.java
@@ -42,12 +42,12 @@ public RubyInlineParsingRequestNode(
         super(language);
         this.context = context;
 
-        final TranslatorDriver translator = new TranslatorDriver(context);
+        final RubySource rubySource = new RubySource(source, language.getSourcePath(source));
 
         // We use the current frame as the lexical scope to parse, but then we may run with a new frame in the future
-
+        final TranslatorDriver translator = new TranslatorDriver(context, rubySource);
         final RubyRootNode rootNode = translator.parse(
-                new RubySource(source, language.getSourcePath(source)),
+                rubySource,
                 ParserContext.INLINE,
                 null,
                 currentFrame,

diff --git a/src/main/java/org/truffleruby/language/RubyParsingRequestNode.java b/src/main/java/org/truffleruby/language/RubyParsingRequestNode.java
@@ -41,10 +41,11 @@ public RubyParsingRequestNode(RubyLanguage language, RubyContext context, Source
         super(language, null, null);
         this.contextReference = lookupContextReference(RubyLanguage.class);
 
-        final TranslatorDriver translator = new TranslatorDriver(context);
+        final RubySource rubySource = new RubySource(source, language.getSourcePath(source));
 
+        final TranslatorDriver translator = new TranslatorDriver(context, rubySource);
         final RubyRootNode rootNode = translator.parse(
-                new RubySource(source, language.getSourcePath(source)),
+                rubySource,
                 ParserContext.TOP_LEVEL,
                 argumentNames,
                 null,

diff --git a/src/main/java/org/truffleruby/language/loader/CodeLoader.java b/src/main/java/org/truffleruby/language/loader/CodeLoader.java
@@ -45,7 +45,7 @@ public RubyRootNode parse(RubySource source,
             RubyModule wrap,
             boolean ownScopeForAssignments,
             Node currentNode) {
-        final TranslatorDriver translator = new TranslatorDriver(context);
+        final TranslatorDriver translator = new TranslatorDriver(context, source);
         return translator.parse(source, parserContext, null, parentFrame, wrap, ownScopeForAssignments, currentNode);
     }
 

diff --git a/src/main/java/org/truffleruby/parser/BodyTranslator.java b/src/main/java/org/truffleruby/parser/BodyTranslator.java
@@ -519,8 +519,9 @@ public RubyNode visitCallNode(CallParseNode node) {
                     new FrozenStringLiteralNode(frozenString, language.coreStrings.METHOD)));
         }
 
-        if (receiver instanceof ConstParseNode &&
-                ((ConstParseNode) receiver).getName().equals("Primitive") && canUsePrimitives()) {
+        if (environment.getParseEnvironment().canUsePrimitives() &&
+                receiver instanceof ConstParseNode &&
+                ((ConstParseNode) receiver).getName().equals("Primitive")) {
             final RubyNode ret = translateInvokePrimitive(sourceSection, node);
             return addNewlineIfNeeded(node, ret);
         }
@@ -539,10 +540,6 @@ public RubyNode visitCallNode(CallParseNode node) {
         return addNewlineIfNeeded(node, translated);
     }
 
-    private boolean canUsePrimitives() {
-        return inCore() || environment.getParseEnvironment().allowTruffleRubyPrimitives;
-    }
-
     private RubyNode translateInvokePrimitive(SourceIndexLength sourceSection, CallParseNode node) {
         /* Translates something that looks like
          *
@@ -1240,11 +1237,6 @@ private RubyNode getLexicalScopeNode(String kind, SourceIndexLength sourceSectio
         }
     }
 
-    private boolean inCore() {
-        final String path = RubyLanguage.getPath(source);
-        return path.startsWith(environment.getParseEnvironment().getCorePath());
-    }
-
     @Override
     public RubyNode visitConstNode(ConstParseNode node) {
         // Unqualified constant access, as in CONST
@@ -1305,30 +1297,38 @@ public RubyNode visitDRegxNode(DRegexpParseNode node) {
 
     @Override
     public RubyNode visitDStrNode(DStrParseNode node) {
-        final RubyNode ret = translateInterpolatedString(node.getPosition(), node.getEncoding(), node.children());
+        final RubyNode ret = translateInterpolatedString(
+                node.getPosition(),
+                node.getEncoding(),
+                node.children(),
+                node.isFrozen() && !environment.getParseEnvironment().inCore());
         return addNewlineIfNeeded(node, ret);
     }
 
     @Override
     public RubyNode visitDSymbolNode(DSymbolParseNode node) {
         SourceIndexLength sourceSection = node.getPosition();
 
-        final RubyNode stringNode = translateInterpolatedString(sourceSection, node.getEncoding(), node.children());
+        final RubyNode stringNode = translateInterpolatedString(
+                sourceSection,
+                node.getEncoding(),
+                node.children(),
+                false);
 
         final RubyNode ret = StringToSymbolNodeGen.create(stringNode);
         ret.unsafeSetSourceSection(sourceSection);
         return addNewlineIfNeeded(node, ret);
     }
 
     private RubyNode translateInterpolatedString(SourceIndexLength sourceSection,
-            Encoding encoding, ParseNode[] childNodes) {
+            Encoding encoding, ParseNode[] childNodes, boolean frozen) {
         final ToSNode[] children = new ToSNode[childNodes.length];
 
         for (int i = 0; i < childNodes.length; i++) {
             children[i] = ToSNodeGen.create(childNodes[i].accept(this));
         }
 
-        final RubyNode ret = new InterpolatedStringNode(children, encoding);
+        final RubyNode ret = new InterpolatedStringNode(children, encoding, frozen);
         ret.unsafeSetSourceSection(sourceSection);
         return ret;
     }
@@ -1363,7 +1363,7 @@ public RubyNode visitDVarNode(DVarParseNode node) {
 
     @Override
     public RubyNode visitDXStrNode(DXStrParseNode node) {
-        final DStrParseNode string = new DStrParseNode(node.getPosition(), node.getEncoding());
+        final DStrParseNode string = new DStrParseNode(node.getPosition(), node.getEncoding(), false);
         string.addAll(node);
         final ParseNode argsNode = buildArrayNode(node.getPosition(), string);
         final ParseNode callNode = new FCallParseNode(node.getPosition(), "`", argsNode, null);
@@ -2925,7 +2925,7 @@ public RubyNode visitUntilNode(UntilParseNode node) {
     @Override
     public RubyNode visitVCallNode(VCallParseNode node) {
         // TODO (pitr-ch 02-Dec-2019): replace with a primitive
-        if (node.getName().equals("undefined") && inCore()) { // translate undefined
+        if (environment.getParseEnvironment().inCore() && node.getName().equals("undefined")) { // translate undefined
             final RubyNode ret = new ObjectLiteralNode(NotProvided.INSTANCE);
             ret.unsafeSetSourceSection(node.getPosition());
             return addNewlineIfNeeded(node, ret);

diff --git a/src/main/java/org/truffleruby/parser/ParseEnvironment.java b/src/main/java/org/truffleruby/parser/ParseEnvironment.java
@@ -10,6 +10,7 @@
 package org.truffleruby.parser;
 
 import org.truffleruby.RubyContext;
+import com.oracle.truffle.api.source.Source;
 import org.truffleruby.RubyLanguage;
 import org.truffleruby.SuppressFBWarnings;
 import org.truffleruby.language.LexicalScope;
@@ -24,18 +25,24 @@ public class ParseEnvironment {
     private LexicalScope lexicalScope = null;
     private boolean dynamicConstantLookup = false;
     public boolean allowTruffleRubyPrimitives = false;
-    private final String corePath;
+    public final Source source;
+    private final boolean inCore;
     private final boolean coverageEnabled;
     public final Optional<RubyContext> contextIfSingleContext;
 
-    public ParseEnvironment(RubyLanguage language) {
-        corePath = language.corePath;
+    public ParseEnvironment(RubyLanguage language, RubySource rubySource) {
+        this.source = rubySource.getSource();
+        this.inCore = RubyLanguage.getPath(source).startsWith(language.corePath);
         coverageEnabled = language.contextIfSingleContext.map(c -> c.getCoverageManager().isEnabled()).orElse(false);
         contextIfSingleContext = language.contextIfSingleContext;
     }
 
-    public String getCorePath() {
-        return corePath;
+    public boolean inCore() {
+        return inCore;
+    }
+
+    public boolean canUsePrimitives() {
+        return inCore() || allowTruffleRubyPrimitives;
     }
 
     public void resetLexicalScope(LexicalScope lexicalScope) {

diff --git a/src/main/java/org/truffleruby/parser/TranslatorDriver.java b/src/main/java/org/truffleruby/parser/TranslatorDriver.java
@@ -40,6 +40,7 @@
 import java.util.Arrays;
 import java.util.List;
 
+import com.oracle.truffle.api.CompilerDirectives;
 import org.jcodings.specific.UTF8Encoding;
 import org.truffleruby.RubyContext;
 import org.truffleruby.RubyLanguage;
@@ -98,17 +99,22 @@ public class TranslatorDriver {
     private final RubyLanguage language;
     private final ParseEnvironment parseEnvironment;
 
-    public TranslatorDriver(RubyContext context) {
+    public TranslatorDriver(RubyContext context, RubySource rubySource) {
         this.context = context;
         this.language = context.getLanguageSlow();
-        parseEnvironment = new ParseEnvironment(language);
+        this.parseEnvironment = new ParseEnvironment(language, rubySource);
     }
 
     public RubyRootNode parse(RubySource rubySource, ParserContext parserContext, String[] argumentNames,
             MaterializedFrame parentFrame, RubyModule wrap, boolean ownScopeForAssignments, Node currentNode) {
-        assert parserContext
-                .isTopLevel() == (parentFrame == null) : "A frame should be given iff the context is not toplevel: " +
-                        parserContext + " " + parentFrame;
+        if (rubySource.getSource() != parseEnvironment.source) {
+            throw CompilerDirectives.shouldNotReachHere("TranslatorDriver used with a different Source");
+        }
+
+        if (parserContext.isTopLevel() != (parentFrame == null)) {
+            throw CompilerDirectives.shouldNotReachHere(
+                    "A frame should be given iff the context is not toplevel: " + parserContext + " " + parentFrame);
+        }
 
         final Source source = rubySource.getSource();
 

diff --git a/src/main/java/org/truffleruby/parser/ast/DStrParseNode.java b/src/main/java/org/truffleruby/parser/ast/DStrParseNode.java
@@ -39,16 +39,26 @@
 
 /** A string which contains some dynamic elements which needs to be evaluated (introduced by #). */
 public class DStrParseNode extends DParseNode implements ILiteralNode {
+    private boolean frozen;
 
-    public DStrParseNode(SourceIndexLength position, Encoding encoding) {
+    public DStrParseNode(SourceIndexLength position, Encoding encoding, boolean frozen) {
         super(position, encoding);
+        setFrozen(frozen);
     }
 
     @Override
     public NodeType getNodeType() {
         return NodeType.DSTRNODE;
     }
 
+    public boolean isFrozen() {
+        return frozen;
+    }
+
+    public void setFrozen(boolean frozen) {
+        this.frozen = frozen;
+    }
+
     /** Accept for the visitor pattern.
      * 
      * @param iVisitor the visitor **/

diff --git a/src/main/java/org/truffleruby/parser/parser/ParserSupport.java b/src/main/java/org/truffleruby/parser/parser/ParserSupport.java
@@ -1213,8 +1213,8 @@ public void setLexer(RubyLexer lexer) {
         this.lexer = lexer;
     }
 
-    public DStrParseNode createDStrNode(SourceIndexLength position) {
-        return new DStrParseNode(position, lexer.getEncoding());
+    public DStrParseNode createDStrNode(SourceIndexLength position, boolean frozen) {
+        return new DStrParseNode(position, lexer.getEncoding(), frozen);
     }
 
     public ParseNodeTuple createKeyValue(ParseNode key, ParseNode value) {
@@ -1270,12 +1270,12 @@ public ParseNode literal_concat(ParseNode head, ParseNode tail) {
         }
 
         if (head instanceof EvStrParseNode) {
-            head = createDStrNode(head.getPosition()).add(head);
+            head = createDStrNode(head.getPosition(), false).add(head);
         }
 
         if (lexer.getHeredocIndent() > 0) {
             if (head instanceof StrParseNode) {
-                head = createDStrNode(head.getPosition()).add(head);
+                head = createDStrNode(head.getPosition(), false).add(head);
                 return list_append(head, tail);
             } else if (head instanceof DStrParseNode) {
                 return list_append(head, tail);
@@ -1298,7 +1298,10 @@ public ParseNode literal_concat(ParseNode head, ParseNode tail) {
 
         } else if (tail instanceof DStrParseNode) {
             if (head instanceof StrParseNode) { // Str + oDStr -> Dstr(Str, oDStr.contents)
-                DStrParseNode newDStr = new DStrParseNode(head.getPosition(), ((DStrParseNode) tail).getEncoding());
+                DStrParseNode newDStr = new DStrParseNode(
+                        head.getPosition(),
+                        ((DStrParseNode) tail).getEncoding(),
+                        false);
                 newDStr.add(head);
                 newDStr.add(tail);
                 return newDStr;
@@ -1309,12 +1312,15 @@ public ParseNode literal_concat(ParseNode head, ParseNode tail) {
 
         // tail must be EvStrParseNode at this point
         if (head instanceof StrParseNode) {
-
+            StrParseNode front = (StrParseNode) head;
             //Do not add an empty string node
-            if (((StrParseNode) head).getValue().byteLength() == 0) {
-                head = createDStrNode(head.getPosition());
+            if (front.getValue().byteLength() == 0) {
+                head = createDStrNode(
+                        head.getPosition(),
+                        front.isFrozen());
             } else {
-                head = createDStrNode(head.getPosition()).add(head);
+                head = createDStrNode(head.getPosition(), front.isFrozen())
+                        .add(head);
             }
         }
         return ((DStrParseNode) head).add(tail);

diff --git a/src/main/java/org/truffleruby/parser/parser/RubyParser.java b/src/main/java/org/truffleruby/parser/parser/RubyParser.java
diff --git a/src/main/java/org/truffleruby/parser/parser/RubyParser.y b/src/main/java/org/truffleruby/parser/parser/RubyParser.y
@@ -2054,7 +2054,7 @@ literal         : numeric {
                 | dsym
 
 strings         : string {
-                    $$ = $1 instanceof EvStrParseNode ? new DStrParseNode($1.getPosition(), lexer.getEncoding()).add($1) : $1;
+                    $$ = $1 instanceof EvStrParseNode ? new DStrParseNode($1.getPosition(), lexer.getEncoding(), support.getConfiguration().isFrozenStringLiteral()).add($1) : $1;
                     /*
                     NODE *node = $1;
                     if (!node) {
@@ -2114,7 +2114,7 @@ word_list       : /* none */ {
                     $$ = new ArrayParseNode(lexer.getPosition());
                 }
                 | word_list word ' ' {
-                     $$ = $1.add($2 instanceof EvStrParseNode ? new DStrParseNode($1.getPosition(), lexer.getEncoding()).add($2) : $2);
+                     $$ = $1.add($2 instanceof EvStrParseNode ? new DStrParseNode($1.getPosition(), lexer.getEncoding(), false).add($2) : $2);
                 }
 
 word            : string_content {
@@ -2711,7 +2711,7 @@ assoc           : arg_value tASSOC arg_value {
                 }
                 | tSTRING_BEG string_contents tLABEL_END arg_value {
                     if ($2 instanceof StrParseNode) {
-                        DStrParseNode dnode = new DStrParseNode(support.getPosition($2), lexer.getEncoding());
+                        DStrParseNode dnode = new DStrParseNode(support.getPosition($2), lexer.getEncoding(), false);
                         dnode.add($2);
                         $$ = support.createKeyValue(new DSymbolParseNode(support.getPosition($2), dnode), $4);
                     } else if ($2 instanceof DStrParseNode) {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -8,6 +8,7 @@ Bug fixes:

		Compatibility:

		* Make interpolated strings frozen for compatibility with Ruby 2.7 (#2304, @kirs).

		Performance:

Expand Down