Don't use ReadCallerFrameNode in BasicObjectNodes.InstanceEvalNode class

Author: andrykonchin

spec/ruby/core/basicobject/instance_eval_spec.rb

@@ -20,12 +20,18 @@
     a.instance_eval('self').equal?(a).should be_true
   end
 
-  it "expects a block with no arguments" do
-    -> { "hola".instance_eval }.should raise_error(ArgumentError)
+  it "raises an ArgumentError when no arguments and no block are given" do
+    -> { "hola".instance_eval }.should raise_error(ArgumentError, "wrong number of arguments (given 0, expected 1..3)")
   end
 
-  it "takes no arguments with a block" do
-    -> { "hola".instance_eval(4, 5) {|a,b| a + b } }.should raise_error(ArgumentError)
+  it "raises an ArgumentError when a block and normal arguments are given" do
+    -> { "hola".instance_eval(4, 5) {|a,b| a + b } }.should raise_error(ArgumentError, "wrong number of arguments (given 2, expected 0)")
+  end
+
+  it "raises an ArgumentError when more than 3 arguments are given" do
+    -> {
+      "hola".instance_eval("1 + 1", "some file", 0, "bogus")
+    }.should raise_error(ArgumentError, "wrong number of arguments (given 4, expected 1..3)")
   end
 
   it "yields the object to the block" do
@@ -185,4 +191,58 @@ def meth(arg); arg; end
 
     x.should == :value
   end
+
+  it "converts string argument with #to_str method" do
+    source_code = Object.new
+    def source_code.to_str() "1" end
+
+    a = BasicObject.new
+    a.instance_eval(source_code).should == 1
+  end
+
+  it "raises ArgumentError if returned value is not String" do
+    source_code = Object.new
+    def source_code.to_str() :symbol end
+
+    a = BasicObject.new
+    -> { a.instance_eval(source_code) }.should raise_error(TypeError, /can't convert Object to String/)
+  end
+
+  it "converts filename argument with #to_str method" do
+    filename = Object.new
+    def filename.to_str() "file.rb" end
+
+    err = begin
+            Object.new.instance_eval("raise", filename)
+          rescue => e
+            e
+          end
+    err.backtrace.first.split(":")[0].should == "file.rb"
+  end
+
+  it "raises ArgumentError if returned value is not String" do
+    filename = Object.new
+    def filename.to_str() :symbol end
+
+    -> { Object.new.instance_eval("raise", filename) }.should raise_error(TypeError, /can't convert Object to String/)
+  end
+
+  it "converts lineno argument with #to_int method" do
+    lineno = Object.new
+    def lineno.to_int() 15 end
+
+    err = begin
+            Object.new.instance_eval("raise", "file.rb", lineno)
+          rescue => e
+            e
+          end
+    err.backtrace.first.split(":")[1].should == "15"
+  end
+
+  it "raises ArgumentError if returned value is not Integer" do
+    lineno = Object.new
+    def lineno.to_int() :symbol end
+
+    -> { Object.new.instance_eval("raise", "file.rb", lineno) }.should raise_error(TypeError, /can't convert Object to Integer/)
+  end
 end

spec/tags/core/basicobject/instance_eval_tags.txt

@@ -1 +1,2 @@
 fails:BasicObject#instance_eval gets constants in the receiver if a string given
+fails:BasicObject#instance_eval raises an ArgumentError when more than 3 arguments are given

src/main/java/org/truffleruby/core/basicobject/BasicObjectNodes.java

@@ -13,6 +13,7 @@
 import com.oracle.truffle.api.interop.InteropLibrary;
 import com.oracle.truffle.api.interop.UnsupportedMessageException;
 import com.oracle.truffle.api.object.Shape;
+import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.strings.AbstractTruffleString;
 import org.truffleruby.Layouts;
 import org.truffleruby.builtins.CoreMethod;
@@ -23,6 +24,8 @@
 import org.truffleruby.core.basicobject.BasicObjectNodesFactory.ReferenceEqualNodeFactory;
 import org.truffleruby.core.cast.BooleanCastNode;
 import org.truffleruby.core.cast.NameToJavaStringNode;
+import org.truffleruby.core.cast.ToIntNode;
+import org.truffleruby.core.cast.ToStrNode;
 import org.truffleruby.core.encoding.RubyEncoding;
 import org.truffleruby.core.exception.ExceptionOperations.ExceptionFormatter;
 import org.truffleruby.core.exception.RubyException;
@@ -44,7 +47,6 @@
 import org.truffleruby.language.RubyNode;
 import org.truffleruby.language.RubySourceNode;
 import org.truffleruby.language.Visibility;
-import org.truffleruby.language.arguments.ReadCallerFrameNode;
 import org.truffleruby.language.arguments.RubyArguments;
 import org.truffleruby.language.control.RaiseException;
 import org.truffleruby.language.dispatch.DispatchNode;
@@ -316,94 +318,66 @@ protected Object initialize(Frame callerFrame, Object self, Object[] rubyArgs, R
         }
     }
 
-    @CoreMethod(names = "instance_eval", needsBlock = true, optional = 3, lowerFixnum = 3)
-    public abstract static class InstanceEvalNode extends CoreMethodArrayArgumentsNode {
-
-        @Specialization(guards = { "strings.isRubyString(string)", "stringsFileName.isRubyString(fileName)" },
-                limit = "1")
-        protected Object instanceEval(
-                VirtualFrame frame, Object receiver, Object string, Object fileName, int line, Nil block,
-                @Cached RubyStringLibrary strings,
-                @Cached RubyStringLibrary stringsFileName,
-                @Cached ToJavaStringNode toJavaStringNode,
-                @Cached ReadCallerFrameNode callerFrameNode,
-                @Cached IndirectCallNode callNode) {
-            final MaterializedFrame callerFrame = callerFrameNode.execute(frame);
+    @GenerateUncached
+    @CoreMethod(names = "instance_eval", needsBlock = true, optional = 3, alwaysInlined = true)
+    public abstract static class InstanceEvalNode extends AlwaysInlinedMethodNode {
+
+        @Specialization(guards = "isBlockProvided(rubyArgs)")
+        protected Object evalWithBlock(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget target,
+                @Cached(allowUncached = true) InstanceExecNode instanceExecNode,
+                @Cached BranchProfile errorProfile) {
+            final int count = RubyArguments.getPositionalArgumentsCount(rubyArgs, false);
+
+            if (count > 0) {
+                errorProfile.enter();
+                throw new RaiseException(getContext(), coreExceptions().argumentError(count, 0, this));
+            }
 
-            return instanceEvalHelper(
-                    callerFrame,
-                    receiver,
-                    strings.getTString(string),
-                    strings.getEncoding(string),
-                    toJavaStringNode.executeToJavaString(fileName),
-                    line,
-                    callNode);
+            final Object block = RubyArguments.getBlock(rubyArgs);
+            return instanceExecNode.executeInstanceExec(callerFrame.materialize(), self, new Object[]{ self },
+                    (RubyProc) block);
         }
 
-        @Specialization(guards = { "strings.isRubyString(string)", "stringsFileName.isRubyString(fileName)" },
-                limit = "1")
-        protected Object instanceEval(
-                VirtualFrame frame, Object receiver, Object string, Object fileName, NotProvided line, Nil block,
+        @Specialization(guards = "!isBlockProvided(rubyArgs)")
+        protected Object evalWithString(Frame callerFrame, Object self, Object[] rubyArgs, RootCallTarget target,
+                @Cached BranchProfile errorProfile,
                 @Cached RubyStringLibrary strings,
-                @Cached RubyStringLibrary stringsFileName,
                 @Cached ToJavaStringNode toJavaStringNode,
-                @Cached ReadCallerFrameNode callerFrameNode,
+                @Cached ToStrNode toStrNode,
+                @Cached ToIntNode toIntNode,
                 @Cached IndirectCallNode callNode) {
-            final MaterializedFrame callerFrame = callerFrameNode.execute(frame);
+            final Object sourceCode;
+            String fileName = coreStrings().EVAL_FILENAME_STRING.toString();
+            int line = 1;
+            int count = RubyArguments.getPositionalArgumentsCount(rubyArgs, false);
+
+            if (count == 0) {
+                errorProfile.enter();
+                throw new RaiseException(getContext(), coreExceptions().argumentError(0, 1, 2, this));
+            }
 
-            return instanceEvalHelper(
-                    callerFrame,
-                    receiver,
-                    strings.getTString(string),
-                    strings.getEncoding(string),
-                    toJavaStringNode.executeToJavaString(fileName),
-                    1,
-                    callNode);
-        }
+            sourceCode = toStrNode.execute(RubyArguments.getArgument(rubyArgs, 0));
 
-        @Specialization(guards = "strings.isRubyString(string)", limit = "1")
-        protected Object instanceEval(
-                VirtualFrame frame, Object receiver, Object string, NotProvided fileName, NotProvided line, Nil block,
-                @Cached RubyStringLibrary strings,
-                @Cached ReadCallerFrameNode callerFrameNode,
-                @Cached IndirectCallNode callNode) {
-            final MaterializedFrame callerFrame = callerFrameNode.execute(frame);
+            if (count >= 2) {
+                fileName = toJavaStringNode
+                        .executeToJavaString(toStrNode.execute(RubyArguments.getArgument(rubyArgs, 1)));
+            }
+
+            if (count >= 3) {
+                line = toIntNode.execute(RubyArguments.getArgument(rubyArgs, 2));
+            }
 
+            needCallerFrame(callerFrame, target);
             return instanceEvalHelper(
-                    callerFrame,
-                    receiver,
-                    strings.getTString(string),
-                    strings.getEncoding(string),
-                    coreStrings().EVAL_FILENAME_STRING.toString(),
-                    1,
+                    callerFrame.materialize(),
+                    self,
+                    strings.getTString(sourceCode),
+                    strings.getEncoding(sourceCode),
+                    fileName,
+                    line,
                     callNode);
         }
 
-        @Specialization
-        protected Object instanceEval(
-                VirtualFrame frame,
-                Object receiver,
-                NotProvided string,
-                NotProvided fileName,
-                NotProvided line,
-                RubyProc block,
-                @Cached InstanceExecNode instanceExecNode) {
-            return instanceExecNode.executeInstanceExec(frame, receiver, new Object[]{ receiver }, block);
-        }
-
-        @Specialization
-        protected Object noArgsNoBlock(
-                Object receiver, NotProvided string, NotProvided fileName, NotProvided line, Nil block) {
-            throw new RaiseException(getContext(), coreExceptions().argumentError(0, 1, 2, this));
-        }
-
-        @Specialization(guards = "wasProvided(string)")
-        protected Object argsAndBlock(
-                Object receiver, Object string, Object maybeFileName, Object maybeLine, RubyProc block) {
-            final int passed = RubyGuards.wasProvided(maybeLine) ? 3 : RubyGuards.wasProvided(maybeFileName) ? 2 : 1;
-            throw new RaiseException(getContext(), coreExceptions().argumentError(passed, 0, this));
-        }
-
         @TruffleBoundary
         private Object instanceEvalHelper(MaterializedFrame callerFrame, Object receiver, AbstractTruffleString code,
                 RubyEncoding encoding,

src/main/java/org/truffleruby/core/inlined/AlwaysInlinedMethodNode.java

@@ -16,6 +16,7 @@
 import com.oracle.truffle.api.frame.Frame;
 import org.truffleruby.language.CallStackManager;
 import org.truffleruby.language.RubyBaseNode;
+import org.truffleruby.language.arguments.RubyArguments;
 import org.truffleruby.language.control.RaiseException;
 
 /** A core method that should always be executed inline, without going through a CallTarget. That enables accessing the
@@ -66,4 +67,8 @@ private RaiseException buildException(String method) {
                 getNode()));
     }
 
+    public static boolean isBlockProvided(Object[] rubyArgs) {
+        return RubyArguments.getBlock(rubyArgs) != nil;
+    }
+
 }