Implement negative line numbers for eval

Author: bjfish

CHANGELOG.md

@@ -38,6 +38,7 @@ Compatibility:
 * Pass the final `super` specs (#2104, @chrisseaton).
 * Fix arity for arguments with optional kwargs (#1669, @ssnickolay)
 * Fix arity for `Proc` (#2098, @ssnickolay)
+* Implement negative line numbers for eval (#1482).
 
 Performance:
 

doc/user/compatibility.md

@@ -147,14 +147,6 @@ be a large effort to support every Ruby String operation on native strings.
 Setting the process title (via `$0` or `Process.setproctitle` in Ruby) is done
 as best-effort. It may not work, or the title you try to set may be truncated.
 
-#### Line numbers other than 1 work differently
-
-In an `eval` where a custom line number can be specified, line numbers below 1
-are treated as 1, and line numbers above 1 are implemented by inserting blank
-lines in front of the source before parsing it.
-
-The `erb` standard library has been modified to not use negative line numbers.
-
 #### Polyglot standard IO streams
 
 If you use standard IO streams provided by the Polyglot engine, via the

lib/mri/erb.rb

@@ -938,11 +938,7 @@ def new_toplevel(vars = nil)
   def def_method(mod, methodname, fname='(ERB)')
     src = self.src.sub(/^(?!#|$)/) {"def #{methodname}\n"} << "\nend\n"
     mod.module_eval do
-      if defined?(::TruffleRuby)
-        eval(src, binding, fname)
-      else
-        eval(src, binding, fname, -1)
-      end
+      eval(src, binding, fname, -1)
     end
   end
 

spec/tags/core/basicobject/instance_eval_tags.txt

@@ -1,2 +1 @@
 fails:BasicObject#instance_eval gets constants in the receiver if a string given
-fails:BasicObject#instance_eval evaluates string with given filename and negative linenumber

spec/tags/core/kernel/eval_tags.txt

@@ -3,5 +3,4 @@ fails:Kernel#eval returns from the scope calling #eval when evaluating 'return'
 fails:Kernel#eval unwinds through a Proc-style closure and returns from a lambda-style closure in the closure chain
 slow:Kernel#eval raises a LocalJumpError if there is no lambda-style closure in the chain
 slow:Kernel#eval does not share locals across eval scopes
-fails:Kernel#eval evaluates string with given filename and negative linenumber
 fails(cannot store constant with name in binary encoding):Kernel#eval with a magic encoding comment ignores the magic encoding comment if it is after a frozen_string_literal magic comment

src/main/java/org/truffleruby/RubyContext.java

@@ -13,7 +13,10 @@
 import java.nio.ByteBuffer;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
+import java.util.Collections;
 import java.util.Objects;
+import java.util.Map;
+import java.util.WeakHashMap;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.logging.Level;
 
@@ -120,6 +123,7 @@ public class RubyContext {
     private final PreInitializationManager preInitializationManager;
     private final NativeConfiguration nativeConfiguration;
     private final ValueWrapperManager valueWrapperManager;
+    private final Map<Source, Integer> sourceLineOffsets = Collections.synchronizedMap(new WeakHashMap<>());
 
     @CompilationFinal private SecureRandom random;
     private final Hashing hashing;
@@ -758,6 +762,10 @@ public ValueWrapperManager getValueWrapperManager() {
         return valueWrapperManager;
     }
 
+    public Map<Source, Integer> getSourceLineOffsets() {
+        return sourceLineOffsets;
+    }
+
     private static SecureRandom createRandomInstance() {
         try {
             /* We want to use a non-blocking source because this is what MRI does (via /dev/urandom) and it's been found

src/main/java/org/truffleruby/language/backtrace/BacktraceFormatter.java

@@ -28,6 +28,7 @@
 import org.truffleruby.core.string.StringUtils;
 import org.truffleruby.language.RubyRootNode;
 import org.truffleruby.language.methods.TranslateExceptionNode;
+import org.truffleruby.parser.RubySource;
 
 import java.io.OutputStream;
 import java.io.PrintStream;
@@ -244,7 +245,7 @@ private String formatLineInternal(TruffleStackTraceElement[] stackTrace, int n,
                     builder.append(RubyContext.getPath(reportedSourceSection.getSource()));
                 }
                 builder.append(":");
-                builder.append(reportedSourceSection.getStartLine());
+                builder.append(RubySource.getStartLineAdjusted(context, reportedSourceSection));
             }
             builder.append(":in `");
             builder.append(reportedName);

src/main/java/org/truffleruby/language/eval/CreateEvalSourceNode.java

@@ -9,10 +9,8 @@
  */
 package org.truffleruby.language.eval;
 
-import java.util.Arrays;
 
 import org.jcodings.Encoding;
-import org.truffleruby.RubyLanguage;
 import org.truffleruby.core.encoding.EncodingManager;
 import org.truffleruby.core.rope.CannotConvertBinaryRubyStringToJavaString;
 import org.truffleruby.core.rope.Rope;
@@ -48,7 +46,10 @@ public RubySource createEvalSource(Rope code, String method, String file, int li
 
         final Source source = Source.newBuilder(TruffleRuby.LANGUAGE_ID, sourceString, file).build();
 
-        return new RubySource(source, file, sourceRope, true);
+        final RubySource rubySource = new RubySource(source, file, sourceRope, true, line - 1);
+
+        getContext().getSourceLineOffsets().put(source, line - 1);
+        return rubySource;
     }
 
     private static Rope createEvalRope(Rope source, String method, String file, int line) {
@@ -64,46 +65,7 @@ private static Rope createEvalRope(Rope source, String method, String file, int
             source = RopeOperations.withEncoding(source, encoding[0]);
         }
 
-        // Do padding after magic comment detection
-        return offsetSource(method, source, file, line);
-    }
-
-    private static Rope offsetSource(String method, Rope source, String file, int line) {
-        // TODO CS 23-Apr-18 Truffle doesn't support line numbers starting at anything but 1
-        if (line == 0) {
-            // fine instead of warning because these seem common
-            RubyLanguage.LOGGER.fine(() -> String.format(
-                    "zero line number %s:%d not supported in #%s - will be reported as starting at 1",
-                    file,
-                    line,
-                    method));
-            return source;
-        } else if (line < 1) {
-            RubyLanguage.LOGGER.warning(
-                    String.format(
-                            "negative line number %s:%d not supported in #%s - will be reported as starting at 1",
-                            file,
-                            line,
-                            method));
-            return source;
-        } else if (line > 1) {
-            // fine instead of warning because we can simulate these
-            RubyLanguage.LOGGER.fine(() -> String.format(
-                    "offset line number %s:%d are simulated in #%s by adding blank lines",
-                    file,
-                    line,
-                    method));
-            if (!source.getEncoding().isAsciiCompatible()) {
-                throw new UnsupportedOperationException("Cannot prepend newlines in an ASCII incompatible encoding");
-            }
-            final int n = line - 1;
-            final byte[] bytes = new byte[n + source.byteLength()];
-            Arrays.fill(bytes, 0, n, (byte) '\n');
-            System.arraycopy(source.getBytes(), 0, bytes, n, source.byteLength());
-            return RopeOperations.create(bytes, source.getEncoding(), source.getCodeRange());
-        } else {
-            return source;
-        }
+        return source;
     }
 
 }

src/main/java/org/truffleruby/parser/RubySource.java

@@ -11,6 +11,7 @@
 
 import java.util.Objects;
 
+import com.oracle.truffle.api.source.SourceSection;
 import org.truffleruby.RubyContext;
 import org.truffleruby.core.rope.Rope;
 
@@ -25,6 +26,7 @@ public class RubySource {
     private final String sourcePath;
     private final Rope sourceRope;
     private final boolean isEval;
+    private final int lineOffset;
 
     public RubySource(Source source, String sourcePath) {
         this(source, sourcePath, null, false);
@@ -35,12 +37,17 @@ public RubySource(Source source, String sourcePath, Rope sourceRope) {
     }
 
     public RubySource(Source source, String sourcePath, Rope sourceRope, boolean isEval) {
+        this(source, sourcePath, sourceRope, isEval, 0);
+    }
+
+    public RubySource(Source source, String sourcePath, Rope sourceRope, boolean isEval, int lineOffset) {
         assert RubyContext.getPath(source).equals(sourcePath) : RubyContext.getPath(source) + " vs " + sourcePath;
         this.source = Objects.requireNonNull(source);
         //intern() to improve footprint
         this.sourcePath = Objects.requireNonNull(sourcePath).intern();
         this.sourceRope = sourceRope;
         this.isEval = isEval;
+        this.lineOffset = lineOffset;
     }
 
     public Source getSource() {
@@ -58,4 +65,18 @@ public Rope getRope() {
     public boolean isEval() {
         return isEval;
     }
+
+    public int getLineOffset() {
+        return lineOffset;
+    }
+
+    public static int getStartLineAdjusted(RubyContext context, SourceSection sourceSection) {
+        final Integer lineOffset = context.getSourceLineOffsets().get(sourceSection.getSource());
+        if (lineOffset != null) {
+            return sourceSection.getStartLine() + lineOffset;
+        } else {
+            return sourceSection.getStartLine();
+        }
+    }
+
 }

src/main/java/org/truffleruby/parser/TranslatorDriver.java

@@ -403,7 +403,7 @@ public RootParseNode parseToJRubyAST(RubySource rubySource, StaticScope blockSco
                 default:
                     StringBuilder buffer = new StringBuilder(100);
                     buffer.append(e.getFile()).append(':');
-                    buffer.append(e.getLine()).append(": ");
+                    buffer.append(e.getLine() + rubySource.getLineOffset()).append(": ");
                     buffer.append(e.getMessage());
 
                     if (context != null) {