Don't use ReadCallerFrameNode in ModuleNodes.ClassEvalNode class

Author: andrykonchin

spec/ruby/core/module/shared/class_eval.rb

@@ -55,40 +55,49 @@ def foo
   it "converts a non-string filename to a string using to_str" do
     (file = mock(__FILE__)).should_receive(:to_str).and_return(__FILE__)
     ModuleSpecs.send(@method, "1+1", file)
+
+    (file = mock(__FILE__)).should_receive(:to_str).and_return(__FILE__)
+    ModuleSpecs.send(@method, "1+1", file, 15)
   end
 
   it "raises a TypeError when the given filename can't be converted to string using to_str" do
     (file = mock('123')).should_receive(:to_str).and_return(123)
-    -> { ModuleSpecs.send(@method, "1+1", file) }.should raise_error(TypeError)
+    -> { ModuleSpecs.send(@method, "1+1", file) }.should raise_error(TypeError, /can't convert MockObject to String/)
   end
 
   it "converts non string eval-string to string using to_str" do
     (o = mock('1 + 1')).should_receive(:to_str).and_return("1 + 1")
     ModuleSpecs.send(@method, o).should == 2
+
+    (o = mock('1 + 1')).should_receive(:to_str).and_return("1 + 1")
+    ModuleSpecs.send(@method, o, "file.rb").should == 2
+
+    (o = mock('1 + 1')).should_receive(:to_str).and_return("1 + 1")
+    ModuleSpecs.send(@method, o, "file.rb", 15).should == 2
   end
 
   it "raises a TypeError when the given eval-string can't be converted to string using to_str" do
     o = mock('x')
-    -> { ModuleSpecs.send(@method, o) }.should raise_error(TypeError)
+    -> { ModuleSpecs.send(@method, o) }.should raise_error(TypeError, "no implicit conversion of MockObject into String")
 
     (o = mock('123')).should_receive(:to_str).and_return(123)
-    -> { ModuleSpecs.send(@method, o) }.should raise_error(TypeError)
+    -> { ModuleSpecs.send(@method, o) }.should raise_error(TypeError, /can't convert MockObject to String/)
   end
 
   it "raises an ArgumentError when no arguments and no block are given" do
-    -> { ModuleSpecs.send(@method) }.should raise_error(ArgumentError)
+    -> { ModuleSpecs.send(@method) }.should raise_error(ArgumentError, "wrong number of arguments (given 0, expected 1..3)")
   end
 
   it "raises an ArgumentError when more than 3 arguments are given" do
     -> {
       ModuleSpecs.send(@method, "1 + 1", "some file", 0, "bogus")
-    }.should raise_error(ArgumentError)
+    }.should raise_error(ArgumentError, "wrong number of arguments (given 4, expected 1..3)")
   end
 
   it "raises an ArgumentError when a block and normal arguments are given" do
     -> {
       ModuleSpecs.send(@method, "1 + 1") { 1 + 1 }
-    }.should raise_error(ArgumentError)
+    }.should raise_error(ArgumentError, "wrong number of arguments (given 1, expected 0)")
   end
 
   # This case was found because Rubinius was caching the compiled

spec/tags/core/module/class_eval_tags.txt

@@ -1,2 +1,3 @@
 fails:Module#class_eval activates refinements from the eval scope
 fails:Module#class_eval activates refinements from the eval scope with block
+fails:Module#class_eval raises an ArgumentError when more than 3 arguments are given

spec/tags/core/module/module_eval_tags.txt

@@ -1,2 +1,3 @@
 fails:Module#module_eval activates refinements from the eval scope
 fails:Module#module_eval activates refinements from the eval scope with block
+fails:Module#module_eval raises an ArgumentError when more than 3 arguments are given

src/main/java/org/truffleruby/core/module/ModuleNodes.java

@@ -46,6 +46,7 @@
 import org.truffleruby.core.array.RubyArray;
 import org.truffleruby.core.cast.BooleanCastWithDefaultNode;
 import org.truffleruby.core.cast.NameToJavaStringNode;
+import org.truffleruby.core.cast.ToIntNode;
 import org.truffleruby.core.cast.ToPathNodeGen;
 import org.truffleruby.core.cast.ToStrNode;
 import org.truffleruby.core.cast.ToStringOrSymbolNodeGen;
@@ -662,89 +663,78 @@ protected Object isAutoload(RubyModule module, String name, boolean inherit) {
         }
     }
 
-    @CoreMethod(names = { "class_eval", "module_eval" }, optional = 3, lowerFixnum = 3, needsBlock = true)
-    public abstract static class ClassEvalNode extends CoreMethodArrayArgumentsNode {
+    @GenerateUncached
+    @CoreMethod(names = { "class_eval", "module_eval" }, optional = 3, needsBlock = true, alwaysInlined = true)
+    public abstract static class ClassEvalNode extends AlwaysInlinedMethodNode {
 
-        @Child private ReadCallerFrameNode readCallerFrameNode = ReadCallerFrameNode.create();
+        @Specialization(guards = "isBlockProvided(rubyArgs)")
+        protected Object evalWithBlock(Frame callerFrame, RubyModule self, Object[] rubyArgs, RootCallTarget target,
+                @Cached BranchProfile errorProfile,
+                @Cached(allowUncached = true) ClassExecNode classExecNode) {
+            final int count = RubyArguments.getPositionalArgumentsCount(rubyArgs, false);
 
-        @Specialization(guards = "libCode.isRubyString(code)", limit = "1")
-        protected Object classEval(
-                VirtualFrame frame, RubyModule module, Object code, NotProvided file, NotProvided line, Nil block,
-                @Cached IndirectCallNode callNode,
-                @Cached RubyStringLibrary libCode) {
-            return classEvalSource(frame, module, code, "(eval)", callNode);
-        }
+            if (count > 0) {
+                errorProfile.enter();
+                throw new RaiseException(getContext(), coreExceptions().argumentError(count, 0, this));
+            }
 
-        @Specialization(guards = { "libCode.isRubyString(code)", "libFile.isRubyString(file)" }, limit = "1")
-        protected Object classEval(
-                VirtualFrame frame, RubyModule module, Object code, Object file, NotProvided line, Nil block,
-                @Cached IndirectCallNode callNode,
-                @Cached RubyStringLibrary libCode,
-                @Cached RubyStringLibrary libFile,
-                @Cached ToJavaStringNode toJavaStringNode) {
-            return classEvalSource(frame, module, code, toJavaStringNode.executeToJavaString(file), callNode);
+            final Object block = RubyArguments.getBlock(rubyArgs);
+            return classExecNode.classExec(EmptyArgumentsDescriptor.INSTANCE, self, new Object[]{ self },
+                    (RubyProc) block);
         }
 
-        @Specialization(guards = { "libCode.isRubyString(code)", "wasProvided(file)" }, limit = "1")
-        protected Object classEval(VirtualFrame frame, RubyModule module, Object code, Object file, int line, Nil block,
-                @Cached IndirectCallNode callNode,
-                @Cached RubyStringLibrary libCode,
-                @Cached RubyStringLibrary libFile,
-                @Cached ToJavaStringNode toJavaStringNode) {
-            final CodeLoader.DeferredCall deferredCall = classEvalSource(
-                    frame,
-                    module,
-                    code,
-                    toJavaStringNode.executeToJavaString(file),
-                    line);
-            return deferredCall.call(callNode);
-        }
+        @Specialization(guards = "!isBlockProvided(rubyArgs)")
+        protected Object evalWithString(Frame callerFrame, RubyModule self, Object[] rubyArgs, RootCallTarget target,
+                @Cached BranchProfile errorProfile,
+                @Cached ToJavaStringNode toJavaStringNode,
+                @Cached ToStrNode toStrNode,
+                @Cached ToIntNode toIntNode,
+                @Cached IndirectCallNode callNode) {
+            final Object sourceCode;
+            String fileName = coreStrings().EVAL_FILENAME_STRING.toString();
+            int line = 1;
 
-        @Specialization(guards = "wasProvided(code)")
-        protected Object classEval(
-                VirtualFrame frame, RubyModule module, Object code, NotProvided file, NotProvided line, Nil block,
-                @Cached IndirectCallNode callNode,
-                @Cached ToStrNode toStrNode) {
-            return classEvalSource(frame, module, toStrNode.execute(code), "(eval)", callNode);
-        }
+            int count = RubyArguments.getPositionalArgumentsCount(rubyArgs, false);
 
-        @Specialization(guards = { "libCode.isRubyString(code)", "wasProvided(file)" }, limit = "1")
-        protected Object classEval(
-                VirtualFrame frame, RubyModule module, Object code, Object file, NotProvided line, Nil block,
-                @Cached RubyStringLibrary stringLibrary,
-                @Cached ToJavaStringNode toJavaStringNode,
-                @Cached IndirectCallNode callNode,
-                @Cached RubyStringLibrary libCode,
-                @Cached ToStrNode toStrNode) {
-            final String javaString = toJavaStringNode.executeToJavaString(toStrNode.execute(file));
-            return classEvalSource(frame, module, code, javaString, callNode);
-        }
+            if (count == 0) {
+                errorProfile.enter();
+                throw new RaiseException(getContext(), coreExceptions().argumentError(0, 1, 2, this));
+            }
 
-        private Object classEvalSource(VirtualFrame frame, RubyModule module, Object code, String file,
-                @Cached IndirectCallNode callNode) {
-            final CodeLoader.DeferredCall deferredCall = classEvalSource(frame, module, code, file, 1);
-            return deferredCall.call(callNode);
-        }
+            sourceCode = toStrNode.execute(RubyArguments.getArgument(rubyArgs, 0));
 
-        private CodeLoader.DeferredCall classEvalSource(VirtualFrame frame, RubyModule module,
-                Object rubySource, String file, int line) {
+            if (count >= 2) {
+                fileName = toJavaStringNode
+                        .executeToJavaString(toStrNode.execute(RubyArguments.getArgument(rubyArgs, 1)));
+            }
 
-            final MaterializedFrame callerFrame = readCallerFrameNode.execute(frame);
+            if (count >= 3) {
+                line = toIntNode.execute(RubyArguments.getArgument(rubyArgs, 2));
+            }
 
-            return classEvalSourceInternal(module, rubySource, file, line, callerFrame);
+            needCallerFrame(callerFrame, target);
+            return classEvalSource(
+                    callerFrame.materialize(),
+                    self,
+                    sourceCode,
+                    fileName,
+                    line,
+                    callNode);
         }
 
         @TruffleBoundary
-        private CodeLoader.DeferredCall classEvalSourceInternal(RubyModule module, Object rubySource,
-                String file, int line, MaterializedFrame callerFrame) {
+        private Object classEvalSource(MaterializedFrame callerFrame, RubyModule module, Object sourceCode, String file,
+                int line,
+                IndirectCallNode callNode) {
             final RubySource source = EvalLoader.createEvalSource(
                     getContext(),
-                    RubyStringLibrary.getUncached().getTString(rubySource),
-                    RubyStringLibrary.getUncached().getEncoding(rubySource),
+                    RubyStringLibrary.getUncached().getTString(sourceCode),
+                    RubyStringLibrary.getUncached().getEncoding(sourceCode),
                     "class/module_eval",
                     file,
                     line,
                     this);
+
             final LexicalScope lexicalScope = new LexicalScope(
                     RubyArguments.getMethod(callerFrame).getLexicalScope(),
                     module);
@@ -756,7 +746,7 @@ private CodeLoader.DeferredCall classEvalSourceInternal(RubyModule module, Objec
                     lexicalScope,
                     this);
 
-            return getContext().getCodeLoader().prepareExecute(
+            final CodeLoader.DeferredCall deferredCall = getContext().getCodeLoader().prepareExecute(
                     callTarget,
                     ParserContext.MODULE,
                     new DeclarationContext(
@@ -766,23 +756,8 @@ private CodeLoader.DeferredCall classEvalSourceInternal(RubyModule module, Objec
                     callerFrame,
                     module,
                     lexicalScope);
-        }
-
-        @Specialization
-        protected Object classEval(
-                RubyModule self, NotProvided code, NotProvided file, NotProvided line, RubyProc block,
-                @Cached ClassExecNode classExecNode) {
-            return classExecNode.classExec(EmptyArgumentsDescriptor.INSTANCE, self, new Object[]{ self }, block);
-        }
 
-        @Specialization
-        protected Object classEval(RubyModule self, NotProvided code, NotProvided file, NotProvided line, Nil block) {
-            throw new RaiseException(getContext(), coreExceptions().argumentError(0, 1, 2, this));
-        }
-
-        @Specialization(guards = "wasProvided(code)")
-        protected Object classEval(RubyModule self, Object code, NotProvided file, NotProvided line, RubyProc block) {
-            throw new RaiseException(getContext(), coreExceptions().argumentError(1, 0, this));
+            return deferredCall.call(callNode);
         }
 
     }