Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 2e27cfcf26c7
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 7f16e7ab8a6e
Choose a head ref
  • 2 commits
  • 10 files changed
  • 1 contributor

Commits on Apr 8, 2015

  1. Revert "Revert "Fixes #2511 - Keyword method parameters cannot take a… 

    … frozen hash. Fixes #2015 - Method keyword arguments are destructive""

This reverts commit 4c86703.
    @enebo
    enebo committed Apr 8, 2015
    Copy the full SHA
    27186ad View commit details

  2. Do not double frobnicate JIT'd kwargs hash through both MixedModeMeth… 

    …od and CompiledIRMethod. -1 as magic value for specificArity was already used and cloning a CompiledMethod would convert to thinking it was a kwargs method
    @enebo
    enebo committed Apr 8, 2015
    Copy the full SHA
    7f16e7a View commit details
7 changes: 5 additions & 2 deletions core/src/main/java/org/jruby/compiler/JITCompiler.java
View file
Original file line number Diff line number Diff line change
@@ -302,7 +302,9 @@ public void run() {
PUBLIC_LOOKUP.findStatic(sourceClass, jittedName, signatures.get(-1)),
method.getIRMethod(),
method.getVisibility(),
method.getImplementationClass()));
method.getImplementationClass(),
method.getIRMethod().receivesKeywordArgs()));

} else {
// also specific-arity
for (Map.Entry<Integer, MethodType> entry : signatures.entrySet()) {
@@ -315,7 +317,8 @@ public void run() {
entry.getKey(),
method.getIRMethod(),
method.getVisibility(),
method.getImplementationClass()));
method.getImplementationClass(),
method.getIRMethod().receivesKeywordArgs()));
break;
}
}
16 changes: 5 additions & 11 deletions core/src/main/java/org/jruby/internal/runtime/methods/CompiledIRMetaClassBody.java
View file
Original file line number Diff line number Diff line change
@@ -2,7 +2,6 @@

import org.jruby.RubyModule;
import org.jruby.ir.IRFlags;
import org.jruby.ir.IRMetaClassBody;
import org.jruby.ir.IRScope;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Block;
@@ -14,18 +13,15 @@
import java.lang.invoke.MethodHandle;

public class CompiledIRMetaClassBody extends CompiledIRMethod {
private final IRMetaClassBody irMetaClassBody;
private final boolean reuseParentDynScope;
private final boolean pushNewDynScope;
private final boolean popDynScope;

public CompiledIRMetaClassBody(MethodHandle handle, IRScope scope, RubyModule implementationClass) {
super(handle, scope, Visibility.PUBLIC, implementationClass);
super(handle, scope, Visibility.PUBLIC, implementationClass, scope.receivesKeywordArgs());

irMetaClassBody = (IRMetaClassBody)scope;
this.reuseParentDynScope = scope.getFlags().contains(IRFlags.REUSE_PARENT_DYNSCOPE);
this.pushNewDynScope = !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED) && !this.reuseParentDynScope;
this.popDynScope = this.pushNewDynScope || this.reuseParentDynScope;
boolean reuseParentDynScope = scope.getFlags().contains(IRFlags.REUSE_PARENT_DYNSCOPE);
this.pushNewDynScope = !scope.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED) && !reuseParentDynScope;
this.popDynScope = this.pushNewDynScope || reuseParentDynScope;
}

public String[] getParameterList() {
@@ -56,8 +52,6 @@ protected void pre(ThreadContext context, StaticScope staticScope, RubyModule im

@Override
public DynamicMethod dup() {
CompiledIRMetaClassBody x = new CompiledIRMetaClassBody(variable, method, implementationClass);

return x;
return new CompiledIRMetaClassBody(variable, method, implementationClass);
}
}
25 changes: 16 additions & 9 deletions core/src/main/java/org/jruby/internal/runtime/methods/CompiledIRMethod.java
View file
Original file line number Diff line number Diff line change
@@ -3,23 +3,20 @@
import org.jruby.RubyModule;
import org.jruby.ir.IRMethod;
import org.jruby.ir.IRScope;
import org.jruby.ir.runtime.IRRuntimeHelpers;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Arity;
import org.jruby.runtime.Block;
import org.jruby.runtime.PositionAware;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.Visibility;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.util.log.Logger;
import org.jruby.util.log.LoggerFactory;

import java.lang.invoke.MethodHandle;

import org.jruby.runtime.Helpers;

public class CompiledIRMethod extends JavaMethod implements MethodArgs2, PositionAware {
private static final Logger LOG = LoggerFactory.getLogger("CompiledIRMethod");

protected final MethodHandle variable;

protected final MethodHandle specific;
@@ -30,21 +27,27 @@ public class CompiledIRMethod extends JavaMethod implements MethodArgs2, Positio
private String[] parameterList;
private final StaticScope staticScope;
private final boolean hasExplicitCallProtocol;
private final boolean hasKwargs;

public CompiledIRMethod(MethodHandle variable, IRScope method, Visibility visibility, RubyModule implementationClass) {
this(variable, null, -1, method, visibility, implementationClass);
public CompiledIRMethod(MethodHandle variable, IRScope method, Visibility visibility,
RubyModule implementationClass, boolean hasKwargs) {
this(variable, null, -1, method, visibility, implementationClass, hasKwargs);
}

public CompiledIRMethod(MethodHandle variable, MethodHandle specific, int specificArity, IRScope method, Visibility visibility, RubyModule implementationClass) {
public CompiledIRMethod(MethodHandle variable, MethodHandle specific, int specificArity, IRScope method,
Visibility visibility, RubyModule implementationClass, boolean hasKwargs) {
super(implementationClass, visibility, CallConfiguration.FrameNoneScopeNone, method.getName());
this.variable = variable;
this.specific = specific;
this.specificArity = specificArity;
// deopt unboxing if we have to process kwargs hash (although this really has nothing to do with arg
// unboxing -- it was a simple path to hacking this in).
this.specificArity = hasKwargs ? -1 : specificArity;
this.method = method;
this.method.getStaticScope().determineModule();
this.arity = calculateArity();
this.staticScope = method.getStaticScope();
this.hasExplicitCallProtocol = method.hasExplicitCallProtocol();
this.hasKwargs = hasKwargs;

setHandle(variable);
}
@@ -99,6 +102,8 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
try {
if (!hasExplicitCallProtocol) return callNoProtocol(context, self, clazz, name, args, block);

if (hasKwargs) IRRuntimeHelpers.frobnicateKwargsArgument(context, arity.required(), args);

return (IRubyObject)this.variable.invokeExact(context, staticScope, self, args, block, implementationClass, name);
} catch (Throwable t) {
Helpers.throwException(t);
@@ -172,6 +177,8 @@ private IRubyObject callNoProtocol(ThreadContext context, IRubyObject self, Ruby
RubyModule implementationClass = this.implementationClass;
pre(context, staticScope, implementationClass, self, name, block);

if (hasKwargs) IRRuntimeHelpers.frobnicateKwargsArgument(context, arity.required(), args);

try {
return (IRubyObject)this.variable.invokeExact(context, staticScope, self, args, block, implementationClass, name);
} finally {
@@ -237,7 +244,7 @@ public IRubyObject callNoProtocol(ThreadContext context, IRubyObject self, RubyM

@Override
public DynamicMethod dup() {
return new CompiledIRMethod(variable, specific, specificArity, method, visibility, implementationClass);
return new CompiledIRMethod(variable, specific, specificArity, method, visibility, implementationClass, hasKwargs);
}

public String getFile() {
2 changes: 0 additions & 2 deletions core/src/main/java/org/jruby/internal/runtime/methods/MixedModeIRMethod.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package org.jruby.internal.runtime.methods;

import java.util.List;

import org.jruby.MetaClass;
import org.jruby.Ruby;
import org.jruby.RubyClass;
2 changes: 0 additions & 2 deletions core/src/main/java/org/jruby/ir/IRMethod.java
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package org.jruby.ir;

import java.lang.invoke.MethodType;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.jruby.ast.MethodDefNode;
import org.jruby.ir.interpreter.InterpreterContext;
6 changes: 4 additions & 2 deletions core/src/main/java/org/jruby/ir/interpreter/InterpreterContext.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package org.jruby.ir.interpreter;

import java.util.Collection;
import java.util.List;
import org.jruby.ir.IRClosure;
import org.jruby.ir.IRFlags;
import org.jruby.ir.IRMetaClassBody;
import org.jruby.ir.IRMethod;
@@ -78,6 +76,10 @@ private Instr[] prepareBuildInstructions(List<Instr> instructions) {
return linearizedInstrArray;
}

public int getRequiredArgsCount() {
return getStaticScope().getRequiredArgs();
}

public IRScope getScope() {
return scope;
}
3 changes: 3 additions & 0 deletions core/src/main/java/org/jruby/ir/interpreter/InterpreterEngine.java
View file
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@
import org.jruby.RubyBoolean;
import org.jruby.RubyFixnum;
import org.jruby.RubyFloat;
import org.jruby.RubyHash;
import org.jruby.RubyModule;
import org.jruby.common.IRubyWarnings;
import org.jruby.exceptions.Unrescuable;
@@ -113,6 +114,8 @@ public IRubyObject interpret(ThreadContext context, IRubyObject self,
int ipc = 0;
Object exception = null;

if (interpreterContext.receivesKeywordArguments()) IRRuntimeHelpers.frobnicateKwargsArgument(context, interpreterContext.getRequiredArgsCount(), args);

StaticScope currScope = interpreterContext.getStaticScope();
DynamicScope currDynScope = context.getCurrentScope();
IRScope scope = currScope.getIRScope();
2 changes: 2 additions & 0 deletions core/src/main/java/org/jruby/ir/interpreter/StartupInterpreterEngine.java
View file
Original file line number Diff line number Diff line change
@@ -39,6 +39,8 @@ public IRubyObject interpret(ThreadContext context, IRubyObject self,
int ipc = 0;
Object exception = null;

if (interpreterContext.receivesKeywordArguments()) IRRuntimeHelpers.frobnicateKwargsArgument(context, interpreterContext.getRequiredArgsCount(), args);

StaticScope currScope = interpreterContext.getStaticScope();
DynamicScope currDynScope = context.getCurrentScope();
IRScope scope = currScope.getIRScope();
29 changes: 21 additions & 8 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
View file
Original file line number Diff line number Diff line change
@@ -246,7 +246,7 @@ public static void defCompiledIRMethod(ThreadContext context, MethodHandle handl
Visibility currVisibility = context.getCurrentVisibility();
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, containingClass, rubyName, currVisibility);

DynamicMethod method = new CompiledIRMethod(handle, irScope, newVisibility, containingClass);
DynamicMethod method = new CompiledIRMethod(handle, irScope, newVisibility, containingClass, irScope.receivesKeywordArgs());

Helpers.addInstanceMethod(containingClass, rubyName, method, currVisibility, context, runtime);
}
@@ -263,7 +263,7 @@ public static void defCompiledIRClassMethod(ThreadContext context, IRubyObject o

RubyClass containingClass = obj.getSingletonClass();

DynamicMethod method = new CompiledIRMethod(handle, irScope, Visibility.PUBLIC, containingClass);
DynamicMethod method = new CompiledIRMethod(handle, irScope, Visibility.PUBLIC, containingClass, irScope.receivesKeywordArgs());

containingClass.addMethod(rubyName, method);

@@ -524,6 +524,19 @@ public static void checkArity(ThreadContext context, Object[] args, int required
}
}

// Due to our current strategy of destructively processing the kwargs hash we need to dup
// and make sure the copy is not frozen. This has a poor name as encouragement to rewrite
// how we handle kwargs internally :)
public static void frobnicateKwargsArgument(ThreadContext context, int requiredArguments, IRubyObject[] args) {
RubyHash kwargs = IRRuntimeHelpers.extractKwargsHash(args, requiredArguments, true);

if (kwargs != null) {
kwargs = (RubyHash) kwargs.dup(context);
kwargs.setFrozen(false);
args[args.length - 1] = kwargs;
}
}

public static RubyHash extractKwargsHash(Object[] args, int requiredArgsCount, boolean receivesKwargs) {
if (!receivesKwargs) return null;
if (args.length <= requiredArgsCount) return null; // No kwarg because required args slurp them up.
@@ -1093,7 +1106,7 @@ public static DynamicMethod newInterpretedModuleBody(ThreadContext context, IRSc
@JIT
public static DynamicMethod newCompiledModuleBody(ThreadContext context, MethodHandle handle, IRScope irModule, Object rubyContainer) {
RubyModule newRubyModule = newRubyModuleFromIR(context, irModule, rubyContainer);
return new CompiledIRMethod(handle, irModule, Visibility.PUBLIC, newRubyModule);
return new CompiledIRMethod(handle, irModule, Visibility.PUBLIC, newRubyModule, false);
}

private static RubyModule newRubyModuleFromIR(ThreadContext context, IRScope irModule, Object rubyContainer) {
@@ -1117,7 +1130,7 @@ public static DynamicMethod newInterpretedClassBody(ThreadContext context, IRSco
public static DynamicMethod newCompiledClassBody(ThreadContext context, MethodHandle handle, IRScope irClassBody, Object container, Object superClass) {
RubyModule newRubyClass = newRubyClassFromIR(context.runtime, irClassBody, superClass, container);

return new CompiledIRMethod(handle, irClassBody, Visibility.PUBLIC, newRubyClass);
return new CompiledIRMethod(handle, irClassBody, Visibility.PUBLIC, newRubyClass, false);
}

public static RubyModule newRubyClassFromIR(Ruby runtime, IRScope irClassBody, Object superClass, Object container) {
@@ -1164,15 +1177,15 @@ public static void defInterpretedClassMethod(ThreadContext context, IRScope meth
public static void defCompiledClassMethod(ThreadContext context, MethodHandle handle, IRScope method, IRubyObject obj) {
RubyClass rubyClass = checkClassForDef(context, method, obj);

rubyClass.addMethod(method.getName(), new CompiledIRMethod(handle, method, Visibility.PUBLIC, rubyClass));
rubyClass.addMethod(method.getName(), new CompiledIRMethod(handle, method, Visibility.PUBLIC, rubyClass, method.receivesKeywordArgs()));
obj.callMethod(context, "singleton_method_added", context.runtime.fastNewSymbol(method.getName()));
}

@JIT
public static void defCompiledClassMethod(ThreadContext context, MethodHandle variable, MethodHandle specific, int specificArity, IRScope method, IRubyObject obj) {
RubyClass rubyClass = checkClassForDef(context, method, obj);

rubyClass.addMethod(method.getName(), new CompiledIRMethod(variable, specific, specificArity, method, Visibility.PUBLIC, rubyClass));
rubyClass.addMethod(method.getName(), new CompiledIRMethod(variable, specific, specificArity, method, Visibility.PUBLIC, rubyClass, method.receivesKeywordArgs()));
obj.callMethod(context, "singleton_method_added", context.runtime.fastNewSymbol(method.getName()));
}

@@ -1212,7 +1225,7 @@ public static void defCompiledInstanceMethod(ThreadContext context, MethodHandle
Visibility currVisibility = context.getCurrentVisibility();
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, method.getName(), currVisibility);

DynamicMethod newMethod = new CompiledIRMethod(handle, method, newVisibility, clazz);
DynamicMethod newMethod = new CompiledIRMethod(handle, method, newVisibility, clazz, method.receivesKeywordArgs());

Helpers.addInstanceMethod(clazz, method.getName(), newMethod, currVisibility, context, runtime);
}
@@ -1225,7 +1238,7 @@ public static void defCompiledInstanceMethod(ThreadContext context, MethodHandle
Visibility currVisibility = context.getCurrentVisibility();
Visibility newVisibility = Helpers.performNormalMethodChecksAndDetermineVisibility(runtime, clazz, method.getName(), currVisibility);

DynamicMethod newMethod = new CompiledIRMethod(variable, specific, specificArity, method, newVisibility, clazz);
DynamicMethod newMethod = new CompiledIRMethod(variable, specific, specificArity, method, newVisibility, clazz, method.receivesKeywordArgs());

Helpers.addInstanceMethod(clazz, method.getName(), newMethod, currVisibility, context, runtime);
}
6 changes: 4 additions & 2 deletions core/src/main/java/org/jruby/runtime/CompiledIRBlockBody.java
View file
Original file line number Diff line number Diff line change
@@ -4,9 +4,7 @@
import org.jruby.ir.IRClosure;
import org.jruby.ir.IRFlags;
import org.jruby.ir.IRScope;
import org.jruby.ir.representations.CFG;
import org.jruby.ir.runtime.IRRuntimeHelpers;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Block.Type;
import org.jruby.runtime.builtin.IRubyObject;

@@ -17,6 +15,7 @@ public class CompiledIRBlockBody extends IRBlockBody {
protected final MethodHandle handle;
protected boolean pushScope;
protected boolean reuseParentScope;
protected boolean usesKwargs;

public CompiledIRBlockBody(MethodHandle handle, IRScope closure, long encodedSignature) {
super(closure.getStaticScope(), ((IRClosure)closure).getParameterList(), closure.getFileName(), closure.getLineNumber(), Signature.decode(encodedSignature));
@@ -25,6 +24,7 @@ public CompiledIRBlockBody(MethodHandle handle, IRScope closure, long encodedSig
// FIXME: duplicated from InterpreterContext
this.reuseParentScope = closure.getFlags().contains(IRFlags.REUSE_PARENT_DYNSCOPE);
this.pushScope = !closure.getFlags().contains(IRFlags.DYNSCOPE_ELIMINATED) && !this.reuseParentScope;
this.usesKwargs = closure.receivesKeywordArgs();

// Done in the interpreter (WrappedIRClosure) but we do it here
closure.getStaticScope().determineModule();
@@ -62,6 +62,8 @@ protected IRubyObject commonYieldPath(ThreadContext context, IRubyObject[] args,
}
this.evalType.set(EvalType.NONE);

if (usesKwargs) IRRuntimeHelpers.frobnicateKwargsArgument(context, arity.required(), args);

try {
return (IRubyObject)handle.invokeExact(context, getStaticScope(), self, args, block, binding.getMethod(), type);
} catch (Throwable t) {