Skip to content

Commit

Permalink
Showing 4 changed files with 30 additions and 24 deletions.
2 changes: 0 additions & 2 deletions spec/truffle/tags/core/kernel/eval_tags.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
fails:Kernel#eval evaluates within the scope of the eval
fails:Kernel#eval evaluates such that consts are scoped to the class of the eval
fails:Kernel#eval allows a binding to be captured inside an eval
fails:Kernel#eval uses the same scope for local variables when given the same binding
fails:Kernel#eval sets constants at the toplevel from inside a block
fails:Kernel#eval uses the filename of the binding if none is provided
fails:Kernel#eval uses the receiver as self inside the eval
34 changes: 19 additions & 15 deletions truffle/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java
Original file line number Diff line number Diff line change
@@ -540,7 +540,7 @@ protected RubyBinding getCallerBinding(VirtualFrame frame) {
public Object eval(VirtualFrame frame, RubyString source, UndefinedPlaceholder binding, UndefinedPlaceholder filename, UndefinedPlaceholder lineNumber) {
notDesignedForCompilation();

return eval(source, getCallerBinding(frame), filename, lineNumber);
return getContext().eval(source.getBytes(), getCallerBinding(frame), true, this);
}

@Specialization
@@ -555,36 +555,49 @@ public Object eval(VirtualFrame frame, RubyString source, RubyNilClass noBinding
public Object eval(RubyString source, RubyBinding binding, UndefinedPlaceholder filename, UndefinedPlaceholder lineNumber) {
notDesignedForCompilation();

return getContext().eval(source.getBytes(), binding, this);
return getContext().eval(source.getBytes(), binding, false, this);
}

@Specialization
public Object eval(RubyString source, RubyBinding binding, RubyString filename, UndefinedPlaceholder lineNumber) {
notDesignedForCompilation();

// TODO (nirvdrum Dec. 29, 2014) Do something with the supplied filename.
return getContext().eval(source.getBytes(), binding, this);
return getContext().eval(source.getBytes(), binding, false, this);
}

@Specialization
public Object eval(RubyString source, RubyBinding binding, RubyString filename, int lineNumber) {
notDesignedForCompilation();

// TODO (nirvdrum Dec. 29, 2014) Do something with the supplied filename and lineNumber.
return getContext().eval(source.getBytes(), binding, this);
return getContext().eval(source.getBytes(), binding, false, this);
}

@Specialization(guards = "!isRubyString(arguments[0])")
public Object eval(VirtualFrame frame, RubyBasicObject object, UndefinedPlaceholder binding, UndefinedPlaceholder filename, UndefinedPlaceholder lineNumber) {
notDesignedForCompilation();

return eval(frame, object, getCallerBinding(frame), filename, lineNumber);
return evalCoerced(frame, object, getCallerBinding(frame), true, filename, lineNumber);
}

@Specialization(guards = "!isRubyString(arguments[0])")
public Object eval(VirtualFrame frame, RubyBasicObject object, RubyBinding binding, UndefinedPlaceholder filename, UndefinedPlaceholder lineNumber) {
notDesignedForCompilation();

return evalCoerced(frame, object, binding, false, filename, lineNumber);
}

@Specialization(guards = "!isRubyBinding(arguments[1])")
public Object eval(RubyBasicObject source, RubyBasicObject badBinding, UndefinedPlaceholder filename, UndefinedPlaceholder lineNumber) {
throw new RaiseException(
getContext().getCoreLibrary().typeError(
String.format("wrong argument type %s (expected binding)",
badBinding.getLogicalClass().getName()),
this));
}

private Object evalCoerced(VirtualFrame frame, RubyBasicObject object, RubyBinding binding, boolean ownScopeForAssignments, UndefinedPlaceholder filename, UndefinedPlaceholder lineNumber) {
Object coerced;

try {
@@ -601,7 +614,7 @@ public Object eval(VirtualFrame frame, RubyBasicObject object, RubyBinding bindi
}

if (coerced instanceof RubyString) {
return getContext().eval(((RubyString) coerced).getBytes(), binding, this);
return getContext().eval(((RubyString) coerced).getBytes(), binding, ownScopeForAssignments, this);
} else {
throw new RaiseException(
getContext().getCoreLibrary().typeError(
@@ -612,15 +625,6 @@ public Object eval(VirtualFrame frame, RubyBasicObject object, RubyBinding bindi
this));
}
}

@Specialization(guards = "!isRubyBinding(arguments[1])")
public Object eval(RubyBasicObject source, RubyBasicObject badBinding, UndefinedPlaceholder filename, UndefinedPlaceholder lineNumber) {
throw new RaiseException(
getContext().getCoreLibrary().typeError(
String.format("wrong argument type %s (expected binding)",
badBinding.getLogicalClass().getName()),
this));
}
}

@CoreMethod(names = "exec", isModuleFunction = true, required = 1, argumentsAsArray = true)
10 changes: 7 additions & 3 deletions truffle/src/main/java/org/jruby/truffle/runtime/RubyContext.java
Original file line number Diff line number Diff line change
@@ -207,13 +207,17 @@ public Object eval(ByteList code, Object self, RubyNode currentNode) {
return execute(this, source, code.getEncoding(), TranslatorDriver.ParserContext.TOP_LEVEL, self, null, currentNode, NodeWrapper.IDENTITY);
}

public Object eval(ByteList code, RubyBinding binding, RubyNode currentNode) {
public Object eval(ByteList code, RubyBinding binding, boolean ownScopeForAssignments, RubyNode currentNode) {
final Source source = Source.fromText(code, "(eval)");
return execute(this, source, code.getEncoding(), TranslatorDriver.ParserContext.TOP_LEVEL, binding.getSelf(), binding.getFrame(), currentNode, NodeWrapper.IDENTITY);
return execute(this, source, code.getEncoding(), TranslatorDriver.ParserContext.TOP_LEVEL, binding.getSelf(), binding.getFrame(), ownScopeForAssignments, currentNode, NodeWrapper.IDENTITY);
}

public Object execute(RubyContext context, Source source, Encoding defaultEncoding, TranslatorDriver.ParserContext parserContext, Object self, MaterializedFrame parentFrame, RubyNode currentNode, NodeWrapper wrapper) {
final RubyRootNode rootNode = translator.parse(context, source, defaultEncoding, parserContext, parentFrame, currentNode, wrapper);
return execute(context, source, defaultEncoding, parserContext, self, parentFrame, true, currentNode, wrapper);
}

public Object execute(RubyContext context, Source source, Encoding defaultEncoding, TranslatorDriver.ParserContext parserContext, Object self, MaterializedFrame parentFrame, boolean ownScopeForAssignments, RubyNode currentNode, NodeWrapper wrapper) {
final RubyRootNode rootNode = translator.parse(context, source, defaultEncoding, parserContext, parentFrame, ownScopeForAssignments, currentNode, wrapper);
final CallTarget callTarget = Truffle.getRuntime().createCallTarget(rootNode);

// TODO(CS): we really need a method here - it's causing problems elsewhere
Original file line number Diff line number Diff line change
@@ -66,7 +66,7 @@ public RubyNode parse(RubyContext context, org.jruby.ast.Node parseTree, org.jru
return translator.compileFunctionNode(null, "(unknown)", argsNode, bodyNode, sharedMethod);
}

public RubyRootNode parse(RubyContext context, Source source, Encoding defaultEncoding, ParserContext parserContext, MaterializedFrame parentFrame, RubyNode currentNode, NodeWrapper wrapper) {
public RubyRootNode parse(RubyContext context, Source source, Encoding defaultEncoding, ParserContext parserContext, MaterializedFrame parentFrame, boolean ownScopeForAssignments, RubyNode currentNode, NodeWrapper wrapper) {
// Set up the JRuby parser

final org.jruby.parser.Parser parser = new org.jruby.parser.Parser(context.getRuntime());
@@ -113,14 +113,14 @@ public RubyRootNode parse(RubyContext context, Source source, Encoding defaultEn
throw new RaiseException(new RubyException(context.getCoreLibrary().getSyntaxErrorClass(), context.makeString(message), RubyCallStack.getBacktrace(currentNode)));
}

return parse(currentNode, context, source, parserContext, parentFrame, node, wrapper);
return parse(currentNode, context, source, parserContext, parentFrame, ownScopeForAssignments, node, wrapper);
}

public RubyRootNode parse(RubyNode currentNode, RubyContext context, Source source, ParserContext parserContext, MaterializedFrame parentFrame, org.jruby.ast.RootNode rootNode, NodeWrapper wrapper) {
public RubyRootNode parse(RubyNode currentNode, RubyContext context, Source source, ParserContext parserContext, MaterializedFrame parentFrame, boolean ownScopeForAssignments, org.jruby.ast.RootNode rootNode, NodeWrapper wrapper) {
final SourceSection sourceSection = source.createSection("<main>", 0, source.getCode().length());
final SharedMethodInfo sharedMethodInfo = new SharedMethodInfo(sourceSection, context.getRootLexicalScope(), "<main>", false, rootNode, false);

final TranslatorEnvironment environment = new TranslatorEnvironment(context, environmentForFrame(context, parentFrame), this, allocateReturnID(), true, false, sharedMethodInfo, sharedMethodInfo.getName(), false);
final TranslatorEnvironment environment = new TranslatorEnvironment(context, environmentForFrame(context, parentFrame), this, allocateReturnID(), ownScopeForAssignments, false, sharedMethodInfo, sharedMethodInfo.getName(), false);

// Get the DATA constant

4 comments on commit f49688f

@nirvdrum
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisseaton @eregon Please review to make sure this is the appropriate fix.

@chrisseaton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok. Hard to follow some of this translator stuff isn't it?

@nirvdrum
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I opted to just introduce the new argument and delegate all existing calls to it. It might make more sense to split method names by intent rather than using an extra argument.

@eregon
Copy link
Member

@eregon eregon commented on f49688f Feb 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, renaming methods by intent would be invaluable for parse/eval and such methods with 4+ arguments and 2+ versions 👍
It's hard to follow in the translator, but looks good.

Please sign in to comment.