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: 005a9027a595
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 994a8a276e1e
Choose a head ref
  • 2 commits
  • 5 files changed
  • 1 contributor

Commits on Sep 14, 2016

  1. Stop using WrapperMethod, since it messes up super logic.

    This is for #3869 and relates to the module_function change from
    and redefine it with a new visibility and implementation class.
    However the impl class never passed through to the contained
    method, preventing it from being used in super. This affected, for
    example, module_fuction singleton methods that need to super or
    methods transplanted using defined_method with a Method instance.
    The new logic always tries to dup the target method so it can
    be truly populated with the altered fields. This change fixed
    
    The previous commit, using cloning instead of construction for
    IR methods, works around the fact that there's no semi-transparent
    WrapperMethod to delegate its serial number to the wrapped method.
    Since in that case and in this one, the method's serial number
    was expected to be the same after duplication, the clone
    technique seems acceptable.
    
    jruby-1_7: Fixes #4155
    headius committed Sep 14, 2016
    Copy the full SHA
    f11ea4b View commit details
  2. Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    Copy the full SHA
    994a8a2 View commit details
13 changes: 10 additions & 3 deletions core/src/main/java/org/jruby/RubyModule.java
Original file line number Diff line number Diff line change
@@ -79,7 +79,6 @@
import org.jruby.internal.runtime.methods.ProcMethod;
import org.jruby.internal.runtime.methods.SynchronizedDynamicMethod;
import org.jruby.internal.runtime.methods.UndefinedMethod;
import org.jruby.internal.runtime.methods.WrapperMethod;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Arity;
import org.jruby.runtime.Block;
@@ -1379,7 +1378,11 @@ public void exportMethod(String name, Visibility visibility) {
method.setVisibility(visibility);
} else {
// FIXME: Why was this using a FullFunctionCallbackMethod before that did callSuper?
addMethod(name, new WrapperMethod(this, method, visibility));
DynamicMethod newMethod = method.dup();
newMethod.setImplementationClass(this);
newMethod.setVisibility(visibility);

addMethod(name, newMethod);
}

invalidateCoreClasses();
@@ -1552,6 +1555,7 @@ public IRubyObject define_method(ThreadContext context, IRubyObject arg0, IRubyO

newMethod = method.getMethod().dup();
newMethod.setImplementationClass(this);
newMethod.setVisibility(visibility);
} else {
throw runtime.newTypeError("wrong argument type " + arg1.getType().getName() + " (expected Proc/Method)");
}
@@ -2247,7 +2251,10 @@ public RubyModule module_function(ThreadContext context, IRubyObject[] args) {
for (int i = 0; i < args.length; i++) {
String name = args[i].asJavaString().intern();
DynamicMethod method = deepMethodSearch(name, runtime);
getSingletonClass().addMethod(name, new WrapperMethod(getSingletonClass(), method, PUBLIC));
DynamicMethod newMethod = method.dup();
newMethod.setImplementationClass(getSingletonClass());
newMethod.setVisibility(PUBLIC);
getSingletonClass().addMethod(name, newMethod);
callMethod(context, "singleton_method_added", context.runtime.fastNewSymbol(name));
}
}
7 changes: 3 additions & 4 deletions core/src/main/java/org/jruby/ast/DefnNode.java
Original file line number Diff line number Diff line change
@@ -36,6 +36,7 @@

import org.jruby.MetaClass;
import org.jruby.Ruby;
import org.jruby.RubyClass;
import org.jruby.RubyModule;
import org.jruby.ast.types.INameNode;
import org.jruby.ast.visitor.NodeVisitor;
@@ -46,6 +47,7 @@
import org.jruby.lexer.yacc.ISourcePosition;
import org.jruby.parser.StaticScope;
import org.jruby.runtime.Block;
import org.jruby.runtime.Helpers;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.Visibility;
import org.jruby.runtime.builtin.IRubyObject;
@@ -116,10 +118,7 @@ public IRubyObject interpret(Ruby runtime, ThreadContext context, IRubyObject se
containingClass.addMethod(name, newMethod);

if (context.getCurrentVisibility() == Visibility.MODULE_FUNCTION) {
containingClass.getSingletonClass().addMethod(name,
new WrapperMethod(containingClass.getSingletonClass(), newMethod, Visibility.PUBLIC));

containingClass.callMethod(context, "singleton_method_added", runtime.fastNewSymbol(name));
Helpers.addModuleMethod(containingClass, name, newMethod, context, runtime.fastNewSymbol(name));
}

// 'class << state.self' and 'class << obj' uses defn as opposed to defs
Original file line number Diff line number Diff line change
@@ -38,6 +38,7 @@
*
* @author jpetersen
*/
@Deprecated
public class WrapperMethod extends DynamicMethod {
private DynamicMethod method;

Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@

import org.jruby.MetaClass;
import org.jruby.Ruby;
import org.jruby.RubyClass;
import org.jruby.RubyModule;
import org.jruby.common.IRubyWarnings.ID;
import org.jruby.internal.runtime.methods.DynamicMethod;
@@ -14,6 +15,7 @@
import org.jruby.ir.transformations.inlining.InlinerInfo;
import org.jruby.runtime.Block;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.Helpers;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.Visibility;
import org.jruby.runtime.builtin.IRubyObject;
@@ -80,8 +82,7 @@ public Object interpret(ThreadContext context, DynamicScope currDynScope, IRubyO
//System.out.println("Added " + name + " to " + clazz + "; self is " + self);

if (context.getCurrentVisibility() == Visibility.MODULE_FUNCTION) {
clazz.getSingletonClass().addMethod(name, new WrapperMethod(clazz.getSingletonClass(), newMethod, Visibility.PUBLIC));
clazz.callMethod(context, "singleton_method_added", runtime.fastNewSymbol(name));
Helpers.addModuleMethod(clazz, name, newMethod, context, runtime.fastNewSymbol(name));
}

// 'class << state.self' and 'class << obj' uses defn as opposed to defs
8 changes: 6 additions & 2 deletions core/src/main/java/org/jruby/runtime/Helpers.java
Original file line number Diff line number Diff line change
@@ -2010,8 +2010,12 @@ public static void addInstanceMethod(RubyModule containingClass, String name, Dy
callNormalMethodHook(containingClass, context, sym);
}

private static void addModuleMethod(RubyModule containingClass, String name, DynamicMethod method, ThreadContext context, RubySymbol sym) {
containingClass.getSingletonClass().addMethod(name, new WrapperMethod(containingClass.getSingletonClass(), method, Visibility.PUBLIC));
public static void addModuleMethod(RubyModule containingClass, String name, DynamicMethod method, ThreadContext context, RubySymbol sym) {
RubyClass singletonClass = containingClass.getSingletonClass();
DynamicMethod modMethod = method.dup();
modMethod.setImplementationClass(singletonClass);
modMethod.setVisibility(Visibility.PUBLIC);
singletonClass.addMethod(name, modMethod);
containingClass.callMethod(context, "singleton_method_added", sym);
}